* [PATCH libftnl 00/12] more misc LLM fixes
@ 2026-10-28 14:56 Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 01/12] src: common: mnl_nlmsg_batch_start may return NULL Florian Westphal
` (11 more replies)
0 siblings, 12 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH libftnl 01/12] src: common: mnl_nlmsg_batch_start may return NULL
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 02/12] src: fix uninitialised attr arrays Florian Westphal
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 02/12] src: fix uninitialised attr arrays
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 01/12] src: common: mnl_nlmsg_batch_start may return NULL Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 03/12] src: move several MNL_STRING types to MNL_NUL_STRING Florian Westphal
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 03/12] src: move several MNL_STRING types to MNL_NUL_STRING
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 01/12] src: common: mnl_nlmsg_batch_start may return NULL Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 02/12] src: fix uninitialised attr arrays Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 04/12] src: expr: make sure NFTA_EXPR_NAME was parsed successfuly Florian Westphal
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 04/12] src: expr: make sure NFTA_EXPR_NAME was parsed successfuly
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (2 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 03/12] src: move several MNL_STRING types to MNL_NUL_STRING Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 05/12] src: object: don't permit to re-set or clear object type field Florian Westphal
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 05/12] src: object: don't permit to re-set or clear object type field
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (3 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 04/12] src: expr: make sure NFTA_EXPR_NAME was parsed successfuly Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 06/12] include: drop semicolon at end of xfree macro Florian Westphal
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 06/12] include: drop semicolon at end of xfree macro
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (4 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 05/12] src: object: don't permit to re-set or clear object type field Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 07/12] src: ruleset: remove set_elem member from union Florian Westphal
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 07/12] src: ruleset: remove set_elem member from union
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (5 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 06/12] include: drop semicolon at end of xfree macro Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 08/12] src: set: clear field_len array before copying Florian Westphal
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 08/12] src: set: clear field_len array before copying
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (6 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 07/12] src: ruleset: remove set_elem member from union Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 09/12] src: set: check for empty list explicitly Florian Westphal
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 09/12] src: set: check for empty list explicitly
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (7 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 08/12] src: set: clear field_len array before copying Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 10/12] src: set_elem: list_del before freeing Florian Westphal
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 10/12] src: set_elem: list_del before freeing
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (8 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 09/12] src: set: check for empty list explicitly Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 11/12] src: udata: clamp buf->end to actally copied size Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 12/12] src: remove all remaining MNL_STRING users Florian Westphal
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 11/12] src: udata: clamp buf->end to actally copied size
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (9 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 10/12] src: set_elem: list_del before freeing Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 12/12] src: remove all remaining MNL_STRING users Florian Westphal
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH libftnl 12/12] src: remove all remaining MNL_STRING users
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
` (10 preceding siblings ...)
2026-10-28 14:56 ` [PATCH libftnl 11/12] src: udata: clamp buf->end to actally copied size Florian Westphal
@ 2026-10-28 14:56 ` Florian Westphal
11 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2026-10-28 14:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-28 14:57 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 14:56 [PATCH libftnl 00/12] more misc LLM fixes Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 01/12] src: common: mnl_nlmsg_batch_start may return NULL Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 02/12] src: fix uninitialised attr arrays Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 03/12] src: move several MNL_STRING types to MNL_NUL_STRING Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 04/12] src: expr: make sure NFTA_EXPR_NAME was parsed successfuly Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 05/12] src: object: don't permit to re-set or clear object type field Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 06/12] include: drop semicolon at end of xfree macro Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 07/12] src: ruleset: remove set_elem member from union Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 08/12] src: set: clear field_len array before copying Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 09/12] src: set: check for empty list explicitly Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 10/12] src: set_elem: list_del before freeing Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 11/12] src: udata: clamp buf->end to actally copied size Florian Westphal
2026-10-28 14:56 ` [PATCH libftnl 12/12] src: remove all remaining MNL_STRING users Florian Westphal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.