[PATCH libftnl 00/12] more misc LLM fixes

[PATCH libftnl 00/12] more misc LLM fixes

[Florian Westphal]

Another round of LLM findings.  These commits are all unrelated,
except that they are LLM findigs with no observed bug reports.

1) Check mnl_nlmsg_batch_start() for a NULL return. Avoid potential NULL
pointer dereference in src/common.

2) Fix uninitialised attr arrays by zero-initialising tb[] arrays.

3) Move several MNL_STRING attribute types to MNL_NUL_STRING so
mnl_attr_validate enforces NUL termination. Harden future
netlink-attribute parsing against non-kernel sources that may omit the
terminator.

4) Validate NFTA_EXPR_NAME parse success. Upgrade string validation to
NUL_STRING, since the kernel always appends a trailing NUL.

5) Forbid re-setting or clearing the object type field to prevent
inconsistent state and segfaults. Check the correct subtype before freeing
to guard against a latent union overlap crash.

6) Drop trailing semicolon from the xfree macro. Callers already append their
own ;, so the macro's is redundant.

7) Remove the obsolete set_elem member from the ruleset union. Its last
consumer (the removed json interface) no longer exists, and the shared
nftnl_set_free path masked a latent type-confusion bug.

8) Clear field_len array before copying to avoid reading stale values from a
prior call. Fix a bug where shrinking the array on a second invocation
leaks leftover field counts.

9) Guard the s->expr_list traversal against the empty-list case to avoid
returning a pointer into the middle of nftnl_set. Initialize data_len to 0.

10) Fix use-after-free by calling list_del() before freeing expressions in
set_elem. Prevents dangling next/prev pointers from corrupting a
subsequent list_add on the same list.

11) Clamp buf->end to the actually copied size in udata_buf_put. Prevent
nftnl_udata_buf_space() from observing an end pointer past the buffer.

12) Remove all remaining MNL_STRING users, switch to MNL_NUL_STRING and
mnl_attr_put_strz. Enforce NUL-termination checking on all string
attributes to prevent illegal strlen/strdup on unchecked data.

Florian Westphal (12):
  src: common: mnl_nlmsg_batch_start may return NULL
  src: fix uninitialised attr arrays
  src: move several MNL_STRING types to MNL_NUL_STRING
  src: expr: make sure NFTA_EXPR_NAME was parsed successfuly
  src: object: don't permit to re-set or clear object type field
  include: drop semicolon at end of xfree macro
  src: ruleset: remove set_elem member from union
  src: set: clear field_len array before copying
  src: set: check for empty list explicitly
  src: set_elem: list_del before freeing
  src: udata: clamp buf->end to actally copied size
  src: remove all remaining MNL_STRING users

 examples/nft-compat-get.c |  2 +-
 include/utils.h           |  2 +-
 src/chain.c               |  4 ++--
 src/common.c              |  4 ++++
 src/expr.c                |  5 ++++-
 src/expr/data_reg.c       |  4 ++--
 src/expr/dynset.c         |  2 +-
 src/expr/flow_offload.c   |  2 +-
 src/expr/log.c            |  2 +-
 src/expr/lookup.c         |  2 +-
 src/expr/objref.c         |  6 +++---
 src/flowtable.c           |  2 +-
 src/obj/ct_helper.c       |  4 ++--
 src/obj/secmark.c         |  4 ++--
 src/object.c              | 19 +++++++++++++++----
 src/rule.c                |  2 +-
 src/ruleset.c             |  3 +--
 src/set.c                 | 13 +++++++------
 src/set_elem.c            | 10 +++++++---
 src/table.c               |  2 +-
 src/trace.c               |  6 +++---
 src/udata.c               |  5 ++++-
 src/utils.c               |  5 +++--
 23 files changed, 68 insertions(+), 42 deletions(-)

-- 
2.55.0

[PATCH libftnl 01/12] src: common: mnl_nlmsg_batch_start may return NULL

[Florian Westphal]

LLM spots that we did not check for a NULL return value from
mnl_nlmsg_batch_start().

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/common.c b/src/common.c
index c92448393b20..0f2745c686dc 100644
--- a/src/common.c
+++ b/src/common.c
@@ -120,6 +120,10 @@ int nftnl_batch_is_supported(void)
 	}
 
 	b = mnl_nlmsg_batch_start(buf, sizeof(buf));
+	if (!b) {
+		mnl_socket_close(nl);
+		return -1;
+	}
 
 	nftnl_batch_begin(mnl_nlmsg_batch_current(b), seq++);
 	mnl_nlmsg_batch_next(b);
-- 
2.55.0

[PATCH libftnl 02/12] src: fix uninitialised attr arrays

[Florian Westphal]

LLM finding: tb[] arrays must be zero-initialised.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/expr/data_reg.c | 2 +-
 src/trace.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c
index d01e0f7d9969..0a1c0bdee6c2 100644
--- a/src/expr/data_reg.c
+++ b/src/expr/data_reg.c
@@ -166,7 +166,7 @@ static int nftnl_verdict_parse_cb(const struct nlattr *attr, void *data)
 static int
 nftnl_parse_verdict(union nftnl_data_reg *data, const struct nlattr *attr, int *type)
 {
-	struct nlattr *tb[NFTA_VERDICT_MAX+1];
+	struct nlattr *tb[NFTA_VERDICT_MAX+1] = {};
 
 	if (mnl_attr_parse_nested(attr, nftnl_verdict_parse_cb, tb) < 0)
 		return -1;
diff --git a/src/trace.c b/src/trace.c
index 5dc4cfabc4a8..61a94a307fee 100644
--- a/src/trace.c
+++ b/src/trace.c
@@ -320,7 +320,7 @@ static int nftnl_trace_parse_verdict_cb(const struct nlattr *attr, void *data)
 static int nftnl_trace_parse_verdict(const struct nlattr *attr,
 				     struct nftnl_trace *t)
 {
-	struct nlattr *tb[NFTA_VERDICT_MAX+1];
+	struct nlattr *tb[NFTA_VERDICT_MAX+1] = {};
 
 	if (mnl_attr_parse_nested(attr, nftnl_trace_parse_verdict_cb, tb) < 0)
 		return -1;
-- 
2.55.0

[PATCH libftnl 03/12] src: move several MNL_STRING types to MNL_NUL_STRING

[Florian Westphal]

LLM finding:
mnl_attr_get_str() returns a pointer into a fixed-length TLV payload with
no guaranteed NUL terminator.

This is the classic 'NLA_STRING' vs. 'NUL_STRING'.
libmnl only checks for terminating NUL when asked to validate a
NUL_STRING attribute.  As we parse nf_tables data coming from kernel,
we can actually rely on the kernel including the terminating NUL
character for us, even for NLA_STRING.

Just switch those affected attributes (strdup() after
mnl_attr_validate(... MNL_STRING) to MNL_NUL_STRING.

This doesn't break any existing users, closes the finding and
hardens future users that might parse netlink attributes coming
from a different source than the kernel.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/chain.c         | 2 +-
 src/expr/data_reg.c | 2 +-
 src/set.c           | 2 +-
 src/utils.c         | 5 +++--
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/chain.c b/src/chain.c
index 13b0231ee345..c72505c22e17 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -613,7 +613,7 @@ static int nftnl_chain_parse_hook_cb(const struct nlattr *attr, void *data)
 			abi_breakage();
 		break;
 	case NFTA_HOOK_DEV:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	}
diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c
index 0a1c0bdee6c2..e38e493f0592 100644
--- a/src/expr/data_reg.c
+++ b/src/expr/data_reg.c
@@ -155,7 +155,7 @@ static int nftnl_verdict_parse_cb(const struct nlattr *attr, void *data)
 			abi_breakage();
 		break;
 	case NFTA_VERDICT_CHAIN:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	}
diff --git a/src/set.c b/src/set.c
index f7b0526253ff..22fd4a839f80 100644
--- a/src/set.c
+++ b/src/set.c
@@ -491,7 +491,7 @@ static int nftnl_set_parse_attr_cb(const struct nlattr *attr, void *data)
 	case NFTA_SET_TABLE:
 	case NFTA_SET_NAME:
 	case NFTA_SET_TYPE:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_SET_HANDLE:
diff --git a/src/utils.c b/src/utils.c
index f7d035baa3ae..95a252ed44f3 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -200,14 +200,15 @@ void nftnl_attr_put_ifname(struct nlmsghdr *nlh, const char *ifname)
 
 char *nftnl_attr_get_ifname(const struct nlattr *attr)
 {
+	uint16_t dlen = mnl_attr_get_payload_len(attr);
 	const char *dev = mnl_attr_get_str(attr);
 	char buf[IFNAMSIZ];
 
 	switch (mnl_attr_get_type(attr)) {
 	case NFTA_DEVICE_NAME:
-		return strdup(dev);
+		return strndup(dev, dlen);
 	case NFTA_DEVICE_PREFIX:
-		snprintf(buf, IFNAMSIZ, "%s*", dev);
+		snprintf(buf, IFNAMSIZ, "%.*s*", (int)dlen, dev);
 		return strdup(buf);
 	default:
 		return NULL;
-- 
2.55.0

[PATCH libftnl 04/12] src: expr: make sure NFTA_EXPR_NAME was parsed successfuly

[Florian Westphal]

LLM finding. Not a bug atm, the kernel always includes this.
While at it, upgrade to NUL_STRING.  Kernel always sets a trailing
NUL even for NLA_STRING, so we can enforce this.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/expr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/expr.c b/src/expr.c
index 6462119e3eb2..980b317ff6db 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -251,7 +251,7 @@ static int nftnl_rule_parse_expr_cb(const struct nlattr *attr, void *data)
 
 	switch (type) {
 	case NFTA_EXPR_NAME:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_EXPR_DATA:
@@ -272,6 +272,9 @@ struct nftnl_expr *nftnl_expr_parse(struct nlattr *attr)
 	if (mnl_attr_parse_nested(attr, nftnl_rule_parse_expr_cb, tb) < 0)
 		goto err1;
 
+	if (!tb[NFTA_EXPR_NAME])
+		goto err1;
+
 	expr = nftnl_expr_alloc(mnl_attr_get_str(tb[NFTA_EXPR_NAME]));
 	if (expr == NULL)
 		goto err1;
-- 
2.55.0

[PATCH libftnl 05/12] src: object: don't permit to re-set or clear object type field

[Florian Westphal]

LLM finding:
If obj->ops was previously valid (object already typed) and the caller passes
an invalid type, obj->ops is nulled. The object is left in an inconsistent state.
flags still has the NFTNL_OBJ_TYPE bit set.

Subsequent default-path sets will fail.
get/build/output calls that dereference obj->ops will segfault.

Don't permit identity change.

The object type uses a set of common flags plus a set of type
specific flags.  The latter overlaps.

Permitting identity switch is just asking for trouble.
Moreover, there is a latent crash bug:

If a future type introduces a flag overlapping with
NFTNL_OBJ_TUNNEL_OPTS, we free a pointer inside union.

Check we have the correct subtype before freeing.
This is NOT a bug at this time, nftables neither switches
ops nor does it call nftnl_obj_unset().

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/object.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/object.c b/src/object.c
index 1198be6965d9..d35e050b9d83 100644
--- a/src/object.c
+++ b/src/object.c
@@ -56,7 +56,8 @@ void nftnl_obj_free(const struct nftnl_obj *obj)
 		xfree(obj->name);
 	if (obj->flags & (1 << NFTNL_OBJ_USERDATA))
 		nftnl_mem_blob_free(&obj->user_data);
-	if (obj->flags & (1 << NFTNL_OBJ_TUNNEL_OPTS)) {
+	if (obj->ops && obj->ops->type == NFT_OBJECT_TUNNEL &&
+	    obj->flags & (1 << NFTNL_OBJ_TUNNEL_OPTS)) {
 		nftnl_tunnel_opts_free(obj->data.tunnel.tun_opts);
 		xfree(obj->data.tunnel.tun_opts);
 	}
@@ -86,13 +87,21 @@ void nftnl_obj_unset(struct nftnl_obj *obj, uint16_t attr)
 	case NFTNL_OBJ_USERDATA:
 		nftnl_mem_blob_free(&obj->user_data);
 		break;
-	case NFTNL_OBJ_TYPE:
 	case NFTNL_OBJ_FAMILY:
 	case NFTNL_OBJ_USE:
 	case NFTNL_OBJ_HANDLE:
 		break;
+	case NFTNL_OBJ_TYPE:
+		/* don't allow to morph one object type
+		 * to another.
+		 */
+		return;
 	default:
-		break;
+		/* object type specific. Clearing e.g.
+		 * NFTNL_OBJ_TUNNEL_OPTS results
+		 * in memory leak.
+		 */
+		return;
 	}
 
 	obj->flags &= ~(1 << attr);
@@ -121,6 +130,8 @@ int nftnl_obj_set_data(struct nftnl_obj *obj, uint16_t attr,
 		return nftnl_set_str_attr(&obj->name, &obj->flags,
 					  attr, data, data_len);
 	case NFTNL_OBJ_TYPE:
+		if (obj->ops)	/* don't permit object type change */
+			return -1;
 		obj->ops = nftnl_obj_ops_lookup(*((uint32_t *)data));
 		if (!obj->ops)
 			return -1;
-- 
2.55.0

[PATCH libftnl 06/12] include: drop semicolon at end of xfree macro

[Florian Westphal]

... not required, callers do 'xfree(x);' already.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/utils.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/utils.h b/include/utils.h
index f1bf3f6ddd41..6b87c5edac24 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -19,7 +19,7 @@
 
 #define __noreturn	__attribute__((__noreturn__))
 
-#define xfree(ptr)	free((void *)ptr);
+#define xfree(ptr)	free((void *)ptr)
 
 #define div_round_up(n, d)	(((n) + (d) - 1) / (d))
 
-- 
2.55.0

[PATCH libftnl 07/12] src: ruleset: remove set_elem member from union

[Florian Westphal]

LLM flags this bug:
case NFTNL_RULESET_SET:
case NFTNL_RULESET_SET_ELEMS:
     nftnl_set_free(ctx->set);
     break;

... because it hints at type confusion, SET_ELEMS
should call nftnl_set_elem_free(ctx->set_elem).

However, this entire API is obsolete, nftables doesn't
use this anymore.  The last user of NFTNL_RULESET_SET_ELEMS
went away when the json interface was removed from libnftnl.

Drop the member from the union.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/ruleset.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/ruleset.c b/src/ruleset.c
index ac50aa604f98..e0f46dd1b20d 100644
--- a/src/ruleset.c
+++ b/src/ruleset.c
@@ -36,7 +36,6 @@ struct nftnl_parse_ctx {
 		struct nftnl_chain	*chain;
 		struct nftnl_rule		*rule;
 		struct nftnl_set		*set;
-		struct nftnl_set_elem	*set_elem;
 	};
 	void *data;
 
@@ -158,9 +157,9 @@ void nftnl_ruleset_ctx_free(const struct nftnl_parse_ctx *ctx)
 		nftnl_rule_free(ctx->rule);
 		break;
 	case NFTNL_RULESET_SET:
-	case NFTNL_RULESET_SET_ELEMS:
 		nftnl_set_free(ctx->set);
 		break;
+	case NFTNL_RULESET_SET_ELEMS:
 	case NFTNL_RULESET_RULESET:
 	case NFTNL_RULESET_UNSPEC:
 		break;
-- 
2.55.0

[PATCH libftnl 08/12] src: set: clear field_len array before copying

[Florian Westphal]

LLM flagged:
      if (data_len > sizeof(s->desc.field_len))
          return -1;

      memcpy(&s->desc.field_len, data, data_len);
      for (s->desc.field_count = 0;
           s->desc.field_count < NFT_REG32_COUNT;
           s->desc.field_count++) {
          if (!s->desc.field_len[s->desc.field_count]) // here

i.e. it relies on the inital zero state.

If for any reason a 2nd call is made that shinks field_len
array, the loop would read stale count from previous
incarnation.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/set.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/set.c b/src/set.c
index 22fd4a839f80..fc50f3f28055 100644
--- a/src/set.c
+++ b/src/set.c
@@ -188,6 +188,7 @@ int nftnl_set_set_data(struct nftnl_set *s, uint16_t attr, const void *data,
 		if (data_len > sizeof(s->desc.field_len))
 			return -1;
 
+		memset(&s->desc.field_len, 0, sizeof(s->desc.field_len));
 		memcpy(&s->desc.field_len, data, data_len);
 		for (s->desc.field_count = 0;
 		     s->desc.field_count < NFT_REG32_COUNT;
-- 
2.55.0

Re: [PATCH libftnl 08/12] src: set: clear field_len array before copying

[Phil Sutter]

On Wed, Oct 28, 2026 at 03:56:28PM +0100, Florian Westphal wrote:
> LLM flagged:
>       if (data_len > sizeof(s->desc.field_len))
>           return -1;
> 
>       memcpy(&s->desc.field_len, data, data_len);
>       for (s->desc.field_count = 0;
>            s->desc.field_count < NFT_REG32_COUNT;
>            s->desc.field_count++) {
>           if (!s->desc.field_len[s->desc.field_count]) // here
> 
> i.e. it relies on the inital zero state.
> 
> If for any reason a 2nd call is made that shinks field_len
                                            ~~~~~~
*shrinks

> array, the loop would read stale count from previous
> incarnation.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/set.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/set.c b/src/set.c
> index 22fd4a839f80..fc50f3f28055 100644
> --- a/src/set.c
> +++ b/src/set.c
> @@ -188,6 +188,7 @@ int nftnl_set_set_data(struct nftnl_set *s, uint16_t attr, const void *data,
>  		if (data_len > sizeof(s->desc.field_len))
>  			return -1;
>  
> +		memset(&s->desc.field_len, 0, sizeof(s->desc.field_len));
>  		memcpy(&s->desc.field_len, data, data_len);
>  		for (s->desc.field_count = 0;
>  		     s->desc.field_count < NFT_REG32_COUNT;
> -- 
> 2.55.0
> 
> 
> 

[PATCH libftnl 09/12] src: set: check for empty list explicitly

[Florian Westphal]

LLM finding:
When s->expr_list is empty the for-init assigns
expr = container_of(&s->expr_list, nftnl_expr, head).

Since `head` is at offset 0 in nftnl_expr, this yields
expr == (nftnl_expr*)&s->expr_list: a pointer into the middle of the
nftnl_set struct.

The function returns this bogus pointer.
Also set data_len to 0; normally users would use the _str getter
which ignores the data_len anyway, but its better to initialise it
to a known value.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/set.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/set.c b/src/set.c
index fc50f3f28055..f06223eaed86 100644
--- a/src/set.c
+++ b/src/set.c
@@ -253,8 +253,6 @@ EXPORT_SYMBOL(nftnl_set_get_data);
 const void *nftnl_set_get_data(const struct nftnl_set *s, uint16_t attr,
 			       uint32_t *data_len)
 {
-	struct nftnl_expr *expr;
-
 	if (!(s->flags & (1 << attr)))
 		return NULL;
 
@@ -313,9 +311,11 @@ const void *nftnl_set_get_data(const struct nftnl_set *s, uint16_t attr,
 	case NFTNL_SET_USERDATA:
 		return nftnl_mem_blob_get(&s->user_data, data_len);
 	case NFTNL_SET_EXPR:
-		list_for_each_entry(expr, &s->expr_list, head)
-			break;
-		return expr;
+		*data_len = 0;
+		if (list_empty(&s->expr_list))
+			return NULL;
+
+		return list_entry(s->expr_list.next, struct nftnl_expr, head);
 	}
 	return NULL;
 }
-- 
2.55.0

[PATCH libftnl 10/12] src: set_elem: list_del before freeing

[Florian Westphal]

LLM finding:
nftnl_expr_free() calls xfree(expr).
It never calls list_del(&expr->head).

After the loop, expr_list.next/.prev point into freed heap memory.
Next list_add results in UaF.

Looks like noone does such re-use, but as the API allows it, make
this safe.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/set_elem.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/set_elem.c b/src/set_elem.c
index 3504a2306e47..0170d0ccefd1 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -88,8 +88,10 @@ void nftnl_set_elem_unset(struct nftnl_set_elem *s, uint16_t attr)
 		break;
 	case NFTNL_SET_ELEM_EXPR:
 	case NFTNL_SET_ELEM_EXPRESSIONS:
-		list_for_each_entry_safe(expr, tmp, &s->expr_list, head)
+		list_for_each_entry_safe(expr, tmp, &s->expr_list, head) {
+			list_del(&expr->head);
 			nftnl_expr_free(expr);
+		}
 		break;
 	case NFTNL_SET_ELEM_OBJREF:
 		xfree(s->objref);
@@ -167,8 +169,10 @@ __nftnl_set_elem_set(struct nftnl_set_elem *s, uint16_t attr, const void *data,
 			return -1;
 		break;
 	case NFTNL_SET_ELEM_EXPR:
-		list_for_each_entry_safe(expr, tmp, &s->expr_list, head)
+		list_for_each_entry_safe(expr, tmp, &s->expr_list, head) {
+			list_del(&expr->head);
 			nftnl_expr_free(expr);
+		}
 
 		expr = (void *)data;
 		list_add(&expr->head, &s->expr_list);
-- 
2.55.0

[PATCH libftnl 11/12] src: udata: clamp buf->end to actally copied size

[Florian Westphal]

LLM finding:

nftnl_udata_buf_space() doesn't tolerate buf->end moving
past the size.

Clamp the end pointer in udata_buf_put so that it matches
what was actually copied.  After this, nftnl_udata_buf_space()
will return 0 if the space was exhausted.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/udata.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/udata.c b/src/udata.c
index a1956571ef5f..11de73402569 100644
--- a/src/udata.c
+++ b/src/udata.c
@@ -53,7 +53,10 @@ EXPORT_SYMBOL(nftnl_udata_buf_put);
 void nftnl_udata_buf_put(struct nftnl_udata_buf *buf, const void *data,
 			 uint32_t len)
 {
-	memcpy(buf->data, data, len <= buf->size ? len : buf->size);
+	if (len > buf->size)
+		len = buf->size;
+
+	memcpy(buf->data, data, len);
 	buf->end = buf->data + len;
 }
 
-- 
2.55.0

[PATCH libftnl 12/12] src: remove all remaining MNL_STRING users

[Florian Westphal]

Switch everything over to MNL_NUL_STRING and mnl_attr_put_strz.

The kernel always emits NLA_STRING attributes with the trailing
0 (strlen() + 1), so this is safe for kernel-originating data.

For non-kernel-originating data, the lack of the NUL check means
greater care needs to be taken on parser side, plain strlen, strdup,
etc is illegal, a known source of bugs.

We already used mnl_attr_put_strz() almost everywhere, this
converts the rest.

No bug report behing this, its related to other LLM findings where
we did strlen/strdup on unchecked MNL_STRING attribute, see
"src: move several MNL_STRING types to MNL_NUL_STRING".

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 examples/nft-compat-get.c | 2 +-
 src/chain.c               | 2 +-
 src/expr/dynset.c         | 2 +-
 src/expr/flow_offload.c   | 2 +-
 src/expr/log.c            | 2 +-
 src/expr/lookup.c         | 2 +-
 src/expr/objref.c         | 6 +++---
 src/flowtable.c           | 2 +-
 src/obj/ct_helper.c       | 4 ++--
 src/obj/secmark.c         | 4 ++--
 src/object.c              | 2 +-
 src/rule.c                | 2 +-
 src/set_elem.c            | 2 +-
 src/table.c               | 2 +-
 src/trace.c               | 4 ++--
 15 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/examples/nft-compat-get.c b/examples/nft-compat-get.c
index fd308db9c460..8d7a2dbc54cc 100644
--- a/examples/nft-compat-get.c
+++ b/examples/nft-compat-get.c
@@ -25,7 +25,7 @@ static int data_attr_cb(const struct nlattr *attr, void *data)
 
 	switch(type) {
 	case NFTA_COMPAT_NAME:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0) {
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0) {
 			perror("mnl_attr_validate");
 			return MNL_CB_ERROR;
 		}
diff --git a/src/chain.c b/src/chain.c
index c72505c22e17..34fa295552a1 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -530,7 +530,7 @@ static int nftnl_chain_parse_attr_cb(const struct nlattr *attr, void *data)
 	case NFTA_CHAIN_NAME:
 	case NFTA_CHAIN_TABLE:
 	case NFTA_CHAIN_TYPE:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_CHAIN_HOOK:
diff --git a/src/expr/dynset.c b/src/expr/dynset.c
index f513b3736b43..1f99a82efd30 100644
--- a/src/expr/dynset.c
+++ b/src/expr/dynset.c
@@ -131,7 +131,7 @@ static int nftnl_expr_dynset_cb(const struct nlattr *attr, void *data)
 			abi_breakage();
 		break;
 	case NFTA_DYNSET_SET_NAME:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_DYNSET_EXPR:
diff --git a/src/expr/flow_offload.c b/src/expr/flow_offload.c
index f9a999bd4843..1b6724b64cb1 100644
--- a/src/expr/flow_offload.c
+++ b/src/expr/flow_offload.c
@@ -53,7 +53,7 @@ static int nftnl_expr_flow_cb(const struct nlattr *attr, void *data)
 
 	switch(type) {
 	case NFTA_FLOW_TABLE_NAME:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	}
diff --git a/src/expr/log.c b/src/expr/log.c
index f8456bcce4bf..3af15e9232b3 100644
--- a/src/expr/log.c
+++ b/src/expr/log.c
@@ -100,7 +100,7 @@ static int nftnl_expr_log_cb(const struct nlattr *attr, void *data)
 
 	switch(type) {
 	case NFTA_LOG_PREFIX:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_LOG_GROUP:
diff --git a/src/expr/lookup.c b/src/expr/lookup.c
index c6a36a68d0b0..7d1ff1823071 100644
--- a/src/expr/lookup.c
+++ b/src/expr/lookup.c
@@ -96,7 +96,7 @@ static int nftnl_expr_lookup_cb(const struct nlattr *attr, void *data)
 			abi_breakage();
 		break;
 	case NFTA_LOOKUP_SET:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	}
diff --git a/src/expr/objref.c b/src/expr/objref.c
index 481103e7883c..415444e95d6c 100644
--- a/src/expr/objref.c
+++ b/src/expr/objref.c
@@ -98,7 +98,7 @@ static int nftnl_expr_objref_cb(const struct nlattr *attr, void *data)
 		break;
 	case NFTA_OBJREF_IMM_NAME:
 	case NFTA_OBJREF_SET_NAME:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_OBJREF_SET_SREG:
@@ -121,12 +121,12 @@ static void nftnl_expr_objref_build(struct nlmsghdr *nlh,
 		mnl_attr_put_u32(nlh, NFTA_OBJREF_IMM_TYPE,
 				 htonl(objref->imm.type));
 	if (e->flags & (1 << NFTNL_EXPR_OBJREF_IMM_NAME))
-		mnl_attr_put_str(nlh, NFTA_OBJREF_IMM_NAME, objref->imm.name);
+		mnl_attr_put_strz(nlh, NFTA_OBJREF_IMM_NAME, objref->imm.name);
 	if (e->flags & (1 << NFTNL_EXPR_OBJREF_SET_SREG))
 		mnl_attr_put_u32(nlh, NFTA_OBJREF_SET_SREG,
 				 htonl(objref->set.sreg));
 	if (e->flags & (1 << NFTNL_EXPR_OBJREF_SET_NAME))
-		mnl_attr_put_str(nlh, NFTA_OBJREF_SET_NAME, objref->set.name);
+		mnl_attr_put_strz(nlh, NFTA_OBJREF_SET_NAME, objref->set.name);
 	if (e->flags & (1 << NFTNL_EXPR_OBJREF_SET_ID))
 		mnl_attr_put_u32(nlh, NFTA_OBJREF_SET_ID,
 				 htonl(objref->set.id));
diff --git a/src/flowtable.c b/src/flowtable.c
index 2adb478ee94a..fa9cf5a4100b 100644
--- a/src/flowtable.c
+++ b/src/flowtable.c
@@ -326,7 +326,7 @@ static int nftnl_flowtable_parse_attr_cb(const struct nlattr *attr, void *data)
 	switch(type) {
 	case NFTA_FLOWTABLE_NAME:
 	case NFTA_FLOWTABLE_TABLE:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_FLOWTABLE_HOOK:
diff --git a/src/obj/ct_helper.c b/src/obj/ct_helper.c
index 6e16f083f2f2..f11595a394b6 100644
--- a/src/obj/ct_helper.c
+++ b/src/obj/ct_helper.c
@@ -67,7 +67,7 @@ static int nftnl_obj_ct_helper_cb(const struct nlattr *attr, void *data)
 
 	switch (type) {
 	case NFTA_CT_HELPER_NAME:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		if (mnl_attr_get_payload_len(attr) >= sizeof(helper->name))
 			abi_breakage();
@@ -92,7 +92,7 @@ nftnl_obj_ct_helper_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 	struct nftnl_obj_ct_helper *helper = nftnl_obj_data(e);
 
 	if (e->flags & (1 << NFTNL_OBJ_CT_HELPER_NAME))
-		mnl_attr_put_str(nlh, NFTA_CT_HELPER_NAME, helper->name);
+		mnl_attr_put_strz(nlh, NFTA_CT_HELPER_NAME, helper->name);
 	if (e->flags & (1 << NFTNL_OBJ_CT_HELPER_L3PROTO))
 		mnl_attr_put_u16(nlh, NFTA_CT_HELPER_L3PROTO, htons(helper->l3proto));
 	if (e->flags & (1 << NFTNL_OBJ_CT_HELPER_L4PROTO))
diff --git a/src/obj/secmark.c b/src/obj/secmark.c
index 25b04e206eef..de74d59a5c64 100644
--- a/src/obj/secmark.c
+++ b/src/obj/secmark.c
@@ -54,7 +54,7 @@ static int nftnl_obj_secmark_cb(const struct nlattr *attr, void *data)
 
 	switch(type) {
 	case NFTA_SECMARK_CTX:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		if (mnl_attr_get_payload_len(attr) >= sizeof(secmark->ctx))
 			abi_breakage();
@@ -71,7 +71,7 @@ nftnl_obj_secmark_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 	struct nftnl_obj_secmark *secmark = nftnl_obj_data(e);
 
 	if (e->flags & (1 << NFTNL_OBJ_SECMARK_CTX))
-		mnl_attr_put_str(nlh, NFTA_SECMARK_CTX, secmark->ctx);
+		mnl_attr_put_strz(nlh, NFTA_SECMARK_CTX, secmark->ctx);
 }
 
 static int
diff --git a/src/object.c b/src/object.c
index d35e050b9d83..4f2087752f67 100644
--- a/src/object.c
+++ b/src/object.c
@@ -315,7 +315,7 @@ static int nftnl_obj_parse_attr_cb(const struct nlattr *attr, void *data)
 	switch(type) {
 	case NFTA_OBJ_TABLE:
 	case NFTA_OBJ_NAME:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_OBJ_HANDLE:
diff --git a/src/rule.c b/src/rule.c
index c7bf6f3b14ad..28aa8df57795 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -324,7 +324,7 @@ static int nftnl_rule_parse_attr_cb(const struct nlattr *attr, void *data)
 	switch(type) {
 	case NFTA_RULE_TABLE:
 	case NFTA_RULE_CHAIN:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_RULE_HANDLE:
diff --git a/src/set_elem.c b/src/set_elem.c
index 0170d0ccefd1..14ff6f580fe4 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -595,7 +595,7 @@ nftnl_set_elem_list_parse_attr_cb(const struct nlattr *attr, void *data)
 	switch(type) {
 	case NFTA_SET_ELEM_LIST_TABLE:
 	case NFTA_SET_ELEM_LIST_SET:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_SET_ELEM_LIST_ELEMENTS:
diff --git a/src/table.c b/src/table.c
index 03aee30db9ec..22786879651a 100644
--- a/src/table.c
+++ b/src/table.c
@@ -243,7 +243,7 @@ static int nftnl_table_parse_attr_cb(const struct nlattr *attr, void *data)
 
 	switch(type) {
 	case NFTA_TABLE_NAME:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_TABLE_HANDLE:
diff --git a/src/trace.c b/src/trace.c
index 61a94a307fee..fac1b366c0f4 100644
--- a/src/trace.c
+++ b/src/trace.c
@@ -118,7 +118,7 @@ static int nftnl_trace_parse_attr_cb(const struct nlattr *attr, void *data)
 		break;
 	case NFTA_TRACE_CHAIN:
 	case NFTA_TRACE_TABLE:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		break;
 	case NFTA_TRACE_RULE_HANDLE:
@@ -308,7 +308,7 @@ static int nftnl_trace_parse_verdict_cb(const struct nlattr *attr, void *data)
 		tb[type] = attr;
 		break;
 	case NFTA_VERDICT_CHAIN:
-		if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
 			abi_breakage();
 		tb[type] = attr;
 		break;
-- 
2.55.0

Re: [PATCH libftnl 00/12] more misc LLM fixes

[Phil Sutter]

On Wed, Oct 28, 2026 at 03:56:20PM +0100, Florian Westphal wrote:
> Another round of LLM findings.  These commits are all unrelated,
> except that they are LLM findigs with no observed bug reports.
> 
> 1) Check mnl_nlmsg_batch_start() for a NULL return. Avoid potential NULL
> pointer dereference in src/common.
> 
> 2) Fix uninitialised attr arrays by zero-initialising tb[] arrays.
> 
> 3) Move several MNL_STRING attribute types to MNL_NUL_STRING so
> mnl_attr_validate enforces NUL termination. Harden future
> netlink-attribute parsing against non-kernel sources that may omit the
> terminator.
> 
> 4) Validate NFTA_EXPR_NAME parse success. Upgrade string validation to
> NUL_STRING, since the kernel always appends a trailing NUL.
> 
> 5) Forbid re-setting or clearing the object type field to prevent
> inconsistent state and segfaults. Check the correct subtype before freeing
> to guard against a latent union overlap crash.
> 
> 6) Drop trailing semicolon from the xfree macro. Callers already append their
> own ;, so the macro's is redundant.
> 
> 7) Remove the obsolete set_elem member from the ruleset union. Its last
> consumer (the removed json interface) no longer exists, and the shared
> nftnl_set_free path masked a latent type-confusion bug.
> 
> 8) Clear field_len array before copying to avoid reading stale values from a
> prior call. Fix a bug where shrinking the array on a second invocation
> leaks leftover field counts.
> 
> 9) Guard the s->expr_list traversal against the empty-list case to avoid
> returning a pointer into the middle of nftnl_set. Initialize data_len to 0.
> 
> 10) Fix use-after-free by calling list_del() before freeing expressions in
> set_elem. Prevents dangling next/prev pointers from corrupting a
> subsequent list_add on the same list.
> 
> 11) Clamp buf->end to the actually copied size in udata_buf_put. Prevent
> nftnl_udata_buf_space() from observing an end pointer past the buffer.
> 
> 12) Remove all remaining MNL_STRING users, switch to MNL_NUL_STRING and
> mnl_attr_put_strz. Enforce NUL-termination checking on all string
> attributes to prevent illegal strlen/strdup on unchecked data.
> 
> Florian Westphal (12):
>   src: common: mnl_nlmsg_batch_start may return NULL
>   src: fix uninitialised attr arrays
>   src: move several MNL_STRING types to MNL_NUL_STRING
>   src: expr: make sure NFTA_EXPR_NAME was parsed successfuly
>   src: object: don't permit to re-set or clear object type field
>   include: drop semicolon at end of xfree macro
>   src: ruleset: remove set_elem member from union
>   src: set: clear field_len array before copying
>   src: set: check for empty list explicitly
>   src: set_elem: list_del before freeing
>   src: udata: clamp buf->end to actally copied size
>   src: remove all remaining MNL_STRING users

Reviewed-by: Phil Sutter <phil@nwl.cc>

Thanks, Phil