summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKlement Sekera <ksekera@cisco.com>2021-12-02 16:36:34 +0000
committerOle Tr�an <otroan@employees.org>2021-12-08 09:39:21 +0000
commit0eb83f484cc74b7ab4e26f6811a3e98442a25e7d (patch)
tree9d82ca16b6aba91f9c3c4d65420e694909a6d585
parent074f3970c6d68d30878e5a2a3a80904182f15e89 (diff)
api: improve REPLY_MACRO safety
Improve vppapigen to generate per-message #define indicating whether said message is dynamically sized (due to VLA or string) or not. Use these #defines in REPLY_MACROs to prevent improper usage. Fix existing improper REPLY_MACRO* usage. Type: improvement Change-Id: Ia77aaf9f6cf3ed68ea21075a4cc8deda78a68651 Signed-off-by: Klement Sekera <ksekera@cisco.com>
-rw-r--r--src/tools/vppapigen/vppapigen_c.py4
-rw-r--r--src/vlibapi/api_helper_macros.h221
-rw-r--r--src/vnet/session/session_api.c76
3 files changed, 167 insertions, 134 deletions
diff --git a/src/tools/vppapigen/vppapigen_c.py b/src/tools/vppapigen/vppapigen_c.py
index d0f63fbdb0c..1e18a83e65e 100644
--- a/src/tools/vppapigen/vppapigen_c.py
+++ b/src/tools/vppapigen/vppapigen_c.py
@@ -24,6 +24,7 @@ VAT2 tests.
'''
import datetime
+import itertools
import os
import time
import sys
@@ -1256,7 +1257,7 @@ def generate_include_types(s, module, stream):
filename = i.filename.replace('plugins/', '')
write('#include <{}_types.h>\n'.format(filename))
- for o in s['types'] + s['Define']:
+ for o in itertools.chain(s['types'], s['Define']):
tname = o.__class__.__name__
if tname == 'Using':
if 'length' in o.alias:
@@ -1315,6 +1316,7 @@ def generate_include_types(s, module, stream):
.format(b, o.name))
write('} vl_api_%s_t;\n' % o.name)
+ write(f'#define VL_API_{o.name.upper()}_IS_CONSTANT_SIZE ({0 if o.vla else 1})\n\n')
for t in s['Define']:
write('#define VL_API_{ID}_CRC "{n}_{crc:08x}"\n'
diff --git a/src/vlibapi/api_helper_macros.h b/src/vlibapi/api_helper_macros.h
index 5c4dd9a6fd5..6b5b37ae746 100644
--- a/src/vlibapi/api_helper_macros.h
+++ b/src/vlibapi/api_helper_macros.h
@@ -33,20 +33,26 @@
endian_fp = am->msg_endian_handlers[t + (REPLY_MSG_ID_BASE)]; \
(*endian_fp) (rmp);
-#define REPLY_MACRO(t) \
-do { \
- vl_api_registration_t *rp; \
- rp = vl_api_client_index_to_registration (mp->client_index); \
- if (rp == 0) \
- return; \
- \
- rmp = vl_msg_api_alloc (sizeof (*rmp)); \
- rmp->_vl_msg_id = htons((t)+(REPLY_MSG_ID_BASE)); \
- rmp->context = mp->context; \
- rmp->retval = ntohl(rv); \
- \
- vl_api_send_msg (rp, (u8 *)rmp); \
-} while(0);
+#define REPLY_MACRO(msg) \
+ do \
+ { \
+ STATIC_ASSERT ( \
+ msg##_IS_CONSTANT_SIZE, \
+ "REPLY_MACRO can only be used with constant size messages, " \
+ "use REPLY_MACRO[3|4]* instead"); \
+ vl_api_registration_t *rp; \
+ rp = vl_api_client_index_to_registration (mp->client_index); \
+ if (rp == 0) \
+ return; \
+ \
+ rmp = vl_msg_api_alloc (sizeof (*rmp)); \
+ rmp->_vl_msg_id = htons (msg + (REPLY_MSG_ID_BASE)); \
+ rmp->context = mp->context; \
+ rmp->retval = ntohl (rv); \
+ \
+ vl_api_send_msg (rp, (u8 *) rmp); \
+ } \
+ while (0);
#define REPLY_MACRO_END(t) \
do \
@@ -65,20 +71,30 @@ do { \
} \
while (0);
-#define REPLY_MACRO2(t, body) \
-do { \
- vl_api_registration_t *rp; \
- rp = vl_api_client_index_to_registration (mp->client_index); \
- if (rp == 0) \
- return; \
- \
- rmp = vl_msg_api_alloc (sizeof (*rmp)); \
- rmp->_vl_msg_id = htons((t)+(REPLY_MSG_ID_BASE)); \
- rmp->context = mp->context; \
- rmp->retval = ntohl(rv); \
- do {body;} while (0); \
- vl_api_send_msg (rp, (u8 *)rmp); \
-} while(0);
+#define REPLY_MACRO2(t, body) \
+ do \
+ { \
+ STATIC_ASSERT ( \
+ t##_IS_CONSTANT_SIZE, \
+ "REPLY_MACRO2 can only be used with constant size messages, " \
+ "use REPLY_MACRO[3|4]* instead"); \
+ vl_api_registration_t *rp; \
+ rp = vl_api_client_index_to_registration (mp->client_index); \
+ if (rp == 0) \
+ return; \
+ \
+ rmp = vl_msg_api_alloc (sizeof (*rmp)); \
+ rmp->_vl_msg_id = htons ((t) + (REPLY_MSG_ID_BASE)); \
+ rmp->context = mp->context; \
+ rmp->retval = ntohl (rv); \
+ do \
+ { \
+ body; \
+ } \
+ while (0); \
+ vl_api_send_msg (rp, (u8 *) rmp); \
+ } \
+ while (0);
#define REPLY_MACRO2_END(t, body) \
do \
@@ -230,20 +246,26 @@ do { \
} \
while (0);
-#define REPLY_MACRO3(t, n, body) \
-do { \
- vl_api_registration_t *rp; \
- rp = vl_api_client_index_to_registration (mp->client_index); \
- if (rp == 0) \
- return; \
- \
- rmp = vl_msg_api_alloc (sizeof (*rmp) + n); \
- rmp->_vl_msg_id = htons((t)+(REPLY_MSG_ID_BASE)); \
- rmp->context = mp->context; \
- rmp->retval = ntohl(rv); \
- do {body;} while (0); \
- vl_api_send_msg (rp, (u8 *)rmp); \
-} while(0);
+#define REPLY_MACRO3(t, n, body) \
+ do \
+ { \
+ vl_api_registration_t *rp; \
+ rp = vl_api_client_index_to_registration (mp->client_index); \
+ if (rp == 0) \
+ return; \
+ \
+ rmp = vl_msg_api_alloc (sizeof (*rmp) + (n)); \
+ rmp->_vl_msg_id = htons ((t) + (REPLY_MSG_ID_BASE)); \
+ rmp->context = mp->context; \
+ rmp->retval = ntohl (rv); \
+ do \
+ { \
+ body; \
+ } \
+ while (0); \
+ vl_api_send_msg (rp, (u8 *) rmp); \
+ } \
+ while (0);
#define REPLY_MACRO3_END(t, n, body) \
do \
@@ -619,62 +641,65 @@ bad_bd_id: \
; \
} while (0);
-#define pub_sub_handler(lca,UCA) \
-static void vl_api_want_##lca##_t_handler ( \
- vl_api_want_##lca##_t *mp) \
-{ \
- vpe_api_main_t *vam = &vpe_api_main; \
- vpe_client_registration_t *rp; \
- vl_api_want_##lca##_reply_t *rmp; \
- uword *p; \
- i32 rv = 0; \
- \
- p = hash_get (vam->lca##_registration_hash, mp->client_index); \
- if (p) { \
- if (mp->enable_disable) { \
- clib_warning ("pid %d: already enabled...", ntohl(mp->pid)); \
- rv = VNET_API_ERROR_INVALID_REGISTRATION; \
- goto reply; \
- } else { \
- rp = pool_elt_at_index (vam->lca##_registrations, p[0]); \
- pool_put (vam->lca##_registrations, rp); \
- hash_unset (vam->lca##_registration_hash, \
- mp->client_index); \
- goto reply; \
- } \
- } \
- if (mp->enable_disable == 0) { \
- clib_warning ("pid %d: already disabled...", mp->pid); \
- rv = VNET_API_ERROR_INVALID_REGISTRATION; \
- goto reply; \
- } \
- pool_get (vam->lca##_registrations, rp); \
- rp->client_index = mp->client_index; \
- rp->client_pid = mp->pid; \
- hash_set (vam->lca##_registration_hash, rp->client_index, \
- rp - vam->lca##_registrations); \
- \
-reply: \
- REPLY_MACRO (VL_API_WANT_##UCA##_REPLY); \
-} \
- \
-static clib_error_t * vl_api_want_##lca##_t_reaper (u32 client_index) \
-{ \
- vpe_api_main_t *vam = &vpe_api_main; \
- vpe_client_registration_t *rp; \
- uword *p; \
- \
- p = hash_get (vam->lca##_registration_hash, client_index); \
- if (p) \
- { \
- rp = pool_elt_at_index (vam->lca##_registrations, p[0]); \
- pool_put (vam->lca##_registrations, rp); \
- hash_unset (vam->lca##_registration_hash, client_index); \
- } \
- return (NULL); \
-} \
- \
-VL_MSG_API_REAPER_FUNCTION (vl_api_want_##lca##_t_reaper); \
+#define pub_sub_handler(lca, UCA) \
+ static void vl_api_want_##lca##_t_handler (vl_api_want_##lca##_t *mp) \
+ { \
+ vpe_api_main_t *vam = &vpe_api_main; \
+ vpe_client_registration_t *rp; \
+ vl_api_want_##lca##_reply_t *rmp; \
+ uword *p; \
+ i32 rv = 0; \
+ \
+ p = hash_get (vam->lca##_registration_hash, mp->client_index); \
+ if (p) \
+ { \
+ if (mp->enable_disable) \
+ { \
+ clib_warning ("pid %d: already enabled...", ntohl (mp->pid)); \
+ rv = VNET_API_ERROR_INVALID_REGISTRATION; \
+ goto reply; \
+ } \
+ else \
+ { \
+ rp = pool_elt_at_index (vam->lca##_registrations, p[0]); \
+ pool_put (vam->lca##_registrations, rp); \
+ hash_unset (vam->lca##_registration_hash, mp->client_index); \
+ goto reply; \
+ } \
+ } \
+ if (mp->enable_disable == 0) \
+ { \
+ clib_warning ("pid %d: already disabled...", mp->pid); \
+ rv = VNET_API_ERROR_INVALID_REGISTRATION; \
+ goto reply; \
+ } \
+ pool_get (vam->lca##_registrations, rp); \
+ rp->client_index = mp->client_index; \
+ rp->client_pid = mp->pid; \
+ hash_set (vam->lca##_registration_hash, rp->client_index, \
+ rp - vam->lca##_registrations); \
+ \
+ reply: \
+ REPLY_MACRO (VL_API_WANT_##UCA##_REPLY); \
+ } \
+ \
+ static clib_error_t *vl_api_want_##lca##_t_reaper (u32 client_index) \
+ { \
+ vpe_api_main_t *vam = &vpe_api_main; \
+ vpe_client_registration_t *rp; \
+ uword *p; \
+ \
+ p = hash_get (vam->lca##_registration_hash, client_index); \
+ if (p) \
+ { \
+ rp = pool_elt_at_index (vam->lca##_registrations, p[0]); \
+ pool_put (vam->lca##_registrations, rp); \
+ hash_unset (vam->lca##_registration_hash, client_index); \
+ } \
+ return (NULL); \
+ } \
+ \
+ VL_MSG_API_REAPER_FUNCTION (vl_api_want_##lca##_t_reaper);
#define foreach_registration_hash \
_(interface_events) \
diff --git a/src/vnet/session/session_api.c b/src/vnet/session/session_api.c
index 2121d2075e6..767a24aa170 100644
--- a/src/vnet/session/session_api.c
+++ b/src/vnet/session/session_api.c
@@ -696,25 +696,28 @@ vl_api_app_attach_t_handler (vl_api_app_attach_t * mp)
done:
/* *INDENT-OFF* */
- REPLY_MACRO2 (VL_API_APP_ATTACH_REPLY, ({
- if (!rv)
- {
- ctrl_thread = n_workers ? 1 : 0;
- segp = (fifo_segment_t *) a->segment;
- rmp->app_index = clib_host_to_net_u32 (a->app_index);
- rmp->app_mq = fifo_segment_msg_q_offset (segp, 0);
- rmp->vpp_ctrl_mq = fifo_segment_msg_q_offset (rx_mqs_seg, ctrl_thread);
- rmp->vpp_ctrl_mq_thread = ctrl_thread;
- rmp->n_fds = n_fds;
- rmp->fd_flags = fd_flags;
- if (vec_len (segp->ssvm.name))
- {
- vl_api_vec_to_api_string (segp->ssvm.name, &rmp->segment_name);
- }
- rmp->segment_size = segp->ssvm.ssvm_size;
- rmp->segment_handle = clib_host_to_net_u64 (a->segment_handle);
- }
- }));
+ REPLY_MACRO3 (
+ VL_API_APP_ATTACH_REPLY,
+ ((!rv) ? vec_len (((fifo_segment_t *) a->segment)->ssvm.name) : 0), ({
+ if (!rv)
+ {
+ ctrl_thread = n_workers ? 1 : 0;
+ segp = (fifo_segment_t *) a->segment;
+ rmp->app_index = clib_host_to_net_u32 (a->app_index);
+ rmp->app_mq = fifo_segment_msg_q_offset (segp, 0);
+ rmp->vpp_ctrl_mq =
+ fifo_segment_msg_q_offset (rx_mqs_seg, ctrl_thread);
+ rmp->vpp_ctrl_mq_thread = ctrl_thread;
+ rmp->n_fds = n_fds;
+ rmp->fd_flags = fd_flags;
+ if (vec_len (segp->ssvm.name))
+ {
+ vl_api_vec_to_api_string (segp->ssvm.name, &rmp->segment_name);
+ }
+ rmp->segment_size = segp->ssvm.ssvm_size;
+ rmp->segment_handle = clib_host_to_net_u64 (a->segment_handle);
+ }
+ }));
/* *INDENT-ON* */
if (n_fds)
@@ -780,22 +783,25 @@ vl_api_app_worker_add_del_t_handler (vl_api_app_worker_add_del_t * mp)
/* *INDENT-OFF* */
done:
- REPLY_MACRO2 (VL_API_APP_WORKER_ADD_DEL_REPLY, ({
- rmp->is_add = mp->is_add;
- rmp->wrk_index = clib_host_to_net_u32 (args.wrk_map_index);
- rmp->segment_handle = clib_host_to_net_u64 (args.segment_handle);
- if (!rv && mp->is_add)
- {
- rmp->app_event_queue_address =
- fifo_segment_msg_q_offset ((fifo_segment_t *) args.segment, 0);
- rmp->n_fds = n_fds;
- rmp->fd_flags = fd_flags;
- if (vec_len (args.segment->name))
- {
- vl_api_vec_to_api_string (args.segment->name, &rmp->segment_name);
- }
- }
- }));
+ REPLY_MACRO3 (
+ VL_API_APP_WORKER_ADD_DEL_REPLY,
+ ((!rv && mp->is_add) ? vec_len (args.segment->name) : 0), ({
+ rmp->is_add = mp->is_add;
+ rmp->wrk_index = clib_host_to_net_u32 (args.wrk_map_index);
+ rmp->segment_handle = clib_host_to_net_u64 (args.segment_handle);
+ if (!rv && mp->is_add)
+ {
+ rmp->app_event_queue_address =
+ fifo_segment_msg_q_offset ((fifo_segment_t *) args.segment, 0);
+ rmp->n_fds = n_fds;
+ rmp->fd_flags = fd_flags;
+ if (vec_len (args.segment->name))
+ {
+ vl_api_vec_to_api_string (args.segment->name,
+ &rmp->segment_name);
+ }
+ }
+ }));
/* *INDENT-ON* */
if (n_fds)