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 <ksekera@cisco.com>
Change-Id: I824658881254b3e7a9bfca228a266cfee448cc2e

diff --git a/src/vpp-api/vapi/vapi.c b/src/vpp-api/vapi/vapi.c
index 513ff98..fbe04c1 100644 (file)
--- 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 1a5665e..100263c 100755 (executable)
--- 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 e9a9726..49c0417 100644 (file)
--- 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();