[PATCH net-next 0/2] tools: ynl: update NULL pointer handling in generated code

[PATCH net-next 0/2] tools: ynl: update NULL pointer handling in generated code

[Thaison Phan]

This series fixes potential NULL pointer dereferences in YNL-generated C
code during dump list freeing and memory allocation parsing/setters.

Testing:
- Regenerated all c headers/sources under tools/net/ynl/generated/
- Built libynl, ynltool, and test binaries without errors or warnings
- Ran make target run_tests in tools/net/ynl/tests successfully
- Ran tools/testing/selftests/drivers/net/psp.py in QEMU x86_64
  successfully

Thaison Phan (2):
  tools: ynl: check for null ptr on dump free
  tools: ynl: check malloc/calloc fails in generated code

 tools/net/ynl/pyynl/ynl_gen_c.py | 55 ++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 17 deletions(-)

-- 
2.55.0.571.g244d577d93-goog

[PATCH net-next 1/2] tools: ynl: check for null ptr on dump free

[Thaison Phan]

Static analysis detected code paths where freeing a dump list after early
errors when creating the corresponding dump list like in ynl_exec_dump()
can result in a null pointer dereference since the first node in the
ynl_dump_state would still be zero initialized. To prevent this potential
problem updated the ynl c generation script to check for a NULL pointer
before continuing to free the nodes in a dump list.

Signed-off-by: Thaison Phan <thaisonphan@google.com>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index cdc3646f2642..2fd68c738075 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -2747,7 +2747,7 @@ def print_dump_type_free(ri):
     ri.cw.block_start()
     ri.cw.p(f"{sub_type} *next = rsp;")
     ri.cw.nl()
-    ri.cw.block_start(line='while ((void *)next != YNL_LIST_END)')
+    ri.cw.block_start(line='while (next && (void *)next != YNL_LIST_END)')
     _free_type_members_iter(ri, ri.struct['reply'])
     ri.cw.p('rsp = next;')
     ri.cw.p('next = rsp->next;')
-- 
2.55.0.571.g244d577d93-goog

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

[Thaison Phan]

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 <thaisonphan@google.com>
---
 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

Re: [PATCH net-next 1/2] tools: ynl: check for null ptr on dump free

[Jakub Kicinski]

On Mon,  3 Aug 2026 20:16:51 +0000 Thaison Phan wrote:
> Static analysis

What's the tool? I guess we can do this but I'd factor it out to 
a separate check at the start of the function. Otherwise it reads
as if there could be a NULL on the list itself.

FWIW NULL is not an empty list, it's a dump error. So the static
tool / LLM is definitely wrong. But I guess free(NULL) is supposed
to be a noop in general so whatever.

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

[Jakub Kicinski]

On Mon,  3 Aug 2026 20:16:52 +0000 Thaison Phan wrote:
>      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});']

Silently eating errors in setters does not seem great, better to crash
and make it clear than things are not working than have a silently
misconfigured system?

AI also points out that setters are preceded by a free (in case we're
replacing existing attr, multiple sets on a single attr are allowed)
So if alloc fails we'll end up with NULL ptr and len from previous
allocation.

Maybe tackle the setters in a separate patch, and keep patch 2 scoped
to the easier case?
-- 
pw-bot: cr