[PATCH net 00/11] Netfilter/IPVS fixes for net

[PATCH net 00/11] Netfilter/IPVS fixes for net

[Pablo Neira Ayuso]

Hi,

The following patchset contains Netfilter/IPVS fixes for net:

1) IEEE1394 ARP payload contains no target hardware address in the
   ARP packet. Apparently, arp_tables was never updated to deal with
   IEEE1394 ARP properly. To deal with this, return no match in case
   the target hardware address selector is used, either for inverse or
   normal match. Moreover, arpt_mangle disallows mangling of the target
   hardware and IP address because, it is not worth to adjust the
   offset calculation to fix this, we suspect no users of arp_tables
   for this family.

2) Use list_del_rcu() to delete device hooks in nf_tables, this hook
   list is RCU protected, concurrent netlink dump readers can be
   walking on this list, fix it by adding a helper function and use it
   for consistency. From Florian Westphal.

3) Add list_splice_rcu(), this is useful for joining the local list of
   new device hooks to the RCU protected hook list in chain and
   flowtable. Reviewed by Paul E. McKenney.

4) Use list_splice_rcu() to publish the new device hooks in chain and
   flowtable to fix concurrent netlink dump traversal.

5) Add a new hook transaction object to track device hook deletions.
   The current approach moves device hooks to be deleted around during
   the preparation phase, this breaks concurrent RCU reader via netlink
   dump. This new hook transaction is combined with NFT_HOOK_REMOVE
   flag to annotate hooks for removal in the preparation phase.

6) xt_policy inbound policy check in strict mode can lead to
   out-of-bound access of the secpath array due to incorrect.
   The iteration over the secpath needs to be reversed in the inbound
   to check for the human readable policy, expecting inner in first
   position and outer in second position, the secpath from inbound
   actually stores outer in first position then in second position.
   From Jiexun Wang.

7) Fix possible zero shift in nft_bitwise triggering UBSAN splat,
   reject zero shift from control plane, from Kai Ma.

8) Replace simple_strtoul() in the conntrack SIP helper since it relies
   on nul-terminated strings. From Florian Westphal.

The IPVS fixes for recent net-next updates, from Julian Anastasov:

9) Fix several issues in the new /proc/net/ip_vs_status interface:
   prevent use-after-free by properly updating svc_table_changes
   during service deletion/flushing; bound bucket traversal and add
   loop detection to prevent infinite loops and overflows; use div_u64
   for safer 32-bit math; and restrict file permissions to 0440 to
   protect hash distribution info from non-root users.

10) Fix a race condition between the sysctl interface and the teardown
    of IPVS hash tables. Specifically, it prevents the system from
    trying to schedulework on a table that has already been destroyed.

11) Fix sleeping function called from invalid context bug. On RT
    kernels, standard spinlocks can sleep, but "bit locks" (used by the
    new hash table) do not. Holding a sleeping lock while a non-sleeping
    bit lock is held is illegal.

Please, pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-26-04-24

Thanks.

----------------------------------------------------------------

The following changes since commit 711987ba281fd806322a7cd244e98e2a81903114:

  netfilter: nfnetlink_osf: fix potential NULL dereference in ttl check (2026-04-20 23:45:44 +0200)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git tags/nf-26-04-24

for you to fetch changes up to b51edb039b1dbcdc83e00c31cf5887bd75486dcc:

  ipvs: fix the spin_lock usage for RT build (2026-04-24 20:09:57 +0200)

----------------------------------------------------------------
netfilter pull request 26-04-24

----------------------------------------------------------------
Florian Westphal (2):
      netfilter: nf_tables: use list_del_rcu for netlink hooks
      netfilter: nf_conntrack_sip: don't use simple_strtoul

Jiexun Wang (1):
      netfilter: xt_policy: fix strict mode inbound policy matching

Julian Anastasov (3):
      ipvs: fixes for the new ip_vs_status info
      ipvs: fix races around the conn_lfactor and svc_lfactor sysctl vars
      ipvs: fix the spin_lock usage for RT build

Kai Ma (1):
      netfilter: reject zero shift in nft_bitwise

Pablo Neira Ayuso (4):
      netfilter: arp_tables: fix IEEE1394 ARP payload parsing
      rculist: add list_splice_rcu() for private lists
      netfilter: nf_tables: join hook list via splice_list_rcu() in commit phase
      netfilter: nf_tables: add hook transactions for device deletions

 include/linux/rculist.h           |  29 ++++
 include/net/netfilter/nf_tables.h |  13 ++
 net/ipv4/netfilter/arp_tables.c   |  18 ++-
 net/ipv4/netfilter/arpt_mangle.c  |   8 +
 net/netfilter/ipvs/ip_vs_conn.c   |  71 ++++-----
 net/netfilter/ipvs/ip_vs_ctl.c    |  63 +++++---
 net/netfilter/nf_conntrack_sip.c  | 152 +++++++++++++-----
 net/netfilter/nf_nat_sip.c        |   1 +
 net/netfilter/nf_tables_api.c     | 314 +++++++++++++++++++++++++++-----------
 net/netfilter/nft_bitwise.c       |   3 +-
 net/netfilter/xt_policy.c         |   2 +-
 11 files changed, 494 insertions(+), 180 deletions(-)

[PATCH net 00/11] Netfilter/IPVS fixes for net

[Pablo Neira Ayuso]

Hi,

The following batch contains Netfilter fixes for net:

1) Expand existing ipset fix for bitmap sets to disallow comments
   updates from kernel-side adds, from Florian Westphal.

2) Drop flowtable reference if nf_ct_netns_get() fails, otherwise
   flowtable cannot ever be removed, from Aohan Mei.

3) nft_rbtree GC should collect end elements that contained in
   this transaction batch, new or deleted elements are never
   expired. From Weiming Shi.

4) Restrict nf_nat_bpf so it does not set unknown NF_NAT_MANIP_*
   values, from Fernando F. Mancera.

5) Flowtable GC must skip flows that are pending hardware updates,
   generalize the PENDING flag and use it to inhibit GC.

6) Restore flowtable with ieee80211 which broke due to a relatively
   recent commit, which was pulled in by -stable, causing a regression
   in 6.18 kernels.

And the following IPVS fixes:

1) Prevent buffer overflow in IPVS sync reported by sashiko, it
   should only be reproducible on very old 2.6.x kernels,
   from Julian Anastasov.

2) Fix accounting of cache entries in IPVS LBLC for destinations,
   from Julian Anastasov.

3) Limit IPVS cache growth for LBLCR and LBLC schedulers,
   from Zhiling Zou.

4) Restrict IP_VS_CONN_F_ONE_PACKET for normal connections,
   do not allow to use it with templates. Also from Julian.

5) Sanitize flags in IPVS sync messages received in the backup.
   From Julian Anastasov.

Please, pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-26-09-27

Thanks.

----------------------------------------------------------------

The following changes since commit 9c572a83037a7dcd653ba3a9cc468c16b857d0c9:

  net/sched: fix potential stack infoleak in em_text_dump() (2026-09-22 19:14:25 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-26-09-27

for you to fetch changes up to 5957f55e476000330f59193b607abcfc0e89d18a:

  netfilter: flowtable: restore ieee80211 forward path (2026-09-27 22:46:56 +0200)

----------------------------------------------------------------
netfilter pull request 26-09-27

----------------------------------------------------------------
Aohan Mei (1):
      netfilter: nft_flow_offload: drop flowtable reference on init error path

Fernando Fernandez Mancera (1):
      netfilter: bpf: reject invalid NAT manipulation types

Florian Westphal (1):
      netfilter: ipset: do not update comments from kernel-side adds

Julian Anastasov (4):
      ipvs: fix buffer overflow when sending sync messages
      ipvs: fix missing counter decrement in lblc
      ipvs: do not create invisible templates
      ipvs: filter some flags received in the backup server

Pablo Neira Ayuso (2):
      netfilter: flowtable: generalize pending status bit
      netfilter: flowtable: restore ieee80211 forward path

Weiming Shi (1):
      netfilter: nft_set_rbtree: skip transaction elements during GC

Zhiling Zou (1):
      ipvs: bound LBLCR and LBLC cache growth

 include/linux/netdevice.h               |  3 ++
 include/net/netfilter/nf_flow_table.h   |  2 +-
 net/mac80211/iface.c                    |  7 +++++
 net/netfilter/ipset/ip_set_bitmap_gen.h |  2 +-
 net/netfilter/ipvs/ip_vs_conn.c         |  3 ++
 net/netfilter/ipvs/ip_vs_lblc.c         |  4 +++
 net/netfilter/ipvs/ip_vs_lblcr.c        |  3 ++
 net/netfilter/ipvs/ip_vs_sync.c         | 56 ++++++++++++++++++++++++++-------
 net/netfilter/nf_flow_table_core.c      |  7 ++++-
 net/netfilter/nf_flow_table_offload.c   | 14 +++------
 net/netfilter/nf_flow_table_path.c      |  3 ++
 net/netfilter/nf_nat_bpf.c              |  3 ++
 net/netfilter/nf_nat_core.c             |  5 +--
 net/netfilter/nft_flow_offload.c        |  7 ++++-
 net/netfilter/nft_set_rbtree.c          |  2 ++
 net/sched/act_ct.c                      |  2 +-
 16 files changed, 95 insertions(+), 28 deletions(-)

[PATCH net 01/11] netfilter: ipset: do not update comments from kernel-side adds

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

'Fixes' commit stopped calling ip_set_init_comment() for hash types
from kernel-side-adds (xtables .. -j SET).  ip_set_init_comment() says:

   "The kadt functions don't use the comment extensions in any way."

But bitmap set type calls the function from kadt cb too.

While this appears to be safe (serialized via the set spinlock), it seems
better to not call the init function either, least of all to keep
behaviour consistent.

ip_set_list calls ip_set_init_comment() only from uadt cb, it can be
kept as-is.

This was triggered by yet another LLM review, hinting that the existing
rcu_dereference_protected() cannot be downgraded to only check if the
nfnl mutex is held.

Fixes: f30415929be8 ("netfilter: ipset: do not update comments from kernel-side hash adds")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipset/ip_set_bitmap_gen.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h b/net/netfilter/ipset/ip_set_bitmap_gen.h
index d6a7e6604542..ae376fa3e7a3 100644
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -159,7 +159,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 
 	if (SET_WITH_COUNTER(set))
 		ip_set_init_counter(ext_counter(x, set), ext);
-	if (SET_WITH_COMMENT(set))
+	if (SET_WITH_COMMENT(set) && !ext->target)
 		ip_set_init_comment(set, ext_comment(x, set), ext);
 	if (SET_WITH_SKBINFO(set))
 		ip_set_init_skbinfo(ext_skbinfo(x, set), ext);
-- 
2.47.3

[PATCH net 02/11] ipvs: fix buffer overflow when sending sync messages

[Pablo Neira Ayuso]

From: Julian Anastasov <ja@ssi.bg>

Sashiko reports for possible heap buffer overflow when
generating sync message for the v0 and v1 message formats.
We should read the cp->flags only once because another
CPU can concurrently set the SEQ_MASK between the two
reads.

Note that IPVS does not set the SEQ_MASK anymore for the
ip_vs_ftp.c helper starting from commit 7f1c40757951
("IPVS: make FTP work with full NAT support") (2.6.36+),
so the problem should not be reproducible.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Link: https://sashiko.dev/#/patchset/20260903004149.1037028-1-pablo%40netfilter.org
Link: https://sashiko.dev/#/patchset/20260909111338.44357-1-ja%40ssi.bg
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipvs/ip_vs_sync.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 5383aeafb0ae..dfa8487ec0c2 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -543,13 +543,15 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp,
 	struct ip_vs_sync_conn_v0 *s;
 	struct ip_vs_sync_buff *buff;
 	struct ipvs_master_sync_state *ms;
-	int id;
+	u32 flags, seq_mask;
 	unsigned int len;
+	int id;
 
 	if (unlikely(cp->af != AF_INET))
 		return;
+	flags = READ_ONCE(cp->flags);
 	/* Do not sync ONE PACKET */
-	if (cp->flags & IP_VS_CONN_F_ONE_PACKET)
+	if (flags & IP_VS_CONN_F_ONE_PACKET)
 		return;
 
 	if (!ip_vs_sync_conn_needed(ipvs, cp, pkts))
@@ -564,8 +566,8 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp,
 	id = select_master_thread_id(ipvs, cp);
 	ms = &ipvs->ms[id];
 	buff = ms->sync_buff;
-	len = (cp->flags & IP_VS_CONN_F_SEQ_MASK) ? FULL_CONN_SIZE :
-		SIMPLE_CONN_SIZE;
+	seq_mask = flags & IP_VS_CONN_F_SEQ_MASK;
+	len = seq_mask ? FULL_CONN_SIZE : SIMPLE_CONN_SIZE;
 	if (buff) {
 		m = (struct ip_vs_sync_mesg_v0 *) buff->mesg;
 		/* Send buffer if it is for v1 */
@@ -597,9 +599,9 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp,
 	s->caddr = cp->caddr.ip;
 	s->vaddr = cp->vaddr.ip;
 	s->daddr = cp->daddr.ip;
-	s->flags = htons(cp->flags & ~IP_VS_CONN_F_HASHED);
+	s->flags = htons(flags & ~IP_VS_CONN_F_HASHED);
 	s->state = htons(cp->state);
-	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
+	if (seq_mask) {
 		struct ip_vs_sync_conn_options *opt =
 			(struct ip_vs_sync_conn_options *)&s[1];
 		memcpy(opt, &cp->sync_conn_opt, sizeof(*opt));
@@ -635,6 +637,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 	int id;
 	__u8 *p;
 	unsigned int len, pe_name_len, pad;
+	u32 flags, seq_mask;
 
 	/* Handle old version of the protocol */
 	if (sysctl_sync_ver(ipvs) == 0) {
@@ -647,6 +650,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 sloop:
 	if (!ip_vs_sync_conn_needed(ipvs, cp, pkts))
 		goto control;
+	flags = READ_ONCE(cp->flags);
 
 	/* Sanity checks */
 	pe_name_len = 0;
@@ -674,7 +678,8 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 #endif
 		len = sizeof(struct ip_vs_sync_v4);
 
-	if (cp->flags & IP_VS_CONN_F_SEQ_MASK)
+	seq_mask = flags & IP_VS_CONN_F_SEQ_MASK;
+	if (seq_mask)
 		len += sizeof(struct ip_vs_sync_conn_options) + 2;
 
 	if (cp->pe_data_len)
@@ -720,7 +725,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 	/* Set message type  & copy members */
 	s->v4.type = (cp->af == AF_INET6 ? STYPE_F_INET6 : 0);
 	s->v4.ver_size = htons(len & SVER_MASK);	/* Version 0 */
-	s->v4.flags = htonl(cp->flags & ~IP_VS_CONN_F_HASHED);
+	s->v4.flags = htonl(flags & ~IP_VS_CONN_F_HASHED);
 	s->v4.state = htons(cp->state);
 	s->v4.protocol = cp->protocol;
 	s->v4.cport = cp->cport;
@@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 		s->v4.vaddr = cp->vaddr.ip;
 		s->v4.daddr = cp->daddr.ip;
 	}
-	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
+	if (seq_mask) {
 		*(p++) = IPVS_OPT_SEQ_DATA;
 		*(p++) = sizeof(struct ip_vs_sync_conn_options);
 		hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);
-- 
2.47.3

[PATCH net 03/11] netfilter: nft_flow_offload: drop flowtable reference on init error path

[Pablo Neira Ayuso]

From: Aohan Mei <henrymei@tencent.com>

nft_flow_offload_init() bumps the flowtable use count with
nft_use_inc() before calling nf_ct_netns_get().  When the latter
fails, the error is returned as-is and the reference is leaked.

The upper layers do not balance it either: nf_tables_newexpr()
clears expr->ops when the expression init callback fails, so the
nft_expr_more() iteration in nft_rule_expr_deactivate() and
nf_tables_rule_destroy() stops right before the failed expression
and its ->destroy callback, which would drop the reference, never
runs.

Each failed rule addition therefore leaks one flowtable reference
and the flowtable can no longer be removed: NFT_MSG_DELFLOWTABLE
keeps reporting -EBUSY even though no rule references it.

Save the nf_ct_netns_get() return value and undo the nft_use_inc()
when it fails, restoring the inc/dec pairing within
nft_flow_offload_init() itself.

Fixes: a3c90f7a2323 ("netfilter: nf_tables: flow offload expression")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_flow_offload.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 32b4281038dd..d3c5651dd699 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -160,6 +160,7 @@ static int nft_flow_offload_init(const struct nft_ctx *ctx,
 	struct nft_flow_offload *priv = nft_expr_priv(expr);
 	u8 genmask = nft_genmask_next(ctx->net);
 	struct nft_flowtable *flowtable;
+	int err;
 
 	if (!tb[NFTA_FLOW_TABLE_NAME])
 		return -EINVAL;
@@ -174,7 +175,11 @@ static int nft_flow_offload_init(const struct nft_ctx *ctx,
 
 	priv->flowtable = flowtable;
 
-	return nf_ct_netns_get(ctx->net, ctx->family);
+	err = nf_ct_netns_get(ctx->net, ctx->family);
+	if (err < 0)
+		nft_use_dec(&flowtable->use);
+
+	return err;
 }
 
 static void nft_flow_offload_deactivate(const struct nft_ctx *ctx,
-- 
2.47.3

[PATCH net 04/11] ipvs: fix missing counter decrement in lblc

[Pablo Neira Ayuso]

From: Julian Anastasov <ja@ssi.bg>

LBLC may delete cache entries for destinations that are
removed or overloaded and replace them with available ones.
But ip_vs_lblc_new() forgets to decrement the tbl->entries
counter after calling ip_vs_lblc_del(). This can lead to
increased shrinking of the cache with every new garbage
collection.

Fixes: 2f3d771a35fe ("ipvs: do not use dest after ip_vs_dest_put in LBLC")
Link: https://sashiko.dev/#/patchset/0bdd5abe9968ded7ca2b9cb6844ba83d94cc8d53.1787318053.git.zhilinz%40nebusec.ai
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipvs/ip_vs_lblc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index 693bcc82ccb7..a2b574904f13 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -203,6 +203,7 @@ ip_vs_lblc_new(struct ip_vs_lblc_table *tbl, const union nf_inet_addr *daddr,
 		if (en->dest == dest)
 			return en;
 		ip_vs_lblc_del(en);
+		atomic_dec(&tbl->entries);
 	}
 	en = kmalloc_obj(*en, GFP_ATOMIC);
 	if (!en)
-- 
2.47.3

[PATCH net 05/11] ipvs: bound LBLCR and LBLC cache growth

[Pablo Neira Ayuso]

From: Zhiling Zou <zhilinz@nebusec.ai>

ip_vs_lblcr_new() and ip_vs_lblc_new() create cache entries for
every previously unseen destination address. The table max_size only
tells the periodic collector to reclaim entries after the cache has
already exceeded the limit. It does not reclaim entries that the
attacker continues to use.

Reject new cache entries once either table reaches max_size * 3 / 2.
The extra headroom lets the periodic collector catch up while the
existing scheduler fallback continues to use the selected destination
when cache creation fails. New traffic therefore stays serviceable
without growing the tables further.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipvs/ip_vs_lblc.c  | 3 +++
 net/netfilter/ipvs/ip_vs_lblcr.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index a2b574904f13..55af77a56929 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -205,6 +205,9 @@ ip_vs_lblc_new(struct ip_vs_lblc_table *tbl, const union nf_inet_addr *daddr,
 		ip_vs_lblc_del(en);
 		atomic_dec(&tbl->entries);
 	}
+	if (atomic_read(&tbl->entries) >= tbl->max_size * 3 / 2)
+		return NULL;
+
 	en = kmalloc_obj(*en, GFP_ATOMIC);
 	if (!en)
 		return NULL;
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index f53f05ceea36..858393b1d2d1 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -363,6 +363,9 @@ ip_vs_lblcr_new(struct ip_vs_lblcr_table *tbl, const union nf_inet_addr *daddr,
 
 	en = ip_vs_lblcr_get(af, tbl, daddr);
 	if (!en) {
+		if (atomic_read(&tbl->entries) >= tbl->max_size * 3 / 2)
+			return NULL;
+
 		en = kmalloc_obj(*en, GFP_ATOMIC);
 		if (!en)
 			return NULL;
-- 
2.47.3

[PATCH net 06/11] ipvs: do not create invisible templates

[Pablo Neira Ayuso]

From: Julian Anastasov <ja@ssi.bg>

The IP_VS_CONN_F_ONE_PACKET flag was implemented for normal
connections. When conn template inherits this flag from
dest->conn_flags it will not be hashed. As result, we will
create new template for every new normal connection.

Fix it to allow one template to be used by many normal
connections.

Fixes: 26ec037f9841 ("IPVS: one-packet scheduling")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916231652.127456-1-pablo%40netfilter.org
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipvs/ip_vs_conn.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 6fa3e1dc534c..cb009208826f 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -1102,6 +1102,9 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
 	if (cp->protocol != IPPROTO_UDP)
 		conn_flags &= ~IP_VS_CONN_F_ONE_PACKET;
 	flags = cp->flags;
+	/* Only visible templates can control multiple connections */
+	if (flags & IP_VS_CONN_F_TEMPLATE)
+		conn_flags &= ~IP_VS_CONN_F_ONE_PACKET;
 	/* Bind with the destination and its corresponding transmitter */
 	if (flags & IP_VS_CONN_F_SYNC) {
 		/* Synced conns are hashed, so they can not get this flag */
-- 
2.47.3

[PATCH net 07/11] ipvs: filter some flags received in the backup server

[Pablo Neira Ayuso]

From: Julian Anastasov <ja@ssi.bg>

While the IPVS SYNC protocol is not secure by design
we can still protect the backup server from messages that
can wreak havoc.

This commit addresses problems from received connection flags
or their combinations. We now drop messages as follows:

1. the NO_CPORT+TEMPLATE combination allows lookups for normal
connections to hit template which can break in many ways.
While the master does not sync connections with NO_CPORT flag,
i.e. before they are established, we still accept NO_CPORT
without TEMPLATE.

2. ONE_PACKET: it is not sent by master, so we do not
expect it in backup. Before now it was ignored by
IP_VS_CONN_F_BACKUP_MASK for protocol v1 while protocol
v0 created connections that are not hashed and dropped
immediately. Better to apply the IP_VS_CONN_F_BACKUP_MASK
also to the flags from v0 messages for consistency with v1.

Fixes: 87375ab47cd0 ("[IPVS]: ip_vs_ftp breaks connections using persistence")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipvs/ip_vs_sync.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index dfa8487ec0c2..1a30817fbbaf 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -954,6 +954,21 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
 	ip_vs_conn_put(cp);
 }
 
+/* Check for incompatible flags */
+static bool ip_vs_sync_validate_flags(u32 flags)
+{
+	/* We do not expect NO_CPORT, especially to allow lookups
+	 * to hit templates
+	 */
+	if (flags & IP_VS_CONN_F_NO_CPORT) {
+		if (flags & IP_VS_CONN_F_TEMPLATE)
+			return false;
+	}
+	if (flags & IP_VS_CONN_F_ONE_PACKET)
+		return false;
+	return true;
+}
+
 /*
  *  Process received multicast message for Version 0
  */
@@ -977,8 +992,7 @@ static void ip_vs_process_message_v0(struct netns_ipvs *ipvs, const char *buffer
 			return;
 		}
 		s = (struct ip_vs_sync_conn_v0 *) p;
-		flags = ntohs(s->flags) | IP_VS_CONN_F_SYNC;
-		flags &= ~IP_VS_CONN_F_HASHED;
+		flags = ntohs(s->flags);
 		if (flags & IP_VS_CONN_F_SEQ_MASK) {
 			opt = (struct ip_vs_sync_conn_options *)&s[1];
 			p += FULL_CONN_SIZE;
@@ -991,6 +1005,13 @@ static void ip_vs_process_message_v0(struct netns_ipvs *ipvs, const char *buffer
 			p += SIMPLE_CONN_SIZE;
 		}
 
+		if (!ip_vs_sync_validate_flags(flags)) {
+			IP_VS_DBG(2, "BACKUP v0, Invalid flags 0x%X\n", flags);
+			continue;
+		}
+		flags &= IP_VS_CONN_F_BACKUP_MASK;
+		flags |= IP_VS_CONN_F_SYNC;
+
 		state = ntohs(s->state);
 		if (!(flags & IP_VS_CONN_F_TEMPLATE)) {
 			pp = ip_vs_proto_get(s->protocol);
@@ -1146,7 +1167,13 @@ static inline int ip_vs_proc_sync_conn(struct netns_ipvs *ipvs, __u8 *p, __u8 *m
 	}
 
 	/* Get flags and Mask off unsupported */
-	flags  = ntohl(s->v4.flags) & IP_VS_CONN_F_BACKUP_MASK;
+	flags = ntohl(s->v4.flags);
+	if (!ip_vs_sync_validate_flags(flags)) {
+		IP_VS_DBG(3, "BACKUP, Invalid flags 0x%X\n", flags);
+		retc = 25;
+		goto out;
+	}
+	flags &= IP_VS_CONN_F_BACKUP_MASK;
 	flags |= IP_VS_CONN_F_SYNC;
 	state = ntohs(s->v4.state);
 
-- 
2.47.3

[PATCH net 08/11] netfilter: nft_set_rbtree: skip transaction elements during GC

[Pablo Neira Ayuso]

From: Weiming Shi <bestswngs@gmail.com>

Since nft_set_commit_update() runs set commit callbacks before processing
NEWSETELEM transactions, nft_rbtree_gc_scan() can observe elements added by
the transaction being committed.

The scan records an interval end in rbe_end without checking the element's
transaction state. A later, unrelated expired start then moves both
elements to the expired list. The synchronous GC queue can free the new end
element before the transaction subsequently activates it, causing a
use-after-free.

Only consider elements that are fully active in both generations. This
keeps transaction-state elements out of the GC scan and preserves interval
pairing across skipped elements.

KASAN reports:

  BUG: KASAN: slab-use-after-free in nft_setelem_activate
  nft_setelem_activate net/netfilter/nf_tables_api.c:7047
  nf_tables_commit net/netfilter/nf_tables_api.c:11137

  Allocated by task 130:
  nft_set_elem_init net/netfilter/nf_tables_api.c:6794
  nft_add_set_elem net/netfilter/nf_tables_api.c:7523

  Freed by task 130:
  nft_trans_gc_trans_free net/netfilter/nf_tables_api.c:10506
  rcu_core kernel/rcu/tree.c:2919

Fixes: 1e3b9e1c77fe ("netfilter: nf_tables: call set ops .commit when building new ruleset blob")
Reported-by: <co+ee5e50ef2670e5f4@bugs.sh>
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_set_rbtree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 9894832281c4..12431b55752f 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -900,6 +900,8 @@ static void nft_rbtree_gc_scan(struct nft_set *set)
 		next = rb_next(node);
 
 		rbe = rb_entry(node, struct nft_rbtree_elem, node);
+		if (!nft_set_elem_active(&rbe->ext, NFT_GENMASK_ANY))
+			continue;
 
 		/* elements are reversed in the rbtree for historical reasons,
 		 * from highest to lowest value, that is why end element is
-- 
2.47.3

[PATCH net 09/11] netfilter: bpf: reject invalid NAT manipulation types

[Pablo Neira Ayuso]

From: Fernando Fernandez Mancera <fmancera@suse.de>

As bpf_ct_set_nat_info() is not validating the NAT manipulation type a
wrong value can be passed directly to nf_nat_setup_info(). This triggers
the WARN_ON() at nf_nat_setup_info() and if panic_on_warn isn't set,
then IPS_SRC_NAT_DONE is set without adding nat_bysource and conntrack
cleanup tries to unlink an uninitialized hlist node.

Fix this by checking that NAT manipulation type is correct before
calling nf_nat_setup_info(). In addition, if the WARN_ON is hit, return
NF_DROP instead of continuing with the processing to avoid similar
situations in the future.

Reported-by: VEGA <vega@nebusec.ai>
Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_nat_bpf.c  | 3 +++
 net/netfilter/nf_nat_core.c | 5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c
index f9dd85ccea01..7572b58c448b 100644
--- a/net/netfilter/nf_nat_bpf.c
+++ b/net/netfilter/nf_nat_bpf.c
@@ -39,6 +39,9 @@ __bpf_kfunc int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
 	if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
 		return -EINVAL;
 
+	if (manip != NF_NAT_MANIP_SRC && manip != NF_NAT_MANIP_DST)
+		return -EINVAL;
+
 	memset(&range, 0, sizeof(struct nf_nat_range2));
 	range.flags = NF_NAT_RANGE_MAP_IPS;
 	range.min_addr = *addr;
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index a4858c2b2d65..cc8e1e81006d 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -767,8 +767,9 @@ nf_nat_setup_info(struct nf_conn *ct,
 	if (nf_ct_is_confirmed(ct))
 		return NF_ACCEPT;
 
-	WARN_ON(maniptype != NF_NAT_MANIP_SRC &&
-		maniptype != NF_NAT_MANIP_DST);
+	if (WARN_ON(maniptype != NF_NAT_MANIP_SRC &&
+		    maniptype != NF_NAT_MANIP_DST))
+		return NF_DROP;
 
 	if (WARN_ON(nf_nat_initialized(ct, maniptype)))
 		return NF_DROP;
-- 
2.47.3

[PATCH net 10/11] netfilter: flowtable: generalize pending status bit

[Pablo Neira Ayuso]

Rename NF_FLOW_HW_PENDING to NF_FLOW_PENDING and use it to inhibit the
flowtable GC worker until pending hw offload work has been completed.

Apparently, nf_flow_offload_stats() can schedule work to retrieve stats
while the flow is being removed by GC.

And this bit can also be used in a follow up patch to disable GC until
the flow has been fully added in both directions.

Revert the reordering done in commit d644b23afe1e ("netfilter:
flowtable: publish HW_DEAD after worker is done") to prevent a race
between GC and hw offload handler.

Fixes: 2c8897953f3b ("netfilter: flowtable: Add pending bit for offload work")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_flow_table.h |  2 +-
 net/netfilter/nf_flow_table_core.c    |  7 ++++++-
 net/netfilter/nf_flow_table_offload.c | 14 +++++---------
 net/sched/act_ct.c                    |  2 +-
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index f2e2771f188f..5b611efaa3cd 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -183,10 +183,10 @@ enum nf_flow_flags {
 	NF_FLOW_DNAT,
 	NF_FLOW_CLOSING,
 	NF_FLOW_TEARDOWN,
+	NF_FLOW_PENDING,
 	NF_FLOW_HW,
 	NF_FLOW_HW_DYING,
 	NF_FLOW_HW_DEAD,
-	NF_FLOW_HW_PENDING,
 	NF_FLOW_HW_BIDIRECTIONAL,
 	NF_FLOW_HW_ESTABLISHED,
 };
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 934c6151f558..36bbc7be2f74 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -575,7 +575,12 @@ static void nf_flow_table_extend_ct_timeout(struct nf_conn *ct)
 static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table,
 				    struct flow_offload *flow, void *data)
 {
-	bool teardown = test_bit(NF_FLOW_TEARDOWN, &flow->flags);
+	bool teardown;
+
+	if (test_bit(NF_FLOW_PENDING, &flow->flags))
+		return;
+
+	teardown = test_bit(NF_FLOW_TEARDOWN, &flow->flags);
 
 	if (nf_flow_has_expired(flow) ||
 	    nf_ct_is_dying(flow->ct) ||
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 6757fd89c1f1..4365859220e6 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -995,6 +995,7 @@ static void flow_offload_work_del(struct flow_offload_work *offload)
 	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
 	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
 		flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
+	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
 }
 
 static void flow_offload_tuple_stats(struct flow_offload_work *offload,
@@ -1056,13 +1057,8 @@ static void flow_offload_work_handler(struct work_struct *work)
 		default:
 			WARN_ON_ONCE(1);
 	}
-
-	clear_bit(NF_FLOW_HW_PENDING, &offload->flow->flags);
-	if (offload->cmd == FLOW_CLS_DESTROY) {
-		/* Publish after the worker's last flow access. */
-		smp_mb__before_atomic();
-		set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
-	}
+	smp_mb__before_atomic();
+	clear_bit(NF_FLOW_PENDING, &offload->flow->flags);
 
 	kfree(offload);
 }
@@ -1089,12 +1085,12 @@ nf_flow_offload_work_alloc(struct nf_flowtable *flowtable,
 {
 	struct flow_offload_work *offload;
 
-	if (test_and_set_bit(NF_FLOW_HW_PENDING, &flow->flags))
+	if (test_and_set_bit(NF_FLOW_PENDING, &flow->flags))
 		return NULL;
 
 	offload = kmalloc_obj(struct flow_offload_work, GFP_ATOMIC);
 	if (!offload) {
-		clear_bit(NF_FLOW_HW_PENDING, &flow->flags);
+		clear_bit(NF_FLOW_PENDING, &flow->flags);
 		return NULL;
 	}
 
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 55f3521edb4c..626c9a5af0ef 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -289,7 +289,7 @@ static bool tcf_ct_flow_is_outdated(const struct flow_offload *flow)
 {
 	return test_bit(IPS_SEEN_REPLY_BIT, &flow->ct->status) &&
 	       test_bit(IPS_HW_OFFLOAD_BIT, &flow->ct->status) &&
-	       !test_bit(NF_FLOW_HW_PENDING, &flow->flags) &&
+	       !test_bit(NF_FLOW_PENDING, &flow->flags) &&
 	       !test_bit(NF_FLOW_HW_ESTABLISHED, &flow->flags);
 }
 
-- 
2.47.3

[PATCH net 11/11] netfilter: flowtable: restore ieee80211 forward path

[Pablo Neira Ayuso]

Before commit 871df5007eda ("netfilter: flowtable: bail out if forward
path cannot be discovered"), there was a fallback to set up a forward
path in case .ndo_fill_forward_path fails or DEV_PATH_MTK_WDMA was used.
Such fallback was used by commit d787a3e38f01 ("mac80211: add support
for .ndo_fill_forward_path").

One possibility is to handle DEV_PATH_MTK_WDMA from the flowtable
forward path discovery. However, this is only used internally by drivers
to retrieve mtk_wdma information to set up hardware offload. Felix
decided to use the .fill_forward_path interface for this purpose due to
the lack of a better interface at that time.

Add a new DEV_PATH_IEEE80211 path which is offered if the new ieee80211
flag is set on in the struct net_device_path_ctx to restore the
flowtable with a ieee80211 netdevice. Handle this new DEV_PATH_IEEE80211
path just like DEV_PATH_ETHERNET and DEV_PATH_DSA, ie. this is the last
netdevice in the stack.

This new ieee80211 flag is implicitly unset for mtk_ppe and airoha which
call dev_fill_forward_path() to retrieve a DEV_PATH_MTK_WDMA path.

Fixes: 871df5007eda ("netfilter: flowtable: bail out if forward path cannot be discovered")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netdevice.h          | 3 +++
 net/mac80211/iface.c               | 7 +++++++
 net/netfilter/nf_flow_table_path.c | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 87cafc932e9e..3cff2174dc03 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -887,6 +887,7 @@ enum net_device_path_type {
 	DEV_PATH_DSA,
 	DEV_PATH_MTK_WDMA,
 	DEV_PATH_TUN,
+	DEV_PATH_IEEE80211,
 };
 
 struct net_device_path {
@@ -953,6 +954,8 @@ struct net_device_path_ctx {
 		u16		id;
 		__be16		proto;
 	} vlan[NET_DEVICE_PATH_VLAN_MAX];
+
+	bool			ieee80211;
 };
 
 enum tc_setup_type {
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 889c32fd8de1..c5584435fde9 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1023,6 +1023,13 @@ static int ieee80211_netdev_fill_forward_path(struct net_device_path_ctx *ctx,
 	struct sta_info *sta;
 	int ret = -ENOENT;
 
+	if (ctx->ieee80211) {
+		path->type = DEV_PATH_IEEE80211;
+		path->dev = ctx->dev;
+		ctx->dev = NULL;
+		return 0;
+	}
+
 	sdata = IEEE80211_DEV_TO_SUB_IF(ctx->dev);
 	local = sdata->local;
 
diff --git a/net/netfilter/nf_flow_table_path.c b/net/netfilter/nf_flow_table_path.c
index 1e55644f2edb..d90013685bf1 100644
--- a/net/netfilter/nf_flow_table_path.c
+++ b/net/netfilter/nf_flow_table_path.c
@@ -53,6 +53,7 @@ static int nft_dev_fill_forward_path(const struct dst_entry *dst_cache,
 	struct net_device_path_ctx ctx = {
 		.dev = dev,
 		.ether_type = ether_type,
+		.ieee80211 = true,
 	};
 	struct neighbour *n;
 	u8 nud_state;
@@ -114,6 +115,7 @@ static int nft_dev_path_info(struct net_device_path_stack *stack,
 		path = &stack->path[i];
 		switch (path->type) {
 		case DEV_PATH_ETHERNET:
+		case DEV_PATH_IEEE80211:
 		case DEV_PATH_DSA:
 		case DEV_PATH_VLAN:
 		case DEV_PATH_PPPOE:
@@ -123,6 +125,7 @@ static int nft_dev_path_info(struct net_device_path_stack *stack,
 				memcpy(info->h_source, path->dev->dev_addr, ETH_ALEN);
 
 			if (path->type == DEV_PATH_ETHERNET ||
+			    path->type == DEV_PATH_IEEE80211 ||
 			    path->type == DEV_PATH_DSA)
 				break;
 
-- 
2.47.3

Re: [PATCH net 02/11] ipvs: fix buffer overflow when sending sync messages

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message for "ipvs: fix buffer overflow when sending
  sync messages" says the SEQ_MASK race "should not be reproducible",…

Pre-existing issues:
- [Medium] After the patch, the flags are a consistent snapshot, but the
  sequence data is still read without locking and without ordering…

--

ipvs: fix buffer overflow when sending sync messages

This commit changes ip_vs_sync_conn_v0() and ip_vs_sync_conn() to read
cp->flags once with READ_ONCE(). The same value is then used to size the
sync entry and to decide whether to emit the sequence options. This
prevents a heap overflow when another CPU sets IP_VS_CONN_F_SEQ_MASK
between the two reads.

Link: https://sashiko.dev/#/patchset/20260903004149.1037028-1-pablo%40netfilter.org
Link: https://sashiko.dev/#/patchset/20260909111338.44357-1-ja%40ssi.bg

> Note that IPVS does not set the SEQ_MASK anymore for the
> ip_vs_ftp.c helper starting from commit 7f1c40757951
> ("IPVS: make FTP work with full NAT support") (2.6.36+),
> so the problem should not be reproducible.

[Severity: Medium]
Is the "should not be reproducible" part accurate?

The ip_vs_ftp part is true. ip_vs_ftp_out() and ip_vs_ftp_in() always set
*diff = 0, so vs_seq_update() is never reached. However, the SEQ bits
still seem to have another writer, and received sync messages drive it.

In ip_vs_proc_conn(), an existing connection has its flags updated under
cp->lock:

net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
    ...
		flags &= IP_VS_CONN_F_BACKUP_UPD_MASK;
		flags |= cp->flags & ~IP_VS_CONN_F_BACKUP_UPD_MASK;
		cp->flags = flags;
    ...
}

IP_VS_CONN_F_BACKUP_UPD_MASK includes the SEQ bits:

#define IP_VS_CONN_F_BACKUP_UPD_MASK (IP_VS_CONN_F_INACTIVE | \
				      IP_VS_CONN_F_SEQ_MASK)

ip_vs_proc_sync_conn() masks the wire flags with IP_VS_CONN_F_BACKUP_MASK,
which also keeps the SEQ bits. It does not require an IPVS_OPT_SEQ_DATA
option to be present. So a peer's sync message can set or clear the SEQ
bits on an existing connection.

start_sync_thread() checks the master and backup roles separately and then
does ipvs->sync_state |= state, so one node can run both daemons. On such
a node:

CPU1 (backup kthread)
ip_vs_process_message()
  ip_vs_proc_sync_conn()
    ip_vs_proc_conn()
      cp->flags = flags;    /* SEQ bits toggled */

CPU2 (packet path, or ip_vs_conn_expire())
ip_vs_in()
  ip_vs_sync_conn()
    reads cp->flags to compute len, then again for IPVS_OPT_SEQ_DATA

Before this patch, that double read could size len without the options.
It could then write struct ip_vs_sync_conn_options past buff->end.

Sync traffic is unauthenticated; a later patch in this series calls it
not secure by design. Also, "ipvs: filter some flags received in the
backup server" does not filter the SEQ bits, so this writer is still
live at the end of the series.

Could the commit message drop the "should not be reproducible" claim and
mention the backup update path? The code change itself does look like it
closes the overflow.

> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 5383aeafb0aed..dfa8487ec0c20 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c

[ ... ]

> @@ -647,6 +650,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
>  sloop:
>  	if (!ip_vs_sync_conn_needed(ipvs, cp, pkts))
>  		goto control;
> +	flags = READ_ONCE(cp->flags);
>  
>  	/* Sanity checks */
>  	pe_name_len = 0;

[ ... ]

> @@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
>  		s->v4.vaddr = cp->vaddr.ip;
>  		s->v4.daddr = cp->daddr.ip;
>  	}
> -	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
> +	if (seq_mask) {
>  		*(p++) = IPVS_OPT_SEQ_DATA;
>  		*(p++) = sizeof(struct ip_vs_sync_conn_options);
>  		hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);

[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch. The flags
are now a consistent snapshot, but the sequence data is still read
without cp->lock. Nothing orders that read against READ_ONCE(cp->flags).
Can the master send stale or half-updated ip_vs_seq values here?

ip_vs_sync_conn() here only holds ipvs->sync_buff_lock. The same applies
to the memcpy() of cp->sync_conn_opt in ip_vs_sync_conn_v0().

On the backup side, ip_vs_proc_conn() sets the SEQ bits in cp->flags under
cp->lock and drops the lock. Only after that does it copy the sequence
data, with no lock held:

net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
    ...
		cp->flags = flags;
		spin_unlock_bh(&cp->lock);
    ...
	if (opt) {
		cp->in_seq = opt->in_seq;
		cp->out_seq = opt->out_seq;
	}
    ...
}

On a node running both the master and backup daemons, ip_vs_sync_conn()
could see seq_mask set and then send old or torn init_seq, delta and
previous_delta values. A peer with an app bound to that connection would
then apply the wrong TCP seq/ack adjustment after failover.

vs_seq_update() in ip_vs_app.c is another writer. It updates the sequence
data and flags under cp->lock, but the sync reader never takes that lock.
In-tree it is only reached with diff != 0, which ip_vs_ftp never
produces.

This does not corrupt memory, because the buffer size and the option
emission now both come from the same snapshot.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927220816.268206-2-pablo%40netfilter.org

Re: [PATCH net 03/11] netfilter: nft_flow_offload: drop flowtable reference on init error path

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] One sentence in the commit message names the wrong callback.

--

netfilter: nft_flow_offload: drop flowtable reference on init error path

In nft_flow_offload_init(), nft_use_inc() raises the flowtable use count
before nf_ct_netns_get() is called. If nf_ct_netns_get() fails, that count
is never dropped. The patch saves the return value of nf_ct_netns_get() and
calls nft_use_dec() on failure, so the inc/dec pairing is restored inside
the init callback.

> The upper layers do not balance it either: nf_tables_newexpr()
> clears expr->ops when the expression init callback fails, so the
> nft_expr_more() iteration in nft_rule_expr_deactivate() and
> nf_tables_rule_destroy() stops right before the failed expression
> and its ->destroy callback, which would drop the reference, never
> runs.

[Severity: Low]
This isn't a bug, but does this paragraph name the wrong callback? In
net/netfilter/nft_flow_offload.c, the ->destroy callback only drops the
conntrack netns reference:

static void nft_flow_offload_destroy(const struct nft_ctx *ctx,
				     const struct nft_expr *expr)
{
	nf_ct_netns_put(ctx->net, ctx->family);
}

The flowtable use count is dropped through ->deactivate instead.
nft_flow_offload_deactivate() calls nf_tables_deactivate_flowtable(), and
that function does the decrement:

net/netfilter/nf_tables_api.c:nf_tables_deactivate_flowtable() {
	switch (phase) {
	case NFT_TRANS_PREPARE_ERROR:
	case NFT_TRANS_PREPARE:
	case NFT_TRANS_ABORT:
	case NFT_TRANS_RELEASE:
		nft_use_dec(&flowtable->use);
	...
}

On the nf_tables_newrule() error path, the callback that gets skipped for
the failed expression is this one:

	nft_rule_expr_deactivate(&ctx, rule, NFT_TRANS_PREPARE_ERROR);

It is skipped because nf_tables_newexpr() cleared expr->ops, which ends the
nft_expr_more() walk.

The code change looks correct. Could the commit message say that the
skipped ->deactivate callback is what would drop the reference, rather than
->destroy? That would help anyone checking the fix against
nft_flow_offload_destroy(), for example during a stable backport.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927220816.268206-2-pablo%40netfilter.org

Re: [PATCH net 07/11] ipvs: filter some flags received in the backup server

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch does not stop forged sync messages from creating
  templates that normal-connection lookups can hit.

Pre-existing issues:
- [Low] This problem wasn't introduced by this patch, but
  ip_vs_sync_validate_flags() still accepts NO_CPORT on non-template…

--

ipvs: filter some flags received in the backup server

This commit adds ip_vs_sync_validate_flags() so the backup drops sync
entries that have both NO_CPORT and TEMPLATE set, or that have ONE_PACKET
set. It also applies IP_VS_CONN_F_BACKUP_MASK to v0 message flags, to
match the existing v1 handling.

> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index dfa8487ec0c20..1a30817fbbafe 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -954,6 +954,21 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
>  	ip_vs_conn_put(cp);
>  }
>  
> +/* Check for incompatible flags */
> +static bool ip_vs_sync_validate_flags(u32 flags)
> +{
> +	/* We do not expect NO_CPORT, especially to allow lookups
> +	 * to hit templates
> +	 */
> +	if (flags & IP_VS_CONN_F_NO_CPORT) {
> +		if (flags & IP_VS_CONN_F_TEMPLATE)
> +			return false;
> +	}

[Severity: High]
Does this fully stop sync messages from creating templates that
normal-connection lookups can hit?

Take a v0 or v1 entry with TEMPLATE set, NO_CPORT clear and a non-zero
cport. It passes this check and survives IP_VS_CONN_F_BACKUP_MASK. Then
ip_vs_proc_conn()->ip_vs_conn_new() creates it with the cport from the
message: ip_vs_conn_fill_param_sync() for v1, s->cport for v0.

With no PE data, the template is hashed on the same (proto, caddr, cport,
vaddr, vport) key a normal connection uses. __ip_vs_conn_in_get() does
not check IP_VS_CONN_F_TEMPLATE:

net/netfilter/ipvs/ip_vs_conn.c:__ip_vs_conn_in_get() {
    ...
				if (p->cport == cp->cport &&
				    p->vport == cp->vport && cp->af == p->af &&
				    ...
				    (!p->cport ^
				     (!(cp->flags & IP_VS_CONN_F_NO_CPORT))) &&
				    p->protocol == cp->protocol) {
    ...
}

When p->cport == cp->cport != 0 and NO_CPORT is clear, the XOR term is 1,
so the lookup returns the template.

A later non-template sync for that tuple then gets the template back from
ip_vs_conn_in_get() in ip_vs_proc_conn(). It either expires the template,
or it toggles dest->activeconns on the template's dest and overwrites
cp->flags, state, timeout and seq:

net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
    ...
		if ((cp->flags ^ flags) & IP_VS_CONN_F_INACTIVE &&
		    !(flags & IP_VS_CONN_F_TEMPLATE) && dest) {
			if (flags & IP_VS_CONN_F_INACTIVE)
				atomic_dec(&dest->activeconns);
			else
				atomic_inc(&dest->activeconns);
		}
    ...
}

For templates, ip_vs_unbind_dest() only decrements persistconns, so the
activeconns change is never undone. After failover, real client packets
for that tuple would also be handled by the template in the ip_vs_in()
path.

The master always builds templates with cport 0. Both
ip_vs_conn_fill_param_persist() callers in ip_vs_core.c pass a literal 0.

Should the backup also reject TEMPLATE entries with a non-zero cport when
there is no PE data?

[Severity: Low]
This is a pre-existing issue, not one introduced by this patch. NO_CPORT
without TEMPLATE is still accepted here, and nothing checks that the
synced cport is 0. Is that intended?

Other code assumes NO_CPORT means cport == 0. ip_vs_ftp creates NO_CPORT
connections with cport 0, and ip_vs_conn_fill_cport() uses !cp->cport to
mean the cport has not been filled in yet:

	if (cp->flags & IP_VS_CONN_F_NO_CPORT && (!cp->cport || by_me)) {

ip_vs_conn_new() creates a NO_CPORT entry with cport X != 0 and
increments ipvs->no_cport_conns[af]. ip_vs_conn_in_get() can never find
that entry. The exact lookup fails the XOR term in __ip_vs_conn_in_get(),
and the cport_zero_p retry fails the cport comparison.

So each refresh in ip_vs_proc_conn() would create another duplicate entry,
and each one lasts until its timeout. no_cport_conns also stays non-zero,
which forces the second lookup on every in-direction miss. The entry can
still be matched in the reply direction, because ip_vs_conn_out_get()
does not check NO_CPORT.

Would it make sense to also require cport == 0 when NO_CPORT is accepted
on a non-template entry?

> +	if (flags & IP_VS_CONN_F_ONE_PACKET)
> +		return false;
> +	return true;
> +}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927220816.268206-2-pablo%40netfilter.org

Re: [PATCH net 08/11] netfilter: nft_set_rbtree: skip transaction elements during GC

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch does not fix the stale end pairing in
  nft_rbtree_gc_scan() (net/netfilter/nft_set_rbtree.c).

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the insertion-
  time GC path has the same wrong-end pairing, and this commit leaves it…

--

netfilter: nft_set_rbtree: skip transaction elements during GC

This patch makes nft_rbtree_gc_scan() skip any element that is not fully
active in both generations. The goal is to keep elements added by the
transaction being committed out of the scan. Without that, a new end
element could be paired with an unrelated expired start and freed before
it is activated.

> diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
> index 9894832281c4b..12431b55752fb 100644
> --- a/net/netfilter/nft_set_rbtree.c
> +++ b/net/netfilter/nft_set_rbtree.c
> @@ -900,6 +900,8 @@ static void nft_rbtree_gc_scan(struct nft_set *set)
>  		next = rb_next(node);
>  
>  		rbe = rb_entry(node, struct nft_rbtree_elem, node);
> +		if (!nft_set_elem_active(&rbe->ext, NFT_GENMASK_ANY))
> +			continue;

[Severity: High]
The commit message says this change "preserves interval pairing across
skipped elements". Is that the case in nft_rbtree_gc_scan()?

rbe_end is only cleared after a GC move. When the scan reaches a fully
active start that has not expired, rbe_end still points at that start's
own end:

	if (nft_rbtree_interval_end(rbe)) {
		rbe_end = rbe;
		continue;
	}
	if (!__nft_set_elem_expired(&rbe->ext, tstamp))
		continue;

The new skip also leaves rbe_end unchanged. Suppose the scan then reaches
an expired start with no end directly above it. This code then moves the
other interval's live end to priv->expired, and nft_rbtree_gc_queue()
frees it later:

	if (rbe_end) {
		nft_rbtree_gc_elem_move(net, set, priv, rbe_end);
		rbe_end = NULL;
	}

Take the reported trigger shape: a live interval above, the new elements
skipped, then an expired start without an end. Here the patch seems to
change only which end gets taken. Before, the new end element was taken,
which caused the UAF. Now it is the older, fully active end of the
interval above.

Can an orphaned expired start like this be created with the stock nft
tool? Running nft destroy element on an interval whose start has expired
seems to be enough:

nf_tables_delsetelem()
  nft_rbtree_deactivate() on the start
    -> returns NULL for the expired start, and start_rbe_cookie is
       reset to 0
    -> the -ENOENT is ignored for NFT_MSG_DESTROYSETELEM
  nft_rbtree_deactivate() on the end
    -> nft_rbtree_deactivate_same_interval() returns true because
       start_rbe_cookie is 0, so the end is deactivated

nft_set_commit_update() then removes the end from the tree with
nft_trans_elems_remove(). That happens before set->ops->commit() runs
nft_rbtree_gc_scan(). A crafted start-only NEWSETELEM placed in a gap is
also accepted by __nft_rbtree_insert() and gives the same shape.

In that case the higher interval [S_hi, E_hi) loses E_hi.
nft_rbtree_commit() then rebuilds it as [S_hi, next start). If it is the
topmost interval, it becomes open-ended (to == NULL).

Once S_hi expires, the next scan takes the end of the next live interval
above it. A later nft delete element on [S_hi, E_hi) also fails with
ENOENT because E_hi is gone.

Would it make sense to also clear rbe_end when a fully active start has
not expired?

	if (!__nft_set_elem_expired(&rbe->ext, tstamp)) {
		rbe_end = NULL;
		continue;
	}

[ ... ]

[Severity: High]
This is a pre-existing issue, not one introduced by this patch. The
insertion-time GC path appears to have the same wrong-end pairing, and
this patch leaves it unchanged.

__nft_rbtree_insert() calls nft_rbtree_gc_elem() when it finds an
expired start that is active in the current generation. The rb_prev()
walk stops at the first fully active end, whatever interval that end
belongs to:

net/netfilter/nft_set_rbtree.c:nft_rbtree_gc_elem() {
    ...
	while (prev) {
		rbe_prev = rb_entry(prev, struct nft_rbtree_elem, node);
		if (nft_rbtree_interval_end(rbe_prev) &&
		    nft_set_elem_active(&rbe_prev->ext, NFT_GENMASK_ANY))
			break;

		prev = rb_prev(prev);
	}

	rbe_prev = NULL;
	if (prev) {
		rbe_prev = rb_entry(prev, struct nft_rbtree_elem, node);
		nft_rbtree_gc_elem_move(net, set, priv, rbe_prev);
	}
    ...
}

The comment says end intervals "are coupled with the interval start
element", but nothing checks that coupling.

Consider a tree that, walked from high to low keys, holds:

  E_w (active), S_w (live), E_x, S_x (expired)

E_x is either pending deletion in the same batch or already removed by an
earlier nft destroy element. When an insertion walk reaches S_x, can the
rb_prev() walk skip E_x and S_w and pick E_w?

If so, nft_rbtree_gc_elem_move() erases E_w, the live end of
[S_w, E_w), and queues it on priv->expired. The next commit frees it and
rebuilds S_w as a wider or open-ended interval.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927220816.268206-2-pablo%40netfilter.org

Re: [PATCH net 10/11] netfilter: flowtable: generalize pending status bit

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 0 (1) · Low: 1 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The new NF_FLOW_PENDING gate in nf_flow_offload_gc_step()
  (nf_flow_table_core.c:580) is a single relaxed test_bit() snapshot.
- [Low] flow_offload_work_handler() now releases the flow with
  smp_mb__before_atomic() followed by clear_bit(NF_FLOW_PENDING)…

Pre-existing issues:
- [High] nf_flow_table_free()
  (net/netfilter/nf_flow_table_core.c:775-788) does not wait for DESTROY…
- [Medium] The datapath can re-offload a flow that GC has already torn
  down and removed from hardware, and that rule is never deleted.
- [Low] In nf_flow_offload_gc_step()
  (net/netfilter/nf_flow_table_core.c:609-610), the last branch `else if…

--

netfilter: flowtable: generalize pending status bit

This renames NF_FLOW_HW_PENDING to NF_FLOW_PENDING. nf_flow_offload_gc_step()
now skips any flow that has hardware offload work pending. It also moves the
setting of NF_FLOW_HW_DEAD back into flow_offload_work_del(), reverting
d644b23afe1e, and clears the pending bit after a barrier at the end of
flow_offload_work_handler().

> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index 934c6151f558b..36bbc7be2f745 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -575,7 +575,12 @@ static void nf_flow_table_extend_ct_timeout(struct nf_conn *ct)
>  static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table,
>  				    struct flow_offload *flow, void *data)
>  {
> -	bool teardown = test_bit(NF_FLOW_TEARDOWN, &flow->flags);
> +	bool teardown;
> +
> +	if (test_bit(NF_FLOW_PENDING, &flow->flags))
> +		return;

[Severity: High]
Does this check actually stop GC from freeing a flow while offload work is
pending? The test_bit() takes one snapshot, and nothing ties it to the
flow_offload_del() call later in this function.

The datapath can still claim NF_FLOW_PENDING after this check.
flow_offload_refresh() is reached from nf_flow_offload_forward() and
tcf_ct_flow_table_lookup() on a flow that was looked up under RCU. It only
checks NF_FLOW_CLOSING and NF_FLOW_HW:

net/netfilter/nf_flow_table_core.c:flow_offload_refresh() {
    ...
	if (likely(!nf_flowtable_hw_offload(flow_table)) ||
	    test_bit(NF_FLOW_CLOSING, &flow->flags))
		return;

	if (test_bit(NF_FLOW_HW, &flow->flags))
		nf_flow_offload_refresh(flow_table, flow);
}

nf_flow_offload_refresh() then calls nf_flow_offload_work_alloc() with
FLOW_CLS_REPLACE. Its test_and_set_bit(NF_FLOW_PENDING) succeeds as soon as
the DESTROY worker has cleared the bit. TEARDOWN, HW_DYING and HW_DEAD are
never checked on this path.

Could this sequence happen?

CPU0 (datapath)                  GC / offload workqueue
flow lookup finds F
                                 gc_step(): teardown F
                                   nf_flow_offload_del(), queue DESTROY
                                 DESTROY worker:
                                   set_bit(NF_FLOW_HW_DEAD)
                                   clear_bit(NF_FLOW_PENDING)
                                 later gc_step():
                                   test_bit(NF_FLOW_PENDING) == 0
flow_offload_refresh(F)
  test_and_set_bit(PENDING)
  queue FLOW_CLS_REPLACE
                                   HW_DEAD set -> flow_offload_del()
                                     call_rcu(flow_offload_free_rcu)
leaves RCU read section
                                 flow_offload_free_rcu():
                                   nf_ct_put(flow->ct); kfree(flow);
                                 REPLACE worker:
                                   flow_offload_work_add() on freed F
                                   clear_bit(NF_FLOW_PENDING) on freed F

call_rcu() only waits for CPU0 to leave its RCU read section. It does not
wait for the work that CPU0 queued.

GC passes can run back to back through
nf_flow_table_cleanup()->flush_delayed_work() on successive NETDEV_DOWN
events. On PREEMPT_RT the reader can also be preempted for a long time.

The commit message says the bit is used "to inhibit the flowtable GC worker
until pending hw offload work has been completed" and "to prevent a race
between GC and hw offload handler". Is a re-check of TEARDOWN or HW_DYING
needed after the fully ordered test_and_set_bit() in
nf_flow_offload_work_alloc() (or in nf_flow_offload_refresh()) to close
this?

[Severity: Low]
Should this be test_bit_acquire()? flow_offload_work_handler() now does
smp_mb__before_atomic() followed by clear_bit(NF_FLOW_PENDING) as the
release side. The reader here is a relaxed test_bit(), though, and the
if/return control dependency only orders later stores, not later loads.

On weakly ordered CPUs such as arm64, GC can see PENDING already cleared by
a FLOW_CLS_STATS work and still read an old flow->timeout in
nf_flow_has_expired(). That would be the value from before
flow_offload_work_stats() stored:

	offload->flow->timeout = max_t(u64, offload->flow->timeout,
				       lastused + flow_offload_get_timeout(offload->flow));

In that case, could GC tear down a flow that the hardware has just reported
as active? The HW_DEAD read is in the same word as PENDING, so it is not
affected.

> +
> +	teardown = test_bit(NF_FLOW_TEARDOWN, &flow->flags);
>  
>  	if (nf_flow_has_expired(flow) ||
>  	    nf_ct_is_dying(flow->ct) ||

[Severity: Low]
This is a pre-existing issue, but the last branch of
nf_flow_offload_gc_step() still queues stats work without checking
NF_FLOW_HW_DYING:

	} else if (test_bit(NF_FLOW_HW, &flow->flags)) {
		nf_flow_offload_stats(flow_table, flow);
	}

Take a CLOSING flow whose hardware rule has already been destroyed
(HW_DYING and HW_DEAD set, TEARDOWN not set). Will GC keep queueing
FLOW_CLS_STATS work for a cookie the driver has already removed?

With this patch each of those works also sets NF_FLOW_PENDING, so GC skips
the flow until the work finishes. The stats result itself is harmless, but
it adds redundant driver calls and can delay teardown of the flow by up to
about one GC period. This behaviour dates back to commit fdbaf5163331.

> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index 6757fd89c1f1a..4365859220e6a 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -995,6 +995,7 @@ static void flow_offload_work_del(struct flow_offload_work *offload)
>  	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
>  	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
>  		flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
> +	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but can the datapath re-offload a
flow after this DESTROY work has removed it from hardware?

flow_offload_lookup() filters out TEARDOWN only at lookup time.
flow_offload_refresh() then checks only NF_FLOW_CLOSING and NF_FLOW_HW.
flow_offload_work_add() does not re-check TEARDOWN, HW_DYING or HW_DEAD
before it does this:

	err = flow_offload_rule_add(offload, flow_rule);
	if (err < 0)
		goto out;

	set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status);

Suppose a reader looks up the flow before GC sets TEARDOWN, and calls
refresh after this worker has set HW_DEAD and cleared PENDING. The REPLACE
work then re-installs the hardware rule.

Once the REPLACE work clears PENDING, the next nf_flow_offload_gc_step()
sees TEARDOWN, HW_DYING and HW_DEAD all set. It calls flow_offload_del()
without issuing another DESTROY.

Doesn't that leave a hardware rule for the 5-tuple, keyed by a cookie that
points into the freed flow_offload? That rule would keep forwarding traffic
outside conntrack and the ruleset until the flowtable or block is torn
down.

[ ... ]

> @@ -1089,12 +1085,12 @@ nf_flow_offload_work_alloc(struct nf_flowtable *flowtable,
>  {
>  	struct flow_offload_work *offload;
>  
> -	if (test_and_set_bit(NF_FLOW_HW_PENDING, &flow->flags))
> +	if (test_and_set_bit(NF_FLOW_PENDING, &flow->flags))
>  		return NULL;
>  
>  	offload = kmalloc_obj(struct flow_offload_work, GFP_ATOMIC);
>  	if (!offload) {
> -		clear_bit(NF_FLOW_HW_PENDING, &flow->flags);
> +		clear_bit(NF_FLOW_PENDING, &flow->flags);
>  		return NULL;
>  	}

[Severity: High]
This is a pre-existing issue, but what happens during flowtable teardown if
this GFP_ATOMIC allocation fails for a DESTROY request?

If it fails, nf_flow_offload_del() returns without setting NF_FLOW_HW_DYING
and nothing is queued. nf_flow_table_free() then continues with:

	nf_flow_table_gc_run(flow_table);
	nf_flow_table_offload_flush_cleanup(flow_table);
	rhashtable_destroy(&flow_table->rhashtable);

and nf_flow_table_offload_flush_cleanup() does:

	if (nf_flowtable_hw_offload(flowtable)) {
		flush_workqueue(nf_flow_offload_del_wq);
		nf_flow_table_gc_run(flowtable);
	}

The second gc_run now queues the DESTROY work on nf_flow_offload_del_wq,
and nothing flushes that workqueue afterwards. rhashtable_destroy() does not
free the entries, so the flow_offload and its ct and dst references are
leaked.

The callers free the flowtable straight away: kfree(flowtable) in
nf_tables_flowtable_destroy() and kfree(ct_ft) in
tcf_ct_flow_table_cleanup_work(). The queued work then runs:

flow_offload_work_handler()
  read_pnet(&offload->flowtable->net)
  flow_offload_work_del()
    flow_offload_tuple_del()
      nf_flow_offload_tuple()
        down_read(&flowtable->flow_block_lock)
        list_for_each_entry(block_cb, block_cb_list, list)

Isn't this a use-after-free of the nf_flowtable? If the allocation fails
again in the second pass, the flow is simply leaked.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927220816.268206-2-pablo%40netfilter.org

Re: [PATCH net 00/11] Netfilter/IPVS fixes for net

[Jakub Kicinski]

On Mon, 28 Sep 2026 00:08:05 +0200 Pablo Neira Ayuso wrote:
> The following batch contains Netfilter fixes for net:

I didn't spot anything obviously needing a respin in the AI feedback,
could you confirm that it's good as is? If you have to respin it'd be
good to remove the claim that patch 2 is a nop, Linus is onto us for
sending too many LLM-induced, low impact fixes.

Re: [PATCH net 02/11] ipvs: fix buffer overflow when sending sync messages

[Julian Anastasov]

[-- Attachment #1: Type: text/plain, Size: 6474 bytes --]


	Hello,

On Mon, 28 Sep 2026, netdev-bot+sashiko@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The commit message for "ipvs: fix buffer overflow when sending
>   sync messages" says the SEQ_MASK race "should not be reproducible",…
> 
> Pre-existing issues:
> - [Medium] After the patch, the flags are a consistent snapshot, but the
>   sequence data is still read without locking and without ordering…
> 
> --
> 
> ipvs: fix buffer overflow when sending sync messages
> 
> This commit changes ip_vs_sync_conn_v0() and ip_vs_sync_conn() to read
> cp->flags once with READ_ONCE(). The same value is then used to size the
> sync entry and to decide whether to emit the sequence options. This
> prevents a heap overflow when another CPU sets IP_VS_CONN_F_SEQ_MASK
> between the two reads.
> 
> Link: https://sashiko.dev/#/patchset/20260903004149.1037028-1-pablo%40netfilter.org
> Link: https://sashiko.dev/#/patchset/20260909111338.44357-1-ja%40ssi.bg
> 
> > Note that IPVS does not set the SEQ_MASK anymore for the
> > ip_vs_ftp.c helper starting from commit 7f1c40757951
> > ("IPVS: make FTP work with full NAT support") (2.6.36+),
> > so the problem should not be reproducible.
> 
> [Severity: Medium]
> Is the "should not be reproducible" part accurate?
> 
> The ip_vs_ftp part is true. ip_vs_ftp_out() and ip_vs_ftp_in() always set
> *diff = 0, so vs_seq_update() is never reached. However, the SEQ bits
> still seem to have another writer, and received sync messages drive it.
> 
> In ip_vs_proc_conn(), an existing connection has its flags updated under
> cp->lock:
> 
> net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
>     ...
> 		flags &= IP_VS_CONN_F_BACKUP_UPD_MASK;
> 		flags |= cp->flags & ~IP_VS_CONN_F_BACKUP_UPD_MASK;
> 		cp->flags = flags;
>     ...
> }
> 
> IP_VS_CONN_F_BACKUP_UPD_MASK includes the SEQ bits:
> 
> #define IP_VS_CONN_F_BACKUP_UPD_MASK (IP_VS_CONN_F_INACTIVE | \
> 				      IP_VS_CONN_F_SEQ_MASK)
> 
> ip_vs_proc_sync_conn() masks the wire flags with IP_VS_CONN_F_BACKUP_MASK,
> which also keeps the SEQ bits. It does not require an IPVS_OPT_SEQ_DATA
> option to be present. So a peer's sync message can set or clear the SEQ
> bits on an existing connection.
> 
> start_sync_thread() checks the master and backup roles separately and then
> does ipvs->sync_state |= state, so one node can run both daemons. On such
> a node:
> 
> CPU1 (backup kthread)
> ip_vs_process_message()
>   ip_vs_proc_sync_conn()
>     ip_vs_proc_conn()
>       cp->flags = flags;    /* SEQ bits toggled */
> 
> CPU2 (packet path, or ip_vs_conn_expire())
> ip_vs_in()
>   ip_vs_sync_conn()
>     reads cp->flags to compute len, then again for IPVS_OPT_SEQ_DATA
> 
> Before this patch, that double read could size len without the options.
> It could then write struct ip_vs_sync_conn_options past buff->end.
> 
> Sync traffic is unauthenticated; a later patch in this series calls it
> not secure by design. Also, "ipvs: filter some flags received in the
> backup server" does not filter the SEQ bits, so this writer is still
> live at the end of the series.
> 
> Could the commit message drop the "should not be reproducible" claim and
> mention the backup update path? The code change itself does look like it
> closes the overflow.
> 
> > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> > index 5383aeafb0aed..dfa8487ec0c20 100644
> > --- a/net/netfilter/ipvs/ip_vs_sync.c
> > +++ b/net/netfilter/ipvs/ip_vs_sync.c
> 
> [ ... ]
> 
> > @@ -647,6 +650,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
> >  sloop:
> >  	if (!ip_vs_sync_conn_needed(ipvs, cp, pkts))
> >  		goto control;
> > +	flags = READ_ONCE(cp->flags);
> >  
> >  	/* Sanity checks */
> >  	pe_name_len = 0;
> 
> [ ... ]
> 
> > @@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
> >  		s->v4.vaddr = cp->vaddr.ip;
> >  		s->v4.daddr = cp->daddr.ip;
> >  	}
> > -	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
> > +	if (seq_mask) {
> >  		*(p++) = IPVS_OPT_SEQ_DATA;
> >  		*(p++) = sizeof(struct ip_vs_sync_conn_options);
> >  		hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);
> 
> [Severity: Medium]
> This is a pre-existing issue, not one introduced by this patch. The flags
> are now a consistent snapshot, but the sequence data is still read
> without cp->lock. Nothing orders that read against READ_ONCE(cp->flags).
> Can the master send stale or half-updated ip_vs_seq values here?
> 
> ip_vs_sync_conn() here only holds ipvs->sync_buff_lock. The same applies
> to the memcpy() of cp->sync_conn_opt in ip_vs_sync_conn_v0().
> 
> On the backup side, ip_vs_proc_conn() sets the SEQ bits in cp->flags under
> cp->lock and drops the lock. Only after that does it copy the sequence
> data, with no lock held:
> 
> net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
>     ...
> 		cp->flags = flags;
> 		spin_unlock_bh(&cp->lock);
>     ...
> 	if (opt) {
> 		cp->in_seq = opt->in_seq;
> 		cp->out_seq = opt->out_seq;
> 	}
>     ...
> }
> 
> On a node running both the master and backup daemons, ip_vs_sync_conn()
> could see seq_mask set and then send old or torn init_seq, delta and
> previous_delta values. A peer with an app bound to that connection would
> then apply the wrong TCP seq/ack adjustment after failover.

	Agreed, this can be improved to take a snapshot of
flags and seqs together under lock. I'll request to drop
this version, it is not urgent to apply it.

> vs_seq_update() in ip_vs_app.c is another writer. It updates the sequence
> data and flags under cp->lock, but the sync reader never takes that lock.
> In-tree it is only reached with diff != 0, which ip_vs_ftp never
> produces.
> 
> This does not corrupt memory, because the buffer size and the option
> emission now both come from the same snapshot.
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927220816.268206-2-pablo%40netfilter.org

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net 07/11] ipvs: filter some flags received in the backup server

[Julian Anastasov]

[-- Attachment #1: Type: text/plain, Size: 5674 bytes --]


	Hello,

On Mon, 28 Sep 2026, netdev-bot+sashiko@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 · Medium: 0 · Low: 0 (1)

	This patch is fine, see below. It is another patch
by another author that adds the missing checks.

> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] The patch does not stop forged sync messages from creating
>   templates that normal-connection lookups can hit.
> 
> Pre-existing issues:
> - [Low] This problem wasn't introduced by this patch, but
>   ip_vs_sync_validate_flags() still accepts NO_CPORT on non-template…
> 
> --
> 
> ipvs: filter some flags received in the backup server
> 
> This commit adds ip_vs_sync_validate_flags() so the backup drops sync
> entries that have both NO_CPORT and TEMPLATE set, or that have ONE_PACKET
> set. It also applies IP_VS_CONN_F_BACKUP_MASK to v0 message flags, to
> match the existing v1 handling.
> 
> > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> > index dfa8487ec0c20..1a30817fbbafe 100644
> > --- a/net/netfilter/ipvs/ip_vs_sync.c
> > +++ b/net/netfilter/ipvs/ip_vs_sync.c
> > @@ -954,6 +954,21 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
> >  	ip_vs_conn_put(cp);
> >  }
> >  
> > +/* Check for incompatible flags */
> > +static bool ip_vs_sync_validate_flags(u32 flags)
> > +{
> > +	/* We do not expect NO_CPORT, especially to allow lookups
> > +	 * to hit templates
> > +	 */
> > +	if (flags & IP_VS_CONN_F_NO_CPORT) {
> > +		if (flags & IP_VS_CONN_F_TEMPLATE)
> > +			return false;
> > +	}
> 
> [Severity: High]
> Does this fully stop sync messages from creating templates that
> normal-connection lookups can hit?
> 
> Take a v0 or v1 entry with TEMPLATE set, NO_CPORT clear and a non-zero
> cport. It passes this check and survives IP_VS_CONN_F_BACKUP_MASK. Then
> ip_vs_proc_conn()->ip_vs_conn_new() creates it with the cport from the
> message: ip_vs_conn_fill_param_sync() for v1, s->cport for v0.
> 
> With no PE data, the template is hashed on the same (proto, caddr, cport,
> vaddr, vport) key a normal connection uses. __ip_vs_conn_in_get() does
> not check IP_VS_CONN_F_TEMPLATE:
> 
> net/netfilter/ipvs/ip_vs_conn.c:__ip_vs_conn_in_get() {
>     ...
> 				if (p->cport == cp->cport &&
> 				    p->vport == cp->vport && cp->af == p->af &&
> 				    ...
> 				    (!p->cport ^
> 				     (!(cp->flags & IP_VS_CONN_F_NO_CPORT))) &&
> 				    p->protocol == cp->protocol) {
>     ...
> }
> 
> When p->cport == cp->cport != 0 and NO_CPORT is clear, the XOR term is 1,
> so the lookup returns the template.
> 
> A later non-template sync for that tuple then gets the template back from
> ip_vs_conn_in_get() in ip_vs_proc_conn(). It either expires the template,
> or it toggles dest->activeconns on the template's dest and overwrites
> cp->flags, state, timeout and seq:
> 
> net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
>     ...
> 		if ((cp->flags ^ flags) & IP_VS_CONN_F_INACTIVE &&
> 		    !(flags & IP_VS_CONN_F_TEMPLATE) && dest) {
> 			if (flags & IP_VS_CONN_F_INACTIVE)
> 				atomic_dec(&dest->activeconns);
> 			else
> 				atomic_inc(&dest->activeconns);
> 		}
>     ...
> }
> 
> For templates, ip_vs_unbind_dest() only decrements persistconns, so the
> activeconns change is never undone. After failover, real client packets
> for that tuple would also be handled by the template in the ip_vs_in()
> path.
> 
> The master always builds templates with cport 0. Both
> ip_vs_conn_fill_param_persist() callers in ip_vs_core.c pass a literal 0.
> 
> Should the backup also reject TEMPLATE entries with a non-zero cport when
> there is no PE data?

	Above is solved with a pending patch
"ipvs: validate cport in received sync records".

> 
> [Severity: Low]
> This is a pre-existing issue, not one introduced by this patch. NO_CPORT
> without TEMPLATE is still accepted here, and nothing checks that the
> synced cport is 0. Is that intended?
> 
> Other code assumes NO_CPORT means cport == 0. ip_vs_ftp creates NO_CPORT
> connections with cport 0, and ip_vs_conn_fill_cport() uses !cp->cport to
> mean the cport has not been filled in yet:
> 
> 	if (cp->flags & IP_VS_CONN_F_NO_CPORT && (!cp->cport || by_me)) {
> 
> ip_vs_conn_new() creates a NO_CPORT entry with cport X != 0 and
> increments ipvs->no_cport_conns[af]. ip_vs_conn_in_get() can never find
> that entry. The exact lookup fails the XOR term in __ip_vs_conn_in_get(),
> and the cport_zero_p retry fails the cport comparison.
> 
> So each refresh in ip_vs_proc_conn() would create another duplicate entry,
> and each one lasts until its timeout. no_cport_conns also stays non-zero,
> which forces the second lookup on every in-direction miss. The entry can
> still be matched in the reply direction, because ip_vs_conn_out_get()
> does not check NO_CPORT.
> 
> Would it make sense to also require cport == 0 when NO_CPORT is accepted
> on a non-template entry?

	Above is solved with a pending patch
"ipvs: validate cport in received sync records".

> 
> > +	if (flags & IP_VS_CONN_F_ONE_PACKET)
> > +		return false;
> > +	return true;
> > +}
> > +
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927220816.268206-2-pablo%40netfilter.org
> 

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net 02/11] ipvs: fix buffer overflow when sending sync messages

[Paolo Abeni]

On 9/29/26 06:06, Julian Anastasov wrote:
>>> @@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
>>>   		s->v4.vaddr = cp->vaddr.ip;
>>>   		s->v4.daddr = cp->daddr.ip;
>>>   	}
>>> -	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
>>> +	if (seq_mask) {
>>>   		*(p++) = IPVS_OPT_SEQ_DATA;
>>>   		*(p++) = sizeof(struct ip_vs_sync_conn_options);
>>>   		hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);
>>
>> [Severity: Medium]
>> This is a pre-existing issue, not one introduced by this patch. The flags
>> are now a consistent snapshot, but the sequence data is still read
>> without cp->lock. Nothing orders that read against READ_ONCE(cp->flags).
>> Can the master send stale or half-updated ip_vs_seq values here?
>>
>> ip_vs_sync_conn() here only holds ipvs->sync_buff_lock. The same applies
>> to the memcpy() of cp->sync_conn_opt in ip_vs_sync_conn_v0().
>>
>> On the backup side, ip_vs_proc_conn() sets the SEQ bits in cp->flags under
>> cp->lock and drops the lock. Only after that does it copy the sequence
>> data, with no lock held:
>>
>> net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
>>      ...
>> 		cp->flags = flags;
>> 		spin_unlock_bh(&cp->lock);
>>      ...
>> 	if (opt) {
>> 		cp->in_seq = opt->in_seq;
>> 		cp->out_seq = opt->out_seq;
>> 	}
>>      ...
>> }
>>
>> On a node running both the master and backup daemons, ip_vs_sync_conn()
>> could see seq_mask set and then send old or torn init_seq, delta and
>> previous_delta values. A peer with an app bound to that connection would
>> then apply the wrong TCP seq/ack adjustment after failover.
> 
> 	Agreed, this can be improved to take a snapshot of
> flags and seqs together under lock. I'll request to drop
> this version, it is not urgent to apply it.
It looks like a respin of the PR is needed, I'll drop the revision from PW.

/P

Re: [PATCH net 00/11] Netfilter/IPVS fixes for net

[Pablo Neira Ayuso]

Hi Jakub, Paolo,

On Mon, Sep 28, 2026 at 07:11:20PM -0700, Jakub Kicinski wrote:
> On Mon, 28 Sep 2026 00:08:05 +0200 Pablo Neira Ayuso wrote:
> > The following batch contains Netfilter fixes for net:
> 
> I didn't spot anything obviously needing a respin in the AI feedback,
> could you confirm that it's good as is? If you have to respin it'd be
> good to remove the claim that patch 2 is a nop, Linus is onto us for
> sending too many LLM-induced, low impact fixes.

I would say yes too. LLM comments say:

- Patch 2/11 (ipvs): commit description could be improved (yes, there
  is always room for improvement in that regard but I think description
  is fair fine enough). There is a report on a pre-existing issue, but
  I think that can be addressed as a follow up.

- Patch 3/11 (netfilter): commit description could be improved again,
  but patch is good IMO.

- Patch 7/11 (ipvs): there's seem to be another path to abuse this
  code LLM found, I would address this as a follow up.

- Patch 8/11 (netfilter): refers to a pre-existing issue.
  It also refers to issues with reordering elements of the range,
  but this API really need elements in order to work fine, otherwise
  overlap detection will likely fire.

- Patch 10/11 (netfilter): refers to a pre-existing issues.
  nf_flow_offload_refresh() also needs to be disabled in pending
  work is enqueued. Also disable stats fetching for dying hw entries.

In particular, I am observing IPVS patches are getting stuck because
of reports of pre-existing issues. Sometimes you find two or three
things that need an adjustment, and you can start tackling one of the
aspects at a time (because addressing them all at once it not easy).
I think it will help Julian if he has a chance to address issues as
follow up, unless LLM reports something really sound and compelling
that can be classified as a blocker.

In that regard, my impression is that LLMs are a bit overwhelming
because they complain about one aspect that still needs to be
addressed. Not coming in this series, but I can see this is happening
too with Florian when he has been addressing some of the existing
issues with ipset hashtable resizing.

Oh well, and me, because most of the reports here seem to be like
pre-existing issues.

Just my two cents here, these folks are doing very useful work and
they (and me too) will just follow up on pre-existing issues.

Re: [PATCH net 02/11] ipvs: fix buffer overflow when sending sync messages

[Pablo Neira Ayuso]

Hi Paolo,

On Tue, Sep 29, 2026 at 10:19:55AM +0200, Paolo Abeni wrote:
> On 9/29/26 06:06, Julian Anastasov wrote:
> > > > @@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
> > > >   		s->v4.vaddr = cp->vaddr.ip;
> > > >   		s->v4.daddr = cp->daddr.ip;
> > > >   	}
> > > > -	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
> > > > +	if (seq_mask) {
> > > >   		*(p++) = IPVS_OPT_SEQ_DATA;
> > > >   		*(p++) = sizeof(struct ip_vs_sync_conn_options);
> > > >   		hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);
> > > 
> > > [Severity: Medium]
> > > This is a pre-existing issue, not one introduced by this patch. The flags
> > > are now a consistent snapshot, but the sequence data is still read
> > > without cp->lock. Nothing orders that read against READ_ONCE(cp->flags).
> > > Can the master send stale or half-updated ip_vs_seq values here?
> > > 
> > > ip_vs_sync_conn() here only holds ipvs->sync_buff_lock. The same applies
> > > to the memcpy() of cp->sync_conn_opt in ip_vs_sync_conn_v0().
> > > 
> > > On the backup side, ip_vs_proc_conn() sets the SEQ bits in cp->flags under
> > > cp->lock and drops the lock. Only after that does it copy the sequence
> > > data, with no lock held:
> > > 
> > > net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
> > >      ...
> > > 		cp->flags = flags;
> > > 		spin_unlock_bh(&cp->lock);
> > >      ...
> > > 	if (opt) {
> > > 		cp->in_seq = opt->in_seq;
> > > 		cp->out_seq = opt->out_seq;
> > > 	}
> > >      ...
> > > }
> > > 
> > > On a node running both the master and backup daemons, ip_vs_sync_conn()
> > > could see seq_mask set and then send old or torn init_seq, delta and
> > > previous_delta values. A peer with an app bound to that connection would
> > > then apply the wrong TCP seq/ack adjustment after failover.
> > 
> > 	Agreed, this can be improved to take a snapshot of
> > flags and seqs together under lock. I'll request to drop
> > this version, it is not urgent to apply it.
>
> It looks like a respin of the PR is needed, I'll drop the revision from PW.

Just wrote to Jakub with a summary on the LLM report.

I would take this PR as is if it is still possible.

If you feel strong about to need to respin this PR, that's also fine
with me, just let confirm where to go.

Thanks.

Re: [PATCH net 02/11] ipvs: fix buffer overflow when sending sync messages

[Paolo Abeni]

On 9/29/26 11:43, Pablo Neira Ayuso wrote:
> Hi Paolo,
> 
> On Tue, Sep 29, 2026 at 10:19:55AM +0200, Paolo Abeni wrote:
>> On 9/29/26 06:06, Julian Anastasov wrote:
>>>>> @@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
>>>>>    		s->v4.vaddr = cp->vaddr.ip;
>>>>>    		s->v4.daddr = cp->daddr.ip;
>>>>>    	}
>>>>> -	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
>>>>> +	if (seq_mask) {
>>>>>    		*(p++) = IPVS_OPT_SEQ_DATA;
>>>>>    		*(p++) = sizeof(struct ip_vs_sync_conn_options);
>>>>>    		hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);
>>>>
>>>> [Severity: Medium]
>>>> This is a pre-existing issue, not one introduced by this patch. The flags
>>>> are now a consistent snapshot, but the sequence data is still read
>>>> without cp->lock. Nothing orders that read against READ_ONCE(cp->flags).
>>>> Can the master send stale or half-updated ip_vs_seq values here?
>>>>
>>>> ip_vs_sync_conn() here only holds ipvs->sync_buff_lock. The same applies
>>>> to the memcpy() of cp->sync_conn_opt in ip_vs_sync_conn_v0().
>>>>
>>>> On the backup side, ip_vs_proc_conn() sets the SEQ bits in cp->flags under
>>>> cp->lock and drops the lock. Only after that does it copy the sequence
>>>> data, with no lock held:
>>>>
>>>> net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
>>>>       ...
>>>> 		cp->flags = flags;
>>>> 		spin_unlock_bh(&cp->lock);
>>>>       ...
>>>> 	if (opt) {
>>>> 		cp->in_seq = opt->in_seq;
>>>> 		cp->out_seq = opt->out_seq;
>>>> 	}
>>>>       ...
>>>> }
>>>>
>>>> On a node running both the master and backup daemons, ip_vs_sync_conn()
>>>> could see seq_mask set and then send old or torn init_seq, delta and
>>>> previous_delta values. A peer with an app bound to that connection would
>>>> then apply the wrong TCP seq/ack adjustment after failover.
>>>
>>> 	Agreed, this can be improved to take a snapshot of
>>> flags and seqs together under lock. I'll request to drop
>>> this version, it is not urgent to apply it.
>>
>> It looks like a respin of the PR is needed, I'll drop the revision from PW.
> 
> Just wrote to Jakub with a summary on the LLM report.
> 
> I would take this PR as is if it is still possible.
> 
> If you feel strong about to need to respin this PR, that's also fine
> with me, just let confirm where to go.
Uhm... I must admit that given the pressure we received from Linus on
shrinking/prevent from increasing the net PR size I think we are better
off dropping patch 2/11 entirely.

I'm sorry for the extra work, but the messaging from Linus was pretty
strong:

https://lore.kernel.org/netdev/CAHk-=wiSnTE9vBZ=5_v+3EEkdazCCbBM5YABRzRHUAeRdyd4Xw@mail.gmail.com/

Thanks,

Paolo

Re: [PATCH net 02/11] ipvs: fix buffer overflow when sending sync messages

[Pablo Neira Ayuso]

On Tue, Sep 29, 2026 at 11:55:11AM +0200, Paolo Abeni wrote:
> On 9/29/26 11:43, Pablo Neira Ayuso wrote:
> > Hi Paolo,
> > 
> > On Tue, Sep 29, 2026 at 10:19:55AM +0200, Paolo Abeni wrote:
> > > On 9/29/26 06:06, Julian Anastasov wrote:
> > > > > > @@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
> > > > > >    		s->v4.vaddr = cp->vaddr.ip;
> > > > > >    		s->v4.daddr = cp->daddr.ip;
> > > > > >    	}
> > > > > > -	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
> > > > > > +	if (seq_mask) {
> > > > > >    		*(p++) = IPVS_OPT_SEQ_DATA;
> > > > > >    		*(p++) = sizeof(struct ip_vs_sync_conn_options);
> > > > > >    		hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);
> > > > > 
> > > > > [Severity: Medium]
> > > > > This is a pre-existing issue, not one introduced by this patch. The flags
> > > > > are now a consistent snapshot, but the sequence data is still read
> > > > > without cp->lock. Nothing orders that read against READ_ONCE(cp->flags).
> > > > > Can the master send stale or half-updated ip_vs_seq values here?
> > > > > 
> > > > > ip_vs_sync_conn() here only holds ipvs->sync_buff_lock. The same applies
> > > > > to the memcpy() of cp->sync_conn_opt in ip_vs_sync_conn_v0().
> > > > > 
> > > > > On the backup side, ip_vs_proc_conn() sets the SEQ bits in cp->flags under
> > > > > cp->lock and drops the lock. Only after that does it copy the sequence
> > > > > data, with no lock held:
> > > > > 
> > > > > net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
> > > > >       ...
> > > > > 		cp->flags = flags;
> > > > > 		spin_unlock_bh(&cp->lock);
> > > > >       ...
> > > > > 	if (opt) {
> > > > > 		cp->in_seq = opt->in_seq;
> > > > > 		cp->out_seq = opt->out_seq;
> > > > > 	}
> > > > >       ...
> > > > > }
> > > > > 
> > > > > On a node running both the master and backup daemons, ip_vs_sync_conn()
> > > > > could see seq_mask set and then send old or torn init_seq, delta and
> > > > > previous_delta values. A peer with an app bound to that connection would
> > > > > then apply the wrong TCP seq/ack adjustment after failover.
> > > > 
> > > > 	Agreed, this can be improved to take a snapshot of
> > > > flags and seqs together under lock. I'll request to drop
> > > > this version, it is not urgent to apply it.
> > > 
> > > It looks like a respin of the PR is needed, I'll drop the revision from PW.
> > 
> > Just wrote to Jakub with a summary on the LLM report.
> > 
> > I would take this PR as is if it is still possible.
> > 
> > If you feel strong about to need to respin this PR, that's also fine
> > with me, just let confirm where to go.
>
> Uhm... I must admit that given the pressure we received from Linus on
> shrinking/prevent from increasing the net PR size I think we are better
> off dropping patch 2/11 entirely.
> 
> I'm sorry for the extra work, but the messaging from Linus was pretty
> strong:
> 
> https://lore.kernel.org/netdev/CAHk-=wiSnTE9vBZ=5_v+3EEkdazCCbBM5YABRzRHUAeRdyd4Xw@mail.gmail.com/

Yes, I read that. I tried to move what is less relevant to nf-next (if
you look at my nf-next PR, it's contains not so relevant fixes too).

 include/linux/netdevice.h               |  3 ++
 include/net/netfilter/nf_flow_table.h   |  2 +-
 net/mac80211/iface.c                    |  7 +++++
 net/netfilter/ipset/ip_set_bitmap_gen.h |  2 +-
 net/netfilter/ipvs/ip_vs_conn.c         |  3 ++
 net/netfilter/ipvs/ip_vs_lblc.c         |  4 +++
 net/netfilter/ipvs/ip_vs_lblcr.c        |  3 ++
 net/netfilter/ipvs/ip_vs_sync.c         | 56 ++++++++++++++++++++++++++-------
 net/netfilter/nf_flow_table_core.c      |  7 ++++-
 net/netfilter/nf_flow_table_offload.c   | 14 +++------
 net/netfilter/nf_flow_table_path.c      |  3 ++
 net/netfilter/nf_nat_bpf.c              |  3 ++
 net/netfilter/nf_nat_core.c             |  5 +--
 net/netfilter/nft_flow_offload.c        |  7 ++++-
 net/netfilter/nft_set_rbtree.c          |  2 ++
 net/sched/act_ct.c                      |  2 +-
 16 files changed, 95 insertions(+), 28 deletions(-)

Yes, this patch is larger in the diffstat. I can just move it to
nf-next if you prefer.

I will respin and send v2 today without this.

Thanks.

Re: [PATCH net 00/11] Netfilter/IPVS fixes for net

[Julian Anastasov]

	Hello,

On Tue, 29 Sep 2026, Pablo Neira Ayuso wrote:

> Hi Jakub, Paolo,
> 
> On Mon, Sep 28, 2026 at 07:11:20PM -0700, Jakub Kicinski wrote:
> > On Mon, 28 Sep 2026 00:08:05 +0200 Pablo Neira Ayuso wrote:
> > > The following batch contains Netfilter fixes for net:
> > 
> > I didn't spot anything obviously needing a respin in the AI feedback,
> > could you confirm that it's good as is? If you have to respin it'd be
> > good to remove the claim that patch 2 is a nop, Linus is onto us for
> > sending too many LLM-induced, low impact fixes.
> 
> I would say yes too. LLM comments say:
> 
> - Patch 2/11 (ipvs): commit description could be improved (yes, there
>   is always room for improvement in that regard but I think description
>   is fair fine enough). There is a report on a pre-existing issue, but
>   I think that can be addressed as a follow up.

	I already have v2 for 2/11, now as 2 patches:

https://sashiko.dev/#/patchset/20260929113436.44306-1-ja%40ssi.bg

	But better to leave it for next week, it is a low
priority fix. As usually, if we stop 1 patch, it is replaced
by 2 for the next round :)

> - Patch 3/11 (netfilter): commit description could be improved again,
>   but patch is good IMO.
> 
> - Patch 7/11 (ipvs): there's seem to be another path to abuse this
>   code LLM found, I would address this as a follow up.
> 
> - Patch 8/11 (netfilter): refers to a pre-existing issue.
>   It also refers to issues with reordering elements of the range,
>   but this API really need elements in order to work fine, otherwise
>   overlap detection will likely fire.
> 
> - Patch 10/11 (netfilter): refers to a pre-existing issues.
>   nf_flow_offload_refresh() also needs to be disabled in pending
>   work is enqueued. Also disable stats fetching for dying hw entries.
> 
> In particular, I am observing IPVS patches are getting stuck because
> of reports of pre-existing issues. Sometimes you find two or three
> things that need an adjustment, and you can start tackling one of the
> aspects at a time (because addressing them all at once it not easy).
> I think it will help Julian if he has a chance to address issues as
> follow up, unless LLM reports something really sound and compelling
> that can be classified as a blocker.

	I don't find the LLM reviews wrong, so I don't
complain :) I'll note if a followup is preferred but in
most of the time the patches need to be corrected. I try
to post different patches for the different problems but
this is also a load for the other maintainers. I hope
I'll improve with the time but it is true that we are
overloaded. The only hope is that the bug-report rate will
slowdown with the time...

Regards

--
Julian Anastasov <ja@ssi.bg>