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

Generated YNL getter 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 in getters.

Signed-off-by: Thaison Phan <[email protected]>
---
v1 -> v2: Removed updates to allocation checks in setters as those can be
fixed in a separate patch. (Suggested by Jakub Kicinski)

v1:
https://lore.kernel.org/netdev/[email protected]/

 tools/net/ynl/pyynl/ynl_gen_c.py | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 95502dbaec94..2b3483db1b60 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -526,8 +526,10 @@ 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));'], \
@@ -582,8 +584,10 @@ 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;']
@@ -601,11 +605,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,9 +637,11 @@ 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;']
@@ -2227,6 +2235,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 +2262,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 +2287,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 +2448,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.654.g21b8a5bc05-goog