From a25ce96ba35f001b974cb32e008efb7ef46710e4 Mon Sep 17 00:00:00 2001 From: Klement Sekera Date: Mon, 15 Nov 2021 15:52:37 +0100 Subject: vapi: verify message size when received Verifying message size including VLA size allows to dismiss some coverity warnings in generated code. Type: improvement Signed-off-by: Klement Sekera Change-Id: I824658881254b3e7a9bfca228a266cfee448cc2e --- src/vpp-api/vapi/vapi.c | 22 +++++++++----------- src/vpp-api/vapi/vapi_c_gen.py | 43 +++++++++++++++++++++++++++++++++++++--- src/vpp-api/vapi/vapi_internal.h | 4 ++-- 3 files changed, 51 insertions(+), 18 deletions(-) (limited to 'src/vpp-api/vapi') diff --git a/src/vpp-api/vapi/vapi.c b/src/vpp-api/vapi/vapi.c index 513ff98b30c..fbe04c187f3 100644 --- a/src/vpp-api/vapi/vapi.c +++ b/src/vpp-api/vapi/vapi.c @@ -753,12 +753,19 @@ vapi_msg_is_with_context (vapi_msg_id_t id) return __vapi_metadata.msgs[id]->has_context; } +static int +vapi_verify_msg_size (vapi_msg_id_t id, void *buf, uword buf_size) +{ + assert (id < __vapi_metadata.count); + return __vapi_metadata.msgs[id]->verify_msg_size (buf, buf_size); +} + vapi_error_e vapi_dispatch_one (vapi_ctx_t ctx) { VAPI_DBG ("vapi_dispatch_one()"); void *msg; - size_t size; + uword size; vapi_error_e rv = vapi_recv (ctx, &msg, &size, SVM_Q_WAIT, 0); if (VAPI_OK != rv) { @@ -781,12 +788,8 @@ vapi_dispatch_one (vapi_ctx_t ctx) return VAPI_EINVAL; } const vapi_msg_id_t id = ctx->vl_msg_id_to_vapi_msg_t[vpp_id]; - const size_t expect_size = vapi_get_message_size (id); - if (size < expect_size) + if (vapi_verify_msg_size (id, msg, size)) { - VAPI_ERR - ("Invalid msg received, unexpected size `%zu' < expected min `%zu'", - size, expect_size); vapi_msg_free (ctx, msg); return VAPI_EINVAL; } @@ -899,13 +902,6 @@ void (*vapi_get_swap_to_be_func (vapi_msg_id_t id)) (void *msg) return __vapi_metadata.msgs[id]->swap_to_be; } -size_t -vapi_get_message_size (vapi_msg_id_t id) -{ - assert (id < __vapi_metadata.count); - return __vapi_metadata.msgs[id]->size; -} - size_t vapi_get_context_offset (vapi_msg_id_t id) { diff --git a/src/vpp-api/vapi/vapi_c_gen.py b/src/vpp-api/vapi/vapi_c_gen.py index 1a5665e5500..100263ccbc3 100755 --- a/src/vpp-api/vapi/vapi_c_gen.py +++ b/src/vpp-api/vapi/vapi_c_gen.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 import argparse +import inspect import os import sys import logging @@ -20,7 +21,8 @@ class CField(Field): return "vl_api_string_t %s;" % (self.name) else: if self.len is not None and type(self.len) != dict: - return "%s %s[%d];" % (self.type.get_c_name(), self.name, self.len) + return "%s %s[%d];" % (self.type.get_c_name(), self.name, + self.len) else: return "%s %s;" % (self.type.get_c_name(), self.name) @@ -366,6 +368,37 @@ class CMessage (Message): "}", ]) + def get_verify_msg_size_func_name(self): + return f"vapi_verify_{self.name}_msg_size" + + def get_verify_msg_size_func_decl(self): + return "int %s(%s *msg, uword buf_size)" % ( + self.get_verify_msg_size_func_name(), + self.get_c_name()) + + def get_verify_msg_size_func_def(self): + return inspect.cleandoc( + f""" + {self.get_verify_msg_size_func_decl()} + {{ + if (sizeof({self.get_c_name()}) > buf_size) + {{ + VAPI_ERR("Truncated '{self.name}' msg received, received %lu" + "bytes, expected %lu bytes.", buf_size, + sizeof({self.get_c_name()})); + return -1; + }} + if ({self.get_calc_msg_size_func_name()}(msg) > buf_size) + {{ + VAPI_ERR("Truncated '{self.name}' msg received, received %lu" + "bytes, expected %lu bytes.", buf_size, + sizeof({self.get_calc_msg_size_func_name()})); + return -1; + }} + return 0; + }} + """) + def get_c_def(self): if self.has_payload(): return "\n".join([ @@ -600,7 +633,8 @@ class CMessage (Message): if has_context else ' 0,', (' offsetof(%s, payload),' % self.get_c_name()) if self.has_payload() else ' VAPI_INVALID_MSG_ID,', - ' sizeof(%s),' % self.get_c_name(), + ' (verify_msg_size_fn_t)%s,' % + self.get_verify_msg_size_func_name(), ' (generic_swap_fn_t)%s,' % self.get_swap_to_be_func_name(), ' (generic_swap_fn_t)%s,' % self.get_swap_to_host_func_name(), ' VAPI_INVALID_MSG_ID,', @@ -666,6 +700,8 @@ def emit_definition(parser, json_file, emitted, o): print("%s%s" % (function_attrs, o.get_swap_to_host_func_def())) print("") print("%s%s" % (function_attrs, o.get_calc_msg_size_func_def())) + print("") + print("%s%s" % (function_attrs, o.get_verify_msg_size_func_def())) if not o.is_reply and not o.is_event: print("") print("%s%s" % (function_attrs, o.get_alloc_func_def())) @@ -691,7 +727,8 @@ def gen_json_unified_header(parser, logger, j, io, name): orig_stdout = sys.stdout sys.stdout = io include_guard = "__included_%s" % ( - j.replace(".", "_").replace("/", "_").replace("-", "_").replace("+", "_")) + j.replace(".", "_").replace("/", "_").replace("-", "_").replace( + "+", "_")) print("#ifndef %s" % include_guard) print("#define %s" % include_guard) print("") diff --git a/src/vpp-api/vapi/vapi_internal.h b/src/vpp-api/vapi/vapi_internal.h index e9a9726d86e..49c041769d0 100644 --- a/src/vpp-api/vapi/vapi_internal.h +++ b/src/vpp-api/vapi/vapi_internal.h @@ -82,6 +82,7 @@ typedef vapi_error_e (*vapi_cb_t) (struct vapi_ctx_s *, void *, vapi_error_e, bool, void *); typedef void (*generic_swap_fn_t) (void *payload); +typedef int (*verify_msg_size_fn_t) (void *msg, uword buf_size); typedef struct { @@ -92,7 +93,7 @@ typedef struct bool has_context; unsigned int context_offset; unsigned int payload_offset; - size_t size; + verify_msg_size_fn_t verify_msg_size; generic_swap_fn_t swap_to_be; generic_swap_fn_t swap_to_host; vapi_msg_id_t id; /* assigned at run-time */ @@ -122,7 +123,6 @@ void vapi_store_request (vapi_ctx_t ctx, u32 context, bool is_dump, int vapi_get_payload_offset (vapi_msg_id_t id); void (*vapi_get_swap_to_host_func (vapi_msg_id_t id)) (void *payload); void (*vapi_get_swap_to_be_func (vapi_msg_id_t id)) (void *payload); -size_t vapi_get_message_size (vapi_msg_id_t id); size_t vapi_get_context_offset (vapi_msg_id_t id); bool vapi_msg_is_with_context (vapi_msg_id_t id); size_t vapi_get_message_count(); -- cgit 1.2.3-korg