[PATCH net-next 2/2] tools: ynl: check alloc fails in generated code

Thaison Phan <[email protected]>
Newsgroups gmane.linux.network,gmane.linux.kernel
Message-ID <[email protected]>
Generated YNL code does not check the return value of malloc() and calloc()
before passing the resulting pointer to memcpy(). This could lead to a NULL
pointer dereference on memory allocation failure.

Updated the C code generator to check for allocation failures and to return
an error code when applicable, or to return with no error code for a user
to check for a NULL in the field that allocation was attempted for.

Signed-off-by: Thaison Phan <[email protected]>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 53 ++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 2fd68c738075..a323acdc42ba 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -526,16 +526,20 @@ class TypeString(Type):
 
     def _attr_get(self, ri, var):
         len_mem = var + '->_len.' + self.c_name
-        return [f"{len_mem} = len;",
-                f"{var}->{self.c_name} = malloc(len + 1);",
+        return [f"{var}->{self.c_name} = malloc(len + 1);",
+                f"if (!{var}->{self.c_name})",
+                "return YNL_PARSE_CB_ERROR;",
+                f"{len_mem} = len;",
                 f"memcpy({var}->{self.c_name}, ynl_attr_get_str(attr), len);",
                 f"{var}->{self.c_name}[len] = 0;"], \
                ['len = strnlen(ynl_attr_get_str(attr), ynl_attr_data_len(attr));'], \
                ['unsigned int len;']
 
     def _setter_lines(self, ri, member, presence):
-        return [f"{presence} = strlen({self.c_name});",
-                f"{member} = malloc({presence} + 1);",
+        return [f"{member} = malloc(strlen({self.c_name}) + 1);",
+                f"if (!{member})",
+                "return;",
+                f"{presence} = strlen({self.c_name});",
                 f'memcpy({member}, {self.c_name}, {presence});',
                 f'{member}[{presence}] = 0;']
 
@@ -582,15 +586,19 @@ class TypeBinary(Type):
 
     def _attr_get(self, ri, var):
         len_mem = var + '->_len.' + self.c_name
-        return [f"{len_mem} = len;",
-                f"{var}->{self.c_name} = malloc(len);",
+        return [f"{var}->{self.c_name} = malloc(len);",
+                f"if (!{var}->{self.c_name})",
+                "return YNL_PARSE_CB_ERROR;",
+                f"{len_mem} = len;",
                 f"memcpy({var}->{self.c_name}, ynl_attr_data(attr), len);"], \
                ['len = ynl_attr_data_len(attr);'], \
                ['unsigned int len;']
 
     def _setter_lines(self, ri, member, presence):
-        return [f"{presence} = len;",
-                f"{member} = malloc({presence});",
+        return [f"{member} = malloc(len);",
+                f"if (!{member})",
+                "return;",
+                f"{presence} = len;",
                 f'memcpy({member}, {self.c_name}, {presence});']
 
 
@@ -601,11 +609,13 @@ class TypeBinaryStruct(TypeBinary):
     def _attr_get(self, ri, var):
         struct_sz = 'sizeof(struct ' + c_lower(self.get("struct")) + ')'
         len_mem = var + '->_' + self.presence_type() + '.' + self.c_name
-        return [f"{len_mem} = len;",
-                f"if (len < {struct_sz})",
+        return [f"if (len < {struct_sz})",
                 f"{var}->{self.c_name} = calloc(1, {struct_sz});",
                 "else",
                 f"{var}->{self.c_name} = malloc(len);",
+                f"if (!{var}->{self.c_name})",
+                "return YNL_PARSE_CB_ERROR;",
+                f"{len_mem} = len;",
                 f"memcpy({var}->{self.c_name}, ynl_attr_data(attr), len);"], \
                ['len = ynl_attr_data_len(attr);'], \
                ['unsigned int len;']
@@ -631,18 +641,21 @@ class TypeBinaryScalarArray(TypeBinary):
 
     def _attr_get(self, ri, var):
         len_mem = var + '->_count.' + self.c_name
-        return [f"{len_mem} = len / sizeof(__{self.get('sub-type')});",
-                f"len = {len_mem} * sizeof(__{self.get('sub-type')});",
+        return [f"len = (len / sizeof(__{self.get('sub-type')})) * sizeof(__{self.get('sub-type')});",
                 f"{var}->{self.c_name} = malloc(len);",
+                f"if (!{var}->{self.c_name})",
+                "return YNL_PARSE_CB_ERROR;",
+                f"{len_mem} = len / sizeof(__{self.get('sub-type')});",
                 f"memcpy({var}->{self.c_name}, ynl_attr_data(attr), len);"], \
                ['len = ynl_attr_data_len(attr);'], \
                ['unsigned int len;']
 
     def _setter_lines(self, ri, member, presence):
-        return [f"{presence} = count;",
-                f"count *= sizeof(__{self.get('sub-type')});",
-                f"{member} = malloc(count);",
-                f'memcpy({member}, {self.c_name}, count);']
+        return [f"{member} = malloc(count * sizeof(__{self.get('sub-type')}));",
+                f"if (!{member})",
+                "return;",
+                f"{presence} = count;",
+                f'memcpy({member}, {self.c_name}, count * sizeof(__{self.get("sub-type")}));']
 
 
 class TypeBitfield32(Type):
@@ -2227,6 +2240,8 @@ def _multi_parse(ri, struct, init_lines, local_vars):
 
         ri.cw.block_start(line=f"if (n_{aspec.c_name})")
         ri.cw.p(f"dst->{aspec.c_name} = calloc(n_{aspec.c_name}, sizeof(*dst->{aspec.c_name}));")
+        ri.cw.p(f"if (!dst->{aspec.c_name})")
+        ri.cw.p("return YNL_PARSE_CB_ERROR;")
         ri.cw.p(f"dst->_count.{aspec.c_name} = n_{aspec.c_name};")
         ri.cw.p('i = 0;')
         if 'nested-attributes' in aspec:
@@ -2252,6 +2267,8 @@ def _multi_parse(ri, struct, init_lines, local_vars):
         aspec = struct[arg]
         ri.cw.block_start(line=f"if (n_{aspec.c_name})")
         ri.cw.p(f"dst->{aspec.c_name} = calloc(n_{aspec.c_name}, sizeof(*dst->{aspec.c_name}));")
+        ri.cw.p(f"if (!dst->{aspec.c_name})")
+        ri.cw.p("return YNL_PARSE_CB_ERROR;")
         ri.cw.p(f"dst->_count.{aspec.c_name} = n_{aspec.c_name};")
         ri.cw.p('i = 0;')
         if 'nested-attributes' in aspec:
@@ -2275,6 +2292,8 @@ def _multi_parse(ri, struct, init_lines, local_vars):
             ri.cw.nl()
             ri.cw.p('len = strnlen(ynl_attr_get_str(attr), ynl_attr_data_len(attr));')
             ri.cw.p(f'dst->{aspec.c_name}[i] = malloc(sizeof(struct ynl_string) + len + 1);')
+            ri.cw.p(f"if (!dst->{aspec.c_name}[i])")
+            ri.cw.p("return YNL_PARSE_CB_ERROR;")
             ri.cw.p(f"dst->{aspec.c_name}[i]->len = len;")
             ri.cw.p(f"memcpy(dst->{aspec.c_name}[i]->str, ynl_attr_get_str(attr), len);")
             ri.cw.p(f"dst->{aspec.c_name}[i]->str[len] = 0;")
@@ -2434,6 +2453,8 @@ def print_req(ri):
 
     if 'reply' in ri.op[ri.op_mode]:
         ri.cw.p('rsp = calloc(1, sizeof(*rsp));')
+        ri.cw.p('if (!rsp)')
+        ri.cw.p(f'return {ret_err};')
         ri.cw.p('yrs.yarg.data = rsp;')
         ri.cw.p(f"yrs.cb = {op_prefix(ri, 'reply')}_parse;")
         if ri.op.value is not None:
-- 
2.55.0.571.g244d577d93-goog
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.