[PATCH net 00/11] netfilter: updates for net

[PATCH net 00/11] netfilter: updates for net

[Florian Westphal]

Hi,

This is a much earlier pull request than usual, due to the large
backlog.  We are aware of several unfixed issues, in particular in
ctnetlink, patches are being worked on.

The following patchset contains Netfilter fixes for *net*:

1) fix a use-after-free in ctnetlink, from Hyunwoo Kim, broken
   since v3.10.
2) add missing netlink range checks in ctnetlink, broken since v2.6
   days.
3) fix content length truncation in sip conntrack helper,
   from Lukas Johannes Möller.  Broken since 2.6.34.
4) Revert a recent patch to add stronger checks for overlapping ranges
   in nf_tables rbtree set type.
   Patch is correct, but several nftables version have a bug (now fixed)
   that trigger the checks incorrectly.
5) Reset mac header before the vlan push to avoid warning splat (and
   make things functional). From Eric Woudstra.
6) Add missing bounds check in H323 conntrack helper, broken since this
   helper was added 20 years ago, from Jenny Guanni Qu.
7) Fix a memory leak in the dynamic set infrastructure, from Pablo Neira
   Ayuso.  Broken since v5.11.
8+9) a few spots failed to purge skbs queued to userspace via nfqueue,
   this causes RCU escape / use-after-free. Also from Pablo. broken
   since v3.4 added the CT target to xtables.
10) Fix undefined behaviour in xt_time, use u32 for a shift-by-31
    operation, not s32, from Jenny Guanni Qu.
11) H323 conntrack helper lacks a check for length variable becoming
    negative after decrement, causes major out-of-bounds read due to
    cast to unsigned size later, also from Jenny.
    Both issues exist since 2.6 days.

Please, pull these changes from:
The following changes since commit 99600f79b28c83c68bae199a3d8e95049a758308:

  mpls: add missing unregister_netdevice_notifier to mpls_init (2026-03-12 19:25:59 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git tags/nf-26-03-13

for you to fetch changes up to f173d0f4c0f689173f8cdac79991043a4a89bf66:

  netfilter: nf_conntrack_h323: check for zero length in DecodeQ931() (2026-03-13 15:31:15 +0100)

----------------------------------------------------------------
netfilter pull request nf-26-03-13

----------------------------------------------------------------
Eric Woudstra (1):
      netfilter: nf_flow_table_ip: reset mac header before vlan push

Florian Westphal (2):
      netfilter: conntrack: add missing netlink policy validations
      netfilter: revert nft_set_rbtree: validate open interval overlap

Hyunwoo Kim (1):
      netfilter: ctnetlink: fix use-after-free in ctnetlink_dump_exp_ct()

Jenny Guanni Qu (3):
      netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case
      netfilter: xt_time: use unsigned int for monthday bit shift
      netfilter: nf_conntrack_h323: check for zero length in DecodeQ931()

Lukas Johannes Möller (1):
      netfilter: nf_conntrack_sip: fix Content-Length u32 truncation in sip_help_tcp()

Pablo Neira Ayuso (3):
      nf_tables: nft_dynset: fix possible stateful expression memleak in error path
      netfilter: nft_ct: drop pending enqueued packets on removal
      netfilter: xt_CT: drop pending enqueued packets on template removal

 include/net/netfilter/nf_tables.h       |  6 +--
 net/netfilter/nf_conntrack_h323_asn1.c  |  4 ++
 net/netfilter/nf_conntrack_netlink.c    | 28 +++++++++-
 net/netfilter/nf_conntrack_proto_sctp.c |  3 +-
 net/netfilter/nf_conntrack_sip.c        |  6 ++-
 net/netfilter/nf_flow_table_ip.c        |  1 +
 net/netfilter/nf_tables_api.c           | 25 +++------
 net/netfilter/nft_ct.c                  |  4 ++
 net/netfilter/nft_dynset.c              | 10 +++-
 net/netfilter/nft_set_rbtree.c          | 71 ++++---------------------
 net/netfilter/xt_CT.c                   |  4 ++
 net/netfilter/xt_time.c                 |  4 +-
 12 files changed, 75 insertions(+), 91 deletions(-)
-- 
2.52.0

[PATCH net 01/11] netfilter: ctnetlink: fix use-after-free in ctnetlink_dump_exp_ct()

[Florian Westphal]

From: Hyunwoo Kim <imv4bel@gmail.com>

ctnetlink_dump_exp_ct() stores a conntrack pointer in cb->data for the
netlink dump callback ctnetlink_exp_ct_dump_table(), but drops the
conntrack reference immediately after netlink_dump_start().  When the
dump spans multiple rounds, the second recvmsg() triggers the dump
callback which dereferences the now-freed conntrack via nfct_help(ct),
leading to a use-after-free on ct->ext.

The bug is that the netlink_dump_control has no .start or .done
callbacks to manage the conntrack reference across dump rounds.  Other
dump functions in the same file (e.g. ctnetlink_get_conntrack) properly
use .start/.done callbacks for this purpose.

Fix this by adding .start and .done callbacks that hold and release the
conntrack reference for the duration of the dump, and move the
nfct_help() call after the cb->args[0] early-return check in the dump
callback to avoid dereferencing ct->ext unnecessarily.

 BUG: KASAN: slab-use-after-free in ctnetlink_exp_ct_dump_table+0x4f/0x2e0
 Read of size 8 at addr ffff88810597ebf0 by task ctnetlink_poc/133

 CPU: 1 UID: 0 PID: 133 Comm: ctnetlink_poc Not tainted 7.0.0-rc2+ #3 PREEMPTLAZY
 Call Trace:
  <TASK>
  ctnetlink_exp_ct_dump_table+0x4f/0x2e0
  netlink_dump+0x333/0x880
  netlink_recvmsg+0x3e2/0x4b0
  ? aa_sk_perm+0x184/0x450
  sock_recvmsg+0xde/0xf0

 Allocated by task 133:
  kmem_cache_alloc_noprof+0x134/0x440
  __nf_conntrack_alloc+0xa8/0x2b0
  ctnetlink_create_conntrack+0xa1/0x900
  ctnetlink_new_conntrack+0x3cf/0x7d0
  nfnetlink_rcv_msg+0x48e/0x510
  netlink_rcv_skb+0xc9/0x1f0
  nfnetlink_rcv+0xdb/0x220
  netlink_unicast+0x3ec/0x590
  netlink_sendmsg+0x397/0x690
  __sys_sendmsg+0xf4/0x180

 Freed by task 0:
  slab_free_after_rcu_debug+0xad/0x1e0
  rcu_core+0x5c3/0x9c0

Fixes: e844a928431f ("netfilter: ctnetlink: allow to dump expectation per master conntrack")
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_conntrack_netlink.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index c9d725fc2d71..65aa44a12d01 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3212,7 +3212,7 @@ ctnetlink_exp_ct_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct nfgenmsg *nfmsg = nlmsg_data(cb->nlh);
 	struct nf_conn *ct = cb->data;
-	struct nf_conn_help *help = nfct_help(ct);
+	struct nf_conn_help *help;
 	u_int8_t l3proto = nfmsg->nfgen_family;
 	unsigned long last_id = cb->args[1];
 	struct nf_conntrack_expect *exp;
@@ -3220,6 +3220,10 @@ ctnetlink_exp_ct_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
 	if (cb->args[0])
 		return 0;
 
+	help = nfct_help(ct);
+	if (!help)
+		return 0;
+
 	rcu_read_lock();
 
 restart:
@@ -3249,6 +3253,24 @@ ctnetlink_exp_ct_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+static int ctnetlink_dump_exp_ct_start(struct netlink_callback *cb)
+{
+	struct nf_conn *ct = cb->data;
+
+	if (!refcount_inc_not_zero(&ct->ct_general.use))
+		return -ENOENT;
+	return 0;
+}
+
+static int ctnetlink_dump_exp_ct_done(struct netlink_callback *cb)
+{
+	struct nf_conn *ct = cb->data;
+
+	if (ct)
+		nf_ct_put(ct);
+	return 0;
+}
+
 static int ctnetlink_dump_exp_ct(struct net *net, struct sock *ctnl,
 				 struct sk_buff *skb,
 				 const struct nlmsghdr *nlh,
@@ -3264,6 +3286,8 @@ static int ctnetlink_dump_exp_ct(struct net *net, struct sock *ctnl,
 	struct nf_conntrack_zone zone;
 	struct netlink_dump_control c = {
 		.dump = ctnetlink_exp_ct_dump_table,
+		.start = ctnetlink_dump_exp_ct_start,
+		.done = ctnetlink_dump_exp_ct_done,
 	};
 
 	err = ctnetlink_parse_tuple(cda, &tuple, CTA_EXPECT_MASTER,
-- 
2.52.0

[PATCH net 02/11] netfilter: conntrack: add missing netlink policy validations

[Florian Westphal]

Hyunwoo Kim reports out-of-bounds access in sctp and ctnetlink.

These attributes are used by the kernel without any validation.
Extend the netlink policies accordingly.

Quoting the reporter:
  nlattr_to_sctp() assigns the user-supplied CTA_PROTOINFO_SCTP_STATE
  value directly to ct->proto.sctp.state without checking that it is
  within the valid range. [..]

  and: ... with exp->dir = 100, the access at
  ct->master->tuplehash[100] reads 5600 bytes past the start of a
  320-byte nf_conn object, causing a slab-out-of-bounds read confirmed by
  UBSAN.

Fixes: 076a0ca02644 ("netfilter: ctnetlink: add NAT support for expectations")
Fixes: a258860e01b8 ("netfilter: ctnetlink: add full support for SCTP to ctnetlink")
Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_conntrack_netlink.c    | 2 +-
 net/netfilter/nf_conntrack_proto_sctp.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 65aa44a12d01..c156574e1273 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3489,7 +3489,7 @@ ctnetlink_change_expect(struct nf_conntrack_expect *x,
 
 #if IS_ENABLED(CONFIG_NF_NAT)
 static const struct nla_policy exp_nat_nla_policy[CTA_EXPECT_NAT_MAX+1] = {
-	[CTA_EXPECT_NAT_DIR]	= { .type = NLA_U32 },
+	[CTA_EXPECT_NAT_DIR]	= NLA_POLICY_MAX(NLA_BE32, IP_CT_DIR_REPLY),
 	[CTA_EXPECT_NAT_TUPLE]	= { .type = NLA_NESTED },
 };
 #endif
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 7c6f7c9f7332..645d2c43ebf7 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -582,7 +582,8 @@ static int sctp_to_nlattr(struct sk_buff *skb, struct nlattr *nla,
 }
 
 static const struct nla_policy sctp_nla_policy[CTA_PROTOINFO_SCTP_MAX+1] = {
-	[CTA_PROTOINFO_SCTP_STATE]	    = { .type = NLA_U8 },
+	[CTA_PROTOINFO_SCTP_STATE]	    = NLA_POLICY_MAX(NLA_U8,
+							 SCTP_CONNTRACK_HEARTBEAT_SENT),
 	[CTA_PROTOINFO_SCTP_VTAG_ORIGINAL]  = { .type = NLA_U32 },
 	[CTA_PROTOINFO_SCTP_VTAG_REPLY]     = { .type = NLA_U32 },
 };
-- 
2.52.0

[PATCH net 03/11] netfilter: nf_conntrack_sip: fix Content-Length u32 truncation in sip_help_tcp()

[Florian Westphal]

From: Lukas Johannes Möller <research@johannes-moeller.dev>

sip_help_tcp() parses the SIP Content-Length header with
simple_strtoul(), which returns unsigned long, but stores the result in
unsigned int clen.  On 64-bit systems, values exceeding UINT_MAX are
silently truncated before computing the SIP message boundary.

For example, Content-Length 4294967328 (2^32 + 32) is truncated to 32,
causing the parser to miscalculate where the current message ends.  The
loop then treats trailing data in the TCP segment as a second SIP
message and processes it through the SDP parser.

Fix this by changing clen to unsigned long to match the return type of
simple_strtoul(), and reject Content-Length values that exceed the
remaining TCP payload length.

Fixes: f5b321bd37fb ("netfilter: nf_conntrack_sip: add TCP support")
Signed-off-by: Lukas Johannes Möller <research@johannes-moeller.dev>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_conntrack_sip.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index ca748f8dbff1..4ab5ef71d96d 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1534,11 +1534,12 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,
 {
 	struct tcphdr *th, _tcph;
 	unsigned int dataoff, datalen;
-	unsigned int matchoff, matchlen, clen;
+	unsigned int matchoff, matchlen;
 	unsigned int msglen, origlen;
 	const char *dptr, *end;
 	s16 diff, tdiff = 0;
 	int ret = NF_ACCEPT;
+	unsigned long clen;
 	bool term;
 
 	if (ctinfo != IP_CT_ESTABLISHED &&
@@ -1573,6 +1574,9 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,
 		if (dptr + matchoff == end)
 			break;
 
+		if (clen > datalen)
+			break;
+
 		term = false;
 		for (; end + strlen("\r\n\r\n") <= dptr + datalen; end++) {
 			if (end[0] == '\r' && end[1] == '\n' &&
-- 
2.52.0

[PATCH net 04/11] netfilter: revert nft_set_rbtree: validate open interval overlap

[Florian Westphal]

This reverts commit 648946966a08 ("netfilter: nft_set_rbtree: validate
open interval overlap").

There have been reports of nft failing to laod valid rulesets after this
patch was merged into -stable.

I can reproduce several such problem with recent nft versions, including
nft 1.1.6 which is widely shipped by distributions.

We currently have little choice here.
This commit can be resurrected at some point once the nftables fix that
triggers the false overlap positive has appeared in common distros
(see e83e32c8d1cd ("mnl: restore create element command with large batches" in
 nftables.git).

Fixes: 648946966a08 ("netfilter: nft_set_rbtree: validate open interval overlap")
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h |  4 --
 net/netfilter/nf_tables_api.c     | 21 ++-------
 net/netfilter/nft_set_rbtree.c    | 71 +++++--------------------------
 3 files changed, 14 insertions(+), 82 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index e2d2bfc1f989..6299af4ef423 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -277,8 +277,6 @@ struct nft_userdata {
 	unsigned char		data[];
 };
 
-#define NFT_SET_ELEM_INTERNAL_LAST	0x1
-
 /* placeholder structure for opaque set element backend representation. */
 struct nft_elem_priv { };
 
@@ -288,7 +286,6 @@ struct nft_elem_priv { };
  *	@key: element key
  *	@key_end: closing element key
  *	@data: element data
- * 	@flags: flags
  *	@priv: element private data and extensions
  */
 struct nft_set_elem {
@@ -304,7 +301,6 @@ struct nft_set_elem {
 		u32		buf[NFT_DATA_VALUE_MAXLEN / sizeof(u32)];
 		struct nft_data val;
 	} data;
-	u32			flags;
 	struct nft_elem_priv	*priv;
 };
 
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index dacec5f8a11c..4ccdd33cf133 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -7156,8 +7156,7 @@ static u32 nft_set_maxsize(const struct nft_set *set)
 }
 
 static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
-			    const struct nlattr *attr, u32 nlmsg_flags,
-			    bool last)
+			    const struct nlattr *attr, u32 nlmsg_flags)
 {
 	struct nft_expr *expr_array[NFT_SET_EXPR_MAX] = {};
 	struct nlattr *nla[NFTA_SET_ELEM_MAX + 1];
@@ -7444,11 +7443,6 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 	if (flags)
 		*nft_set_ext_flags(ext) = flags;
 
-	if (last)
-		elem.flags = NFT_SET_ELEM_INTERNAL_LAST;
-	else
-		elem.flags = 0;
-
 	if (obj)
 		*nft_set_ext_obj(ext) = obj;
 
@@ -7613,8 +7607,7 @@ static int nf_tables_newsetelem(struct sk_buff *skb,
 	nft_ctx_init(&ctx, net, skb, info->nlh, family, table, NULL, nla);
 
 	nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) {
-		err = nft_add_set_elem(&ctx, set, attr, info->nlh->nlmsg_flags,
-				       nla_is_last(attr, rem));
+		err = nft_add_set_elem(&ctx, set, attr, info->nlh->nlmsg_flags);
 		if (err < 0) {
 			NL_SET_BAD_ATTR(extack, attr);
 			return err;
@@ -7738,7 +7731,7 @@ static void nft_trans_elems_destroy_abort(const struct nft_ctx *ctx,
 }
 
 static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
-			   const struct nlattr *attr, bool last)
+			   const struct nlattr *attr)
 {
 	struct nlattr *nla[NFTA_SET_ELEM_MAX + 1];
 	struct nft_set_ext_tmpl tmpl;
@@ -7806,11 +7799,6 @@ static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
 	if (flags)
 		*nft_set_ext_flags(ext) = flags;
 
-	if (last)
-		elem.flags = NFT_SET_ELEM_INTERNAL_LAST;
-	else
-		elem.flags = 0;
-
 	trans = nft_trans_elem_alloc(ctx, NFT_MSG_DELSETELEM, set);
 	if (trans == NULL)
 		goto fail_trans;
@@ -7961,8 +7949,7 @@ static int nf_tables_delsetelem(struct sk_buff *skb,
 		return nft_set_flush(&ctx, set, genmask);
 
 	nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) {
-		err = nft_del_setelem(&ctx, set, attr,
-				      nla_is_last(attr, rem));
+		err = nft_del_setelem(&ctx, set, attr);
 		if (err == -ENOENT &&
 		    NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_DESTROYSETELEM)
 			continue;
diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index ee3d4f5b9ff7..fe8bd497d74a 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -304,19 +304,10 @@ static void nft_rbtree_set_start_cookie(struct nft_rbtree *priv,
 	priv->start_rbe_cookie = (unsigned long)rbe;
 }
 
-static void nft_rbtree_set_start_cookie_open(struct nft_rbtree *priv,
-					     const struct nft_rbtree_elem *rbe,
-					     unsigned long open_interval)
-{
-	priv->start_rbe_cookie = (unsigned long)rbe | open_interval;
-}
-
-#define NFT_RBTREE_OPEN_INTERVAL	1UL
-
 static bool nft_rbtree_cmp_start_cookie(struct nft_rbtree *priv,
 					const struct nft_rbtree_elem *rbe)
 {
-	return (priv->start_rbe_cookie & ~NFT_RBTREE_OPEN_INTERVAL) == (unsigned long)rbe;
+	return priv->start_rbe_cookie == (unsigned long)rbe;
 }
 
 static bool nft_rbtree_insert_same_interval(const struct net *net,
@@ -346,14 +337,13 @@ static bool nft_rbtree_insert_same_interval(const struct net *net,
 
 static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
 			       struct nft_rbtree_elem *new,
-			       struct nft_elem_priv **elem_priv, u64 tstamp, bool last)
+			       struct nft_elem_priv **elem_priv, u64 tstamp)
 {
 	struct nft_rbtree_elem *rbe, *rbe_le = NULL, *rbe_ge = NULL, *rbe_prev;
 	struct rb_node *node, *next, *parent, **p, *first = NULL;
 	struct nft_rbtree *priv = nft_set_priv(set);
 	u8 cur_genmask = nft_genmask_cur(net);
 	u8 genmask = nft_genmask_next(net);
-	unsigned long open_interval = 0;
 	int d;
 
 	/* Descend the tree to search for an existing element greater than the
@@ -459,18 +449,10 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
 		}
 	}
 
-	if (nft_rbtree_interval_null(set, new)) {
+	if (nft_rbtree_interval_null(set, new))
+		priv->start_rbe_cookie = 0;
+	else if (nft_rbtree_interval_start(new) && priv->start_rbe_cookie)
 		priv->start_rbe_cookie = 0;
-	} else if (nft_rbtree_interval_start(new) && priv->start_rbe_cookie) {
-		if (nft_set_is_anonymous(set)) {
-			priv->start_rbe_cookie = 0;
-		} else if (priv->start_rbe_cookie & NFT_RBTREE_OPEN_INTERVAL) {
-			/* Previous element is an open interval that partially
-			 * overlaps with an existing non-open interval.
-			 */
-			return -ENOTEMPTY;
-		}
-	}
 
 	/* - new start element matching existing start element: full overlap
 	 *   reported as -EEXIST, cleared by caller if NLM_F_EXCL is not given.
@@ -478,27 +460,7 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
 	if (rbe_ge && !nft_rbtree_cmp(set, new, rbe_ge) &&
 	    nft_rbtree_interval_start(rbe_ge) == nft_rbtree_interval_start(new)) {
 		*elem_priv = &rbe_ge->priv;
-
-		/* - Corner case: new start element of open interval (which
-		 *   comes as last element in the batch) overlaps the start of
-		 *   an existing interval with an end element: partial overlap.
-		 */
-		node = rb_first(&priv->root);
-		rbe = __nft_rbtree_next_active(node, genmask);
-		if (rbe && nft_rbtree_interval_end(rbe)) {
-			rbe = nft_rbtree_next_active(rbe, genmask);
-			if (rbe &&
-			    nft_rbtree_interval_start(rbe) &&
-			    !nft_rbtree_cmp(set, new, rbe)) {
-				if (last)
-					return -ENOTEMPTY;
-
-				/* Maybe open interval? */
-				open_interval = NFT_RBTREE_OPEN_INTERVAL;
-			}
-		}
-		nft_rbtree_set_start_cookie_open(priv, rbe_ge, open_interval);
-
+		nft_rbtree_set_start_cookie(priv, rbe_ge);
 		return -EEXIST;
 	}
 
@@ -553,12 +515,6 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
 	    nft_rbtree_interval_end(rbe_ge) && nft_rbtree_interval_end(new))
 		return -ENOTEMPTY;
 
-	/* - start element overlaps an open interval but end element is new:
-	 *   partial overlap, reported as -ENOEMPTY.
-	 */
-	if (!rbe_ge && priv->start_rbe_cookie && nft_rbtree_interval_end(new))
-		return -ENOTEMPTY;
-
 	/* Accepted element: pick insertion point depending on key value */
 	parent = NULL;
 	p = &priv->root.rb_node;
@@ -668,7 +624,6 @@ static int nft_rbtree_insert(const struct net *net, const struct nft_set *set,
 			     struct nft_elem_priv **elem_priv)
 {
 	struct nft_rbtree_elem *rbe = nft_elem_priv_cast(elem->priv);
-	bool last = !!(elem->flags & NFT_SET_ELEM_INTERNAL_LAST);
 	struct nft_rbtree *priv = nft_set_priv(set);
 	u64 tstamp = nft_net_tstamp(net);
 	int err;
@@ -685,12 +640,8 @@ static int nft_rbtree_insert(const struct net *net, const struct nft_set *set,
 		cond_resched();
 
 		write_lock_bh(&priv->lock);
-		err = __nft_rbtree_insert(net, set, rbe, elem_priv, tstamp, last);
+		err = __nft_rbtree_insert(net, set, rbe, elem_priv, tstamp);
 		write_unlock_bh(&priv->lock);
-
-		if (nft_rbtree_interval_end(rbe))
-			priv->start_rbe_cookie = 0;
-
 	} while (err == -EAGAIN);
 
 	return err;
@@ -778,7 +729,6 @@ nft_rbtree_deactivate(const struct net *net, const struct nft_set *set,
 		      const struct nft_set_elem *elem)
 {
 	struct nft_rbtree_elem *rbe, *this = nft_elem_priv_cast(elem->priv);
-	bool last = !!(elem->flags & NFT_SET_ELEM_INTERNAL_LAST);
 	struct nft_rbtree *priv = nft_set_priv(set);
 	const struct rb_node *parent = priv->root.rb_node;
 	u8 genmask = nft_genmask_next(net);
@@ -819,10 +769,9 @@ nft_rbtree_deactivate(const struct net *net, const struct nft_set *set,
 				continue;
 			}
 
-			if (nft_rbtree_interval_start(rbe)) {
-				if (!last)
-					nft_rbtree_set_start_cookie(priv, rbe);
-			} else if (!nft_rbtree_deactivate_same_interval(net, priv, rbe))
+			if (nft_rbtree_interval_start(rbe))
+				nft_rbtree_set_start_cookie(priv, rbe);
+			else if (!nft_rbtree_deactivate_same_interval(net, priv, rbe))
 				return NULL;
 
 			nft_rbtree_flush(net, set, &rbe->priv);
-- 
2.52.0

[PATCH net 05/11] netfilter: nf_flow_table_ip: reset mac header before vlan push

[Florian Westphal]

From: Eric Woudstra <ericwouds@gmail.com>

With double vlan tagged packets in the fastpath, getting the error:

skb_vlan_push got skb with skb->data not at mac header (offset 18)

Call skb_reset_mac_header() before calling skb_vlan_push().

Fixes: c653d5a78f34 ("netfilter: flowtable: inline vlan encapsulation in xmit path")
Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_flow_table_ip.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 3fdb10d9bf7f..fd56d663cb5b 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -738,6 +738,7 @@ static int nf_flow_encap_push(struct sk_buff *skb,
 		switch (tuple->encap[i].proto) {
 		case htons(ETH_P_8021Q):
 		case htons(ETH_P_8021AD):
+			skb_reset_mac_header(skb);
 			if (skb_vlan_push(skb, tuple->encap[i].proto,
 					  tuple->encap[i].id) < 0)
 				return -1;
-- 
2.52.0

[PATCH net 06/11] netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case

[Florian Westphal]

From: Jenny Guanni Qu <qguanni@gmail.com>

In decode_int(), the CONS case calls get_bits(bs, 2) to read a length
value, then calls get_uint(bs, len) without checking that len bytes
remain in the buffer. The existing boundary check only validates the
2 bits for get_bits(), not the subsequent 1-4 bytes that get_uint()
reads. This allows a malformed H.323/RAS packet to cause a 1-4 byte
slab-out-of-bounds read.

Add a boundary check for len bytes after get_bits() and before
get_uint().

Fixes: 5e35941d9901 ("[NETFILTER]: Add H.323 conntrack/NAT helper")
Reported-by: Klaudia Kloc <klaudia@vidocsecurity.com>
Reported-by: Dawid Moczadło <dawid@vidocsecurity.com>
Signed-off-by: Jenny Guanni Qu <qguanni@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_conntrack_h323_asn1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
index 62aa22a07876..c972e9488e16 100644
--- a/net/netfilter/nf_conntrack_h323_asn1.c
+++ b/net/netfilter/nf_conntrack_h323_asn1.c
@@ -331,6 +331,8 @@ static int decode_int(struct bitstr *bs, const struct field_t *f,
 		if (nf_h323_error_boundary(bs, 0, 2))
 			return H323_ERROR_BOUND;
 		len = get_bits(bs, 2) + 1;
+		if (nf_h323_error_boundary(bs, len, 0))
+			return H323_ERROR_BOUND;
 		BYTE_ALIGN(bs);
 		if (base && (f->attr & DECODE)) {	/* timeToLive */
 			unsigned int v = get_uint(bs, len) + f->lb;
-- 
2.52.0

[PATCH net 07/11] nf_tables: nft_dynset: fix possible stateful expression memleak in error path

[Florian Westphal]

From: Pablo Neira Ayuso <pablo@netfilter.org>

If cloning the second stateful expression in the element via GFP_ATOMIC
fails, then the first stateful expression remains in place without being
released.

   unreferenced object (percpu) 0x607b97e9cab8 (size 16):
     comm "softirq", pid 0, jiffies 4294931867
     hex dump (first 16 bytes on cpu 3):
       00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     backtrace (crc 0):
       pcpu_alloc_noprof+0x453/0xd80
       nft_counter_clone+0x9c/0x190 [nf_tables]
       nft_expr_clone+0x8f/0x1b0 [nf_tables]
       nft_dynset_new+0x2cb/0x5f0 [nf_tables]
       nft_rhash_update+0x236/0x11c0 [nf_tables]
       nft_dynset_eval+0x11f/0x670 [nf_tables]
       nft_do_chain+0x253/0x1700 [nf_tables]
       nft_do_chain_ipv4+0x18d/0x270 [nf_tables]
       nf_hook_slow+0xaa/0x1e0
       ip_local_deliver+0x209/0x330

Fixes: 563125a73ac3 ("netfilter: nftables: generalize set extension to support for several expressions")
Reported-by: Gurpreet Shergill <giki.shergill@proton.me>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h |  2 ++
 net/netfilter/nf_tables_api.c     |  4 ++--
 net/netfilter/nft_dynset.c        | 10 +++++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 6299af4ef423..ec8a8ec9c0aa 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -874,6 +874,8 @@ struct nft_elem_priv *nft_set_elem_init(const struct nft_set *set,
 					u64 timeout, u64 expiration, gfp_t gfp);
 int nft_set_elem_expr_clone(const struct nft_ctx *ctx, struct nft_set *set,
 			    struct nft_expr *expr_array[]);
+void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
+			       struct nft_set_elem_expr *elem_expr);
 void nft_set_elem_destroy(const struct nft_set *set,
 			  const struct nft_elem_priv *elem_priv,
 			  bool destroy_expr);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 4ccdd33cf133..9b1c8d0a35fb 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6744,8 +6744,8 @@ static void __nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
 	}
 }
 
-static void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
-				      struct nft_set_elem_expr *elem_expr)
+void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
+			       struct nft_set_elem_expr *elem_expr)
 {
 	struct nft_expr *expr;
 	u32 size;
diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c
index 7807d8129664..9123277be03c 100644
--- a/net/netfilter/nft_dynset.c
+++ b/net/netfilter/nft_dynset.c
@@ -30,18 +30,26 @@ static int nft_dynset_expr_setup(const struct nft_dynset *priv,
 				 const struct nft_set_ext *ext)
 {
 	struct nft_set_elem_expr *elem_expr = nft_set_ext_expr(ext);
+	struct nft_ctx ctx = {
+		.net	= read_pnet(&priv->set->net),
+		.family	= priv->set->table->family,
+	};
 	struct nft_expr *expr;
 	int i;
 
 	for (i = 0; i < priv->num_exprs; i++) {
 		expr = nft_setelem_expr_at(elem_expr, elem_expr->size);
 		if (nft_expr_clone(expr, priv->expr_array[i], GFP_ATOMIC) < 0)
-			return -1;
+			goto err_out;
 
 		elem_expr->size += priv->expr_array[i]->ops->size;
 	}
 
 	return 0;
+err_out:
+	nft_set_elem_expr_destroy(&ctx, elem_expr);
+
+	return -1;
 }
 
 struct nft_elem_priv *nft_dynset_new(struct nft_set *set,
-- 
2.52.0

[PATCH net 08/11] netfilter: nft_ct: drop pending enqueued packets on removal

[Florian Westphal]

From: Pablo Neira Ayuso <pablo@netfilter.org>

Packets sitting in nfqueue might hold a reference to:

- templates that specify the conntrack zone, because a percpu area is
  used and module removal is possible.
- conntrack timeout policies and helper, where object removal leave
  a stale reference.

Since these objects can just go away, drop enqueued packets to avoid
stale reference to them.

If there is a need for finer grain removal, this logic can be revisited
to make selective packet drop upon dependencies.

Fixes: 7e0b2b57f01d ("netfilter: nft_ct: add ct timeout support")
Reported-by: Yiming Qian <yimingqian591@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nft_ct.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 47d3ef109a99..128ff8155b5d 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -23,6 +23,7 @@
 #include <net/netfilter/nf_conntrack_l4proto.h>
 #include <net/netfilter/nf_conntrack_expect.h>
 #include <net/netfilter/nf_conntrack_seqadj.h>
+#include "nf_internals.h"
 
 struct nft_ct_helper_obj  {
 	struct nf_conntrack_helper *helper4;
@@ -543,6 +544,7 @@ static void __nft_ct_set_destroy(const struct nft_ctx *ctx, struct nft_ct *priv)
 #endif
 #ifdef CONFIG_NF_CONNTRACK_ZONES
 	case NFT_CT_ZONE:
+		nf_queue_nf_hook_drop(ctx->net);
 		mutex_lock(&nft_ct_pcpu_mutex);
 		if (--nft_ct_pcpu_template_refcnt == 0)
 			nft_ct_tmpl_put_pcpu();
@@ -1015,6 +1017,7 @@ static void nft_ct_timeout_obj_destroy(const struct nft_ctx *ctx,
 	struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
 	struct nf_ct_timeout *timeout = priv->timeout;
 
+	nf_queue_nf_hook_drop(ctx->net);
 	nf_ct_untimeout(ctx->net, timeout);
 	nf_ct_netns_put(ctx->net, ctx->family);
 	kfree(priv->timeout);
@@ -1147,6 +1150,7 @@ static void nft_ct_helper_obj_destroy(const struct nft_ctx *ctx,
 {
 	struct nft_ct_helper_obj *priv = nft_obj_data(obj);
 
+	nf_queue_nf_hook_drop(ctx->net);
 	if (priv->helper4)
 		nf_conntrack_helper_put(priv->helper4);
 	if (priv->helper6)
-- 
2.52.0

[PATCH net 09/11] netfilter: xt_CT: drop pending enqueued packets on template removal

[Florian Westphal]

From: Pablo Neira Ayuso <pablo@netfilter.org>

Templates refer to objects that can go away while packets are sitting in
nfqueue refer to:

- helper, this can be an issue on module removal.
- timeout policy, nfnetlink_cttimeout might remove it.

The use of templates with zone and event cache filter are safe, since
this just copies values.

Flush these enqueued packets in case the template rule gets removed.

Fixes: 24de58f46516 ("netfilter: xt_CT: allow to attach timeout policy + glue code")
Reported-by: Yiming Qian <yimingqian591@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/xt_CT.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index 3ba94c34297c..498f5871c84a 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -16,6 +16,7 @@
 #include <net/netfilter/nf_conntrack_ecache.h>
 #include <net/netfilter/nf_conntrack_timeout.h>
 #include <net/netfilter/nf_conntrack_zones.h>
+#include "nf_internals.h"
 
 static inline int xt_ct_target(struct sk_buff *skb, struct nf_conn *ct)
 {
@@ -283,6 +284,9 @@ static void xt_ct_tg_destroy(const struct xt_tgdtor_param *par,
 	struct nf_conn_help *help;
 
 	if (ct) {
+		if (info->helper[0] || info->timeout[0])
+			nf_queue_nf_hook_drop(par->net);
+
 		help = nfct_help(ct);
 		xt_ct_put_helper(help);
 
-- 
2.52.0

[PATCH net 10/11] netfilter: xt_time: use unsigned int for monthday bit shift

[Florian Westphal]

From: Jenny Guanni Qu <qguanni@gmail.com>

The monthday field can be up to 31, and shifting a signed integer 1
by 31 positions (1 << 31) is undefined behavior in C, as the result
overflows a 32-bit signed int. Use 1U to ensure well-defined behavior
for all valid monthday values.

Change the weekday shift to 1U as well for consistency.

Fixes: ee4411a1b1e0 ("[NETFILTER]: x_tables: add xt_time match")
Reported-by: Klaudia Kloc <klaudia@vidocsecurity.com>
Reported-by: Dawid Moczadło <dawid@vidocsecurity.com>
Tested-by: Jenny Guanni Qu <qguanni@gmail.com>
Signed-off-by: Jenny Guanni Qu <qguanni@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/xt_time.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
index 00319d2a54da..d9d74011bb64 100644
--- a/net/netfilter/xt_time.c
+++ b/net/netfilter/xt_time.c
@@ -223,13 +223,13 @@ time_mt(const struct sk_buff *skb, struct xt_action_param *par)
 
 	localtime_2(&current_time, stamp);
 
-	if (!(info->weekdays_match & (1 << current_time.weekday)))
+	if (!(info->weekdays_match & (1U << current_time.weekday)))
 		return false;
 
 	/* Do not spend time computing monthday if all days match anyway */
 	if (info->monthdays_match != XT_TIME_ALL_MONTHDAYS) {
 		localtime_3(&current_time, stamp);
-		if (!(info->monthdays_match & (1 << current_time.monthday)))
+		if (!(info->monthdays_match & (1U << current_time.monthday)))
 			return false;
 	}
 
-- 
2.52.0

[PATCH net 11/11] netfilter: nf_conntrack_h323: check for zero length in DecodeQ931()

[Florian Westphal]

From: Jenny Guanni Qu <qguanni@gmail.com>

In DecodeQ931(), the UserUserIE code path reads a 16-bit length from
the packet, then decrements it by 1 to skip the protocol discriminator
byte before passing it to DecodeH323_UserInformation(). If the encoded
length is 0, the decrement wraps to -1, which is then passed as a
large value to the decoder, leading to an out-of-bounds read.

Add a check to ensure len is positive after the decrement.

Fixes: 5e35941d9901 ("[NETFILTER]: Add H.323 conntrack/NAT helper")
Reported-by: Klaudia Kloc <klaudia@vidocsecurity.com>
Reported-by: Dawid Moczadło <dawid@vidocsecurity.com>
Tested-by: Jenny Guanni Qu <qguanni@gmail.com>
Signed-off-by: Jenny Guanni Qu <qguanni@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_conntrack_h323_asn1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
index c972e9488e16..7b1497ed97d2 100644
--- a/net/netfilter/nf_conntrack_h323_asn1.c
+++ b/net/netfilter/nf_conntrack_h323_asn1.c
@@ -924,6 +924,8 @@ int DecodeQ931(unsigned char *buf, size_t sz, Q931 *q931)
 				break;
 			p++;
 			len--;
+			if (len <= 0)
+				break;
 			return DecodeH323_UserInformation(buf, p, len,
 							  &q931->UUIE);
 		}
-- 
2.52.0

Re: [net,06/11] netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case

This patch adds a boundary check in the CONS case of decode_int() to
prevent reading beyond the buffer after get_bits() returns a length
value. The fix validates that len bytes remain before calling
get_uint().

> diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
> index 62aa22a07876..c972e9488e16 100644
> --- a/net/netfilter/nf_conntrack_h323_asn1.c
> +++ b/net/netfilter/nf_conntrack_h323_asn1.c
> @@ -331,6 +331,8 @@ static int decode_int(struct bitstr *bs, const struct field_t *f,
>  		if (nf_h323_error_boundary(bs, 0, 2))
>  			return H323_ERROR_BOUND;
>  		len = get_bits(bs, 2) + 1;
> +		if (nf_h323_error_boundary(bs, len, 0))
> +			return H323_ERROR_BOUND;
>  		BYTE_ALIGN(bs);
>  		if (base && (f->attr & DECODE)) {	/* timeToLive */
>  			unsigned int v = get_uint(bs, len) + f->lb;

This patch fixes the CONS case, but the UNCO case in the same function
has the same vulnerability. In the UNCO case, get_len(bs) can return up
to 0x3FFF (16383 bytes), and the code advances bs->cur by that amount
without checking that len bytes remain:

	case UNCO:
		BYTE_ALIGN(bs);
		if (nf_h323_error_boundary(bs, 2, 0))
			return H323_ERROR_BOUND;
		len = get_len(bs);
		bs->cur += len;
		break;

The check only validates 2 bytes for get_len() itself, not the
subsequent len bytes that will be skipped. Does this need the same fix
pattern?

The decode_bitstr() function has a similar issue. In the SEMI case, it
calls get_len(bs) which can return up to 0x3FFF bits, then advances the
buffer by len >> 3 bytes plus partial bits without checking that
sufficient buffer space exists:

	case SEMI:
		if (nf_h323_error_boundary(bs, 2, 0))
			return H323_ERROR_BOUND;
		len = get_len(bs);
		break;
	}

	bs->cur += len >> 3;
	bs->bit = len & 7;

The check at the top only validates 2 bytes for the get_len() call. The
actual advance at the bottom can be up to 2047 bytes. Should this be
checked as well?

The decode_octstr() function has the same pattern in both the SEMI case
and the default case:

	case SEMI:
		BYTE_ALIGN(bs);
		if (nf_h323_error_boundary(bs, 2, 0))
			return H323_ERROR_BOUND;
		len = get_len(bs) + f->lb;
		break;
	default:		/* 2 <= Range <= 255 */
		if (nf_h323_error_boundary(bs, 0, f->sz))
			return H323_ERROR_BOUND;
		len = get_bits(bs, f->sz) + f->lb;
		BYTE_ALIGN(bs);
		break;
	}

	bs->cur += len;

In the SEMI case, get_len() can return up to 0x3FFF and f->lb is added.
In the default case, get_bits(bs, f->sz) returns an attacker-controlled
value based on field size and f->lb is added. Both cases then advance by
len bytes without checking. The existing checks only validate the bytes
needed to READ the length value, not the len bytes that will be
consumed. Should boundary checks be added after both break statements?

Similarly, decode_bmpstr() (which handles 2-byte character strings) has
the same issue in both the BYTE case and default case:

	case BYTE:		/* Range == 256 */
		BYTE_ALIGN(bs);
		if (nf_h323_error_boundary(bs, 1, 0))
			return H323_ERROR_BOUND;
		len = (*bs->cur++) + f->lb;
		break;
	default:		/* 2 <= Range <= 255 */
		if (nf_h323_error_boundary(bs, 0, f->sz))
			return H323_ERROR_BOUND;
		len = get_bits(bs, f->sz) + f->lb;
		BYTE_ALIGN(bs);
		break;
	}

	bs->cur += len << 1;

After computing len from packet data plus f->lb, the function advances
by len << 1 bytes (doubling for 2-byte characters) without verifying
that len << 1 bytes are available. Should this also be checked?

Re: [PATCH net 01/11] netfilter: ctnetlink: fix use-after-free in ctnetlink_dump_exp_ct()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Florian Westphal <fw@strlen.de>:

On Fri, 13 Mar 2026 16:06:04 +0100 you wrote:
> From: Hyunwoo Kim <imv4bel@gmail.com>
> 
> ctnetlink_dump_exp_ct() stores a conntrack pointer in cb->data for the
> netlink dump callback ctnetlink_exp_ct_dump_table(), but drops the
> conntrack reference immediately after netlink_dump_start().  When the
> dump spans multiple rounds, the second recvmsg() triggers the dump
> callback which dereferences the now-freed conntrack via nfct_help(ct),
> leading to a use-after-free on ct->ext.
> 
> [...]

Here is the summary with links:
  - [net,01/11] netfilter: ctnetlink: fix use-after-free in ctnetlink_dump_exp_ct()
    https://git.kernel.org/netdev/net/c/5cb81eeda909
  - [net,02/11] netfilter: conntrack: add missing netlink policy validations
    https://git.kernel.org/netdev/net/c/f900e1d77ee0
  - [net,03/11] netfilter: nf_conntrack_sip: fix Content-Length u32 truncation in sip_help_tcp()
    https://git.kernel.org/netdev/net/c/fbce58e719a1
  - [net,04/11] netfilter: revert nft_set_rbtree: validate open interval overlap
    https://git.kernel.org/netdev/net/c/598adea720b9
  - [net,05/11] netfilter: nf_flow_table_ip: reset mac header before vlan push
    https://git.kernel.org/netdev/net/c/a3aca98aec9a
  - [net,06/11] netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case
    https://git.kernel.org/netdev/net/c/1e3a3593162c
  - [net,07/11] nf_tables: nft_dynset: fix possible stateful expression memleak in error path
    https://git.kernel.org/netdev/net/c/0548a13b5a14
  - [net,08/11] netfilter: nft_ct: drop pending enqueued packets on removal
    https://git.kernel.org/netdev/net/c/36eae0956f65
  - [net,09/11] netfilter: xt_CT: drop pending enqueued packets on template removal
    https://git.kernel.org/netdev/net/c/f62a218a946b
  - [net,10/11] netfilter: xt_time: use unsigned int for monthday bit shift
    https://git.kernel.org/netdev/net/c/00050ec08cec
  - [net,11/11] netfilter: nf_conntrack_h323: check for zero length in DecodeQ931()
    https://git.kernel.org/netdev/net/c/f173d0f4c0f6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [net,06/11] netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case

[Florian Westphal]

Jakub Kicinski <kuba@kernel.org> wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.

Jenny, could you please double-check?
I don't think this report is right.

> netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case
> 
> This patch adds a boundary check in the CONS case of decode_int() to
> prevent reading beyond the buffer after get_bits() returns a length
> value. The fix validates that len bytes remain before calling
> get_uint().

SO far I can follow LLM response.

> > diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
> > index 62aa22a07876..c972e9488e16 100644
> > --- a/net/netfilter/nf_conntrack_h323_asn1.c
> > +++ b/net/netfilter/nf_conntrack_h323_asn1.c
> > @@ -331,6 +331,8 @@ static int decode_int(struct bitstr *bs, const struct field_t *f,
> >  		if (nf_h323_error_boundary(bs, 0, 2))
> >  			return H323_ERROR_BOUND;
> >  		len = get_bits(bs, 2) + 1;
> > +		if (nf_h323_error_boundary(bs, len, 0))
> > +			return H323_ERROR_BOUND;
> >  		BYTE_ALIGN(bs);
> >  		if (base && (f->attr & DECODE)) {	/* timeToLive */
> >  			unsigned int v = get_uint(bs, len) + f->lb;
> 
> This patch fixes the CONS case, but the UNCO case in the same function
> has the same vulnerability. In the UNCO case, get_len(bs) can return up
> to 0x3FFF (16383 bytes), and the code advances bs->cur by that amount
> without checking that len bytes remain:
> 
> 	case UNCO:
> 		BYTE_ALIGN(bs);
> 		if (nf_h323_error_boundary(bs, 2, 0))
> 			return H323_ERROR_BOUND;
> 		len = get_len(bs);
> 		bs->cur += len;
> 		break;
> 
> The check only validates 2 bytes for get_len() itself, not the
> subsequent len bytes that will be skipped. Does this need the same fix
> pattern?

I don't think so.  Pointer can advance, but right after the swtich/case
there is:

358         if (nf_h323_error_boundary(bs, 0, 0))
359                 return H323_ERROR_BOUND;
360         return H323_ERROR_NONE;

So bs->cur can advance past cur->end, but then the function
returns ERROR_BOUND.

> The decode_bitstr() function has a similar issue. In the SEMI case, it
> calls get_len(bs) which can return up to 0x3FFF bits, then advances the
> buffer by len >> 3 bytes plus partial bits without checking that
> sufficient buffer space exists:
> 
> 	case SEMI:
> 		if (nf_h323_error_boundary(bs, 2, 0))
> 			return H323_ERROR_BOUND;
> 		len = get_len(bs);

AFAICS we've checked get_len() doesn't cause oob access.

> 		break;
> 	}
> 
> 	bs->cur += len >> 3;
> 	bs->bit = len & 7;
> 
> The check at the top only validates 2 bytes for the get_len() call. The
> actual advance at the bottom can be up to 2047 bytes. Should this be
> checked as well?

AFAICS it is, right after there is:
410         if (nf_h323_error_boundary(bs, 0, 0))
411                 return H323_ERROR_BOUND;

> The decode_octstr() function has the same pattern in both the SEMI case
> and the default case:
> 
> 	case SEMI:
> 		BYTE_ALIGN(bs);
> 		if (nf_h323_error_boundary(bs, 2, 0))
> 			return H323_ERROR_BOUND;
> 		len = get_len(bs) + f->lb;
> 		break;
> 	default:		/* 2 <= Range <= 255 */
> 		if (nf_h323_error_boundary(bs, 0, f->sz))
> 			return H323_ERROR_BOUND;
> 		len = get_bits(bs, f->sz) + f->lb;
> 		BYTE_ALIGN(bs);
> 		break;
> 	}
> 
> 	bs->cur += len;

Pointer increment, right after there is:

483         if (nf_h323_error_boundary(bs, 0, 0))
484                 return H323_ERROR_BOUND;

> len bytes without checking. The existing checks only validate the bytes
> needed to READ the length value, not the len bytes that will be
> consumed. Should boundary checks be added after both break statements?

AFAICS there are such checks... :-/

> Similarly, decode_bmpstr() (which handles 2-byte character strings) has
> the same issue in both the BYTE case and default case:
> 
> 	case BYTE:		/* Range == 256 */
> 		BYTE_ALIGN(bs);
> 		if (nf_h323_error_boundary(bs, 1, 0))
> 			return H323_ERROR_BOUND;
> 		len = (*bs->cur++) + f->lb;
> 		break;

We write to bs->cur, but AFAICS the write has been vetted for.

> 	default:		/* 2 <= Range <= 255 */
> 		if (nf_h323_error_boundary(bs, 0, f->sz))
> 			return H323_ERROR_BOUND;
> 		len = get_bits(bs, f->sz) + f->lb;
> 		BYTE_ALIGN(bs);
> 		break;
> 	}
> 
> 	bs->cur += len << 1;
> 
> After computing len from packet data plus f->lb, the function advances
> by len << 1 bytes (doubling for 2-byte characters) without verifying
> that len << 1 bytes are available. Should this also be checked?

AFAICS it is...

510         bs->cur += len << 1;
511
512         if (nf_h323_error_boundary(bs, 0, 0))
513                 return H323_ERROR_BOUND;

So AFAICS this LLM response is bunk.

Re: [net,06/11] netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case

[Guanni Qu]

Hi Florian,

Confirmed, the AI review is incorrect on all points.

The UNCO, SEMI, BYTE, and default cases all advance bs->cur without
a pre-read check, but each has nf_h323_error_boundary(bs, 0, 0)
immediately after (lines 358, 410, 483, 512). The pointer can
temporarily overshoot bs->end, but the check catches it before any
subsequent dereference.

The CONS case my patch fixes is different: get_uint(bs, len)
dereferences *bs->cur++ inline (lines 258, 262, 266), it reads
1-4 bytes from memory before any post-advance boundary check can
fire. That's the gap the patch closes.

Jenny

On Sat, Mar 14, 2026 at 11:31 AM Florian Westphal <fw@strlen.de> wrote:
>
> Jakub Kicinski <kuba@kernel.org> wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
>
> Jenny, could you please double-check?
> I don't think this report is right.
>
> > netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case
> >
> > This patch adds a boundary check in the CONS case of decode_int() to
> > prevent reading beyond the buffer after get_bits() returns a length
> > value. The fix validates that len bytes remain before calling
> > get_uint().
>
> SO far I can follow LLM response.
>
> > > diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
> > > index 62aa22a07876..c972e9488e16 100644
> > > --- a/net/netfilter/nf_conntrack_h323_asn1.c
> > > +++ b/net/netfilter/nf_conntrack_h323_asn1.c
> > > @@ -331,6 +331,8 @@ static int decode_int(struct bitstr *bs, const struct field_t *f,
> > >             if (nf_h323_error_boundary(bs, 0, 2))
> > >                     return H323_ERROR_BOUND;
> > >             len = get_bits(bs, 2) + 1;
> > > +           if (nf_h323_error_boundary(bs, len, 0))
> > > +                   return H323_ERROR_BOUND;
> > >             BYTE_ALIGN(bs);
> > >             if (base && (f->attr & DECODE)) {       /* timeToLive */
> > >                     unsigned int v = get_uint(bs, len) + f->lb;
> >
> > This patch fixes the CONS case, but the UNCO case in the same function
> > has the same vulnerability. In the UNCO case, get_len(bs) can return up
> > to 0x3FFF (16383 bytes), and the code advances bs->cur by that amount
> > without checking that len bytes remain:
> >
> >       case UNCO:
> >               BYTE_ALIGN(bs);
> >               if (nf_h323_error_boundary(bs, 2, 0))
> >                       return H323_ERROR_BOUND;
> >               len = get_len(bs);
> >               bs->cur += len;
> >               break;
> >
> > The check only validates 2 bytes for get_len() itself, not the
> > subsequent len bytes that will be skipped. Does this need the same fix
> > pattern?
>
> I don't think so.  Pointer can advance, but right after the swtich/case
> there is:
>
> 358         if (nf_h323_error_boundary(bs, 0, 0))
> 359                 return H323_ERROR_BOUND;
> 360         return H323_ERROR_NONE;
>
> So bs->cur can advance past cur->end, but then the function
> returns ERROR_BOUND.
>
> > The decode_bitstr() function has a similar issue. In the SEMI case, it
> > calls get_len(bs) which can return up to 0x3FFF bits, then advances the
> > buffer by len >> 3 bytes plus partial bits without checking that
> > sufficient buffer space exists:
> >
> >       case SEMI:
> >               if (nf_h323_error_boundary(bs, 2, 0))
> >                       return H323_ERROR_BOUND;
> >               len = get_len(bs);
>
> AFAICS we've checked get_len() doesn't cause oob access.
>
> >               break;
> >       }
> >
> >       bs->cur += len >> 3;
> >       bs->bit = len & 7;
> >
> > The check at the top only validates 2 bytes for the get_len() call. The
> > actual advance at the bottom can be up to 2047 bytes. Should this be
> > checked as well?
>
> AFAICS it is, right after there is:
> 410         if (nf_h323_error_boundary(bs, 0, 0))
> 411                 return H323_ERROR_BOUND;
>
> > The decode_octstr() function has the same pattern in both the SEMI case
> > and the default case:
> >
> >       case SEMI:
> >               BYTE_ALIGN(bs);
> >               if (nf_h323_error_boundary(bs, 2, 0))
> >                       return H323_ERROR_BOUND;
> >               len = get_len(bs) + f->lb;
> >               break;
> >       default:                /* 2 <= Range <= 255 */
> >               if (nf_h323_error_boundary(bs, 0, f->sz))
> >                       return H323_ERROR_BOUND;
> >               len = get_bits(bs, f->sz) + f->lb;
> >               BYTE_ALIGN(bs);
> >               break;
> >       }
> >
> >       bs->cur += len;
>
> Pointer increment, right after there is:
>
> 483         if (nf_h323_error_boundary(bs, 0, 0))
> 484                 return H323_ERROR_BOUND;
>
> > len bytes without checking. The existing checks only validate the bytes
> > needed to READ the length value, not the len bytes that will be
> > consumed. Should boundary checks be added after both break statements?
>
> AFAICS there are such checks... :-/
>
> > Similarly, decode_bmpstr() (which handles 2-byte character strings) has
> > the same issue in both the BYTE case and default case:
> >
> >       case BYTE:              /* Range == 256 */
> >               BYTE_ALIGN(bs);
> >               if (nf_h323_error_boundary(bs, 1, 0))
> >                       return H323_ERROR_BOUND;
> >               len = (*bs->cur++) + f->lb;
> >               break;
>
> We write to bs->cur, but AFAICS the write has been vetted for.
>
> >       default:                /* 2 <= Range <= 255 */
> >               if (nf_h323_error_boundary(bs, 0, f->sz))
> >                       return H323_ERROR_BOUND;
> >               len = get_bits(bs, f->sz) + f->lb;
> >               BYTE_ALIGN(bs);
> >               break;
> >       }
> >
> >       bs->cur += len << 1;
> >
> > After computing len from packet data plus f->lb, the function advances
> > by len << 1 bytes (doubling for 2-byte characters) without verifying
> > that len << 1 bytes are available. Should this also be checked?
>
> AFAICS it is...
>
> 510         bs->cur += len << 1;
> 511
> 512         if (nf_h323_error_boundary(bs, 0, 0))
> 513                 return H323_ERROR_BOUND;
>
> So AFAICS this LLM response is bunk.

Re: [net,06/11] netfilter: nf_conntrack_h323: fix OOB read in decode_int() CONS case

[Jakub Kicinski]

On Sat, 14 Mar 2026 15:16:37 -0700 Guanni Qu wrote:
> Confirmed, the AI review is incorrect on all points.
> 
> The UNCO, SEMI, BYTE, and default cases all advance bs->cur without
> a pre-read check, but each has nf_h323_error_boundary(bs, 0, 0)
> immediately after (lines 358, 410, 483, 512). The pointer can
> temporarily overshoot bs->end, but the check catches it before any
> subsequent dereference.
> 
> The CONS case my patch fixes is different: get_uint(bs, len)
> dereferences *bs->cur++ inline (lines 258, 262, 266), it reads
> 1-4 bytes from memory before any post-advance boundary check can
> fire. That's the gap the patch closes.

Thanks for checking / sorry for the noise ;(

Re: [PATCH net 04/11] netfilter: revert nft_set_rbtree: validate open interval overlap: manual merge

[Matthieu Baerts]

Hi Florian,

(+cc linux-next)

On 13/03/2026 16:06, Florian Westphal wrote:
> This reverts commit 648946966a08 ("netfilter: nft_set_rbtree: validate
> open interval overlap").
> 
> There have been reports of nft failing to laod valid rulesets after this
> patch was merged into -stable.
> 
> I can reproduce several such problem with recent nft versions, including
> nft 1.1.6 which is widely shipped by distributions.
> 
> We currently have little choice here.
> This commit can be resurrected at some point once the nftables fix that
> triggers the false overlap positive has appeared in common distros
> (see e83e32c8d1cd ("mnl: restore create element command with large batches" in
>  nftables.git).

FYI, we got a small conflict when merging 'net' in 'net-next' in the
MPTCP tree due to this patch applied in 'net':

  598adea720b9 ("netfilter: revert nft_set_rbtree: validate open interval overlap")

and this one from 'net-next':

  3aea466a4399 ("netfilter: nft_set_rbtree: don't disable bh when acquiring tree lock")

(...)

> diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
> index ee3d4f5b9ff7..fe8bd497d74a 100644
> --- a/net/netfilter/nft_set_rbtree.c
> +++ b/net/netfilter/nft_set_rbtree.c

(...)

> @@ -685,12 +640,8 @@ static int nft_rbtree_insert(const struct net *net, const struct nft_set *set,
>  		cond_resched();
>  
>  		write_lock_bh(&priv->lock);
> -		err = __nft_rbtree_insert(net, set, rbe, elem_priv, tstamp, last);
> +		err = __nft_rbtree_insert(net, set, rbe, elem_priv, tstamp);
>  		write_unlock_bh(&priv->lock);

The conflict was in the context: the patch in "net-next" stop disabling
bh, modifying the two write_(un)lock_bh() calls, while here the code
executed with the lock and just after is modified.

Rerere cache is available in [1]. The patch is attached below.

Cheers,
Matt

1: https://github.com/multipath-tcp/mptcp-upstream-rr-cache/commit/a0ec8d0

-------------------- 8< --------------------
diff --cc net/netfilter/nft_set_rbtree.c
index e42905376654,fe8bd497d74a..b7501b2b873e
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@@ -684,13 -639,9 +639,9 @@@ static int nft_rbtree_insert(const stru
  
  		cond_resched();
  
 -		write_lock_bh(&priv->lock);
 +		write_lock(&priv->lock);
- 		err = __nft_rbtree_insert(net, set, rbe, elem_priv, tstamp, last);
+ 		err = __nft_rbtree_insert(net, set, rbe, elem_priv, tstamp);
 -		write_unlock_bh(&priv->lock);
 +		write_unlock(&priv->lock);
- 
- 		if (nft_rbtree_interval_end(rbe))
- 			priv->start_rbe_cookie = 0;
- 
  	} while (err == -EAGAIN);
  
  	return err;
-------------------- 8< --------------------

-- 
Sponsored by the NGI0 Core fund.