Netdev List
 help / color / mirror / Atom feed
* [PATCH net 00/14] pull request: batman-adv 2026-10-30
@ 2026-10-30 13:50 Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure Simon Wunderlich
                   ` (13 more replies)
  0 siblings, 14 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Simon Wunderlich

Dear net maintainers,

here is a bugfix pull request of batman-adv to go into net. This
pull request supersedes batadv-net-pullrequest-20261026 [1]. The
following changes were made (I marked the changed patches with v2):

* "Close OGM aggregation": dropped the Assisted-by tag and documented
  the new meaning of num_packets

* "tt: only transmit local ROAM flag together with DEL": clarified the
  commit message and noted that batadv_tt_global_purge_local() is not
  addressed

* "tt: only update flags for TT events": corrected the Fixes tag

* "tt: don't drop local events queued during TVLV update": moved the
  atomic snapshot of the changes into
  batadv_tt_local_commit_changes_nolock() so the TTVN decision covers
  the same events. If the TVLV allocation fails, the last changeset is
  now dropped only when the TTVN was incremented

* "tt: limit local table to single TVLV response size": Fixes now
  points to the original commit, and the patch is marked
  stable+noautosel because it is relevant mostly for the over-allocation
  fix 85fb027f611b

* new (patches were queued for later but Sashiko complained about
  problems which would be solved by them)

  - "tt: clear ROAM flag when re-adding a pending local client"
  - "tt: send ROAM_ADV on each roamed back client"
  - "tt: ignore ROAM flag of received ADD changes"

* new: "mcast: validate tracker padding before forwarding"

Please pull or let me know of any problem!

Thank you,
      Simon

[1] e-mail subject was: [PATCH net 00/10] pull request: batman-adv 2026-10-26
    reference: https://lore.kernel.org/r/20261026085309.1074911-1-sw@simonwunderlich.de

The following changes since commit d1807b20d548171d40c5d21a7cf22830ea8f348e:

  batman-adv: use assign_bit() where applicable (2026-09-27 19:03:34 +0200)

are available in the Git repository at:

  https://git.open-mesh.org/batadv.git tags/batadv-net-pullrequest-20261030

for you to fetch changes up to a137095419b4be0b204a11af4f1cea872ea262e9:

  batman-adv: tt: limit local table to single TVLV response size (2026-10-30 10:11:27 +0100)

----------------------------------------------------------------
Here are a few batman-adv bugfixes:

- avoid WARN_ON for orig_ifinfo allocation failure, by Deepanshu Kartikey

- close OGM aggregation before transmission, by Chengfeng Ye

- mcast: validate tracker padding before forwarding, by Kyle Zeng

The remaining bugfixes are authored by Sven Eckelmann:

- tpmeter: fix u32 truncation of reported bytes

- dat: initialize orig_node of unfilled candidate slots

- fix skb leak when hard_iface is being released

- stop originator purge worker before freeing tt

- tt: fix flag and event handling (6 patches)

- tt: limit local table to single TVLV response size

----------------------------------------------------------------
Chengfeng Ye (1):
      batman-adv: Close OGM aggregation before transmission

Deepanshu Kartikey (1):
      batman-adv: avoid WARN_ON for orig_ifinfo allocation failure

Kyle Zeng (1):
      batman-adv: mcast: validate tracker padding before forwarding

Sven Eckelmann (11):
      batman-adv: tpmeter: fix u32 truncation of reported bytes
      batman-adv: dat: initialize orig_node of unfilled candidate slots
      batman-adv: fix skb leak when hard_iface is being released
      batman-adv: stop originator purge worker before freeing TT
      batman-adv: tt: only transmit local ROAM flag together with DEL
      batman-adv: tt: clear ROAM flag when re-adding a pending local client
      batman-adv: tt: send ROAM_ADV on each roamed back client
      batman-adv: tt: ignore ROAM flag of received ADD changes
      batman-adv: tt: only update flags for TT events
      batman-adv: tt: don't drop local events queued during TVLV update
      batman-adv: tt: limit local table to single TVLV response size

 net/batman-adv/bat_iv_ogm.c            |   7 +-
 net/batman-adv/distributed-arp-table.c |   1 +
 net/batman-adv/main.c                  |  12 +-
 net/batman-adv/multicast_forw.c        |   5 +-
 net/batman-adv/tp_meter.c              |   2 +-
 net/batman-adv/translation-table.c     | 215 ++++++++++++++++++++++-----------
 net/batman-adv/types.h                 |   8 +-
 7 files changed, 174 insertions(+), 76 deletions(-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
@ 2026-10-30 13:50 ` Simon Wunderlich
  2026-10-30 13:54   ` netdev-bot+sinfo
  2026-10-30 22:00   ` patchwork-bot+netdevbpf
  2026-10-30 13:50 ` [PATCH net v2 02/14] batman-adv: Close OGM aggregation before transmission Simon Wunderlich
                   ` (12 subsequent siblings)
  13 siblings, 2 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Deepanshu Kartikey,
	syzbot+a079f084653164a652d1, Sven Eckelmann, Simon Wunderlich

From: Deepanshu Kartikey <kartikey406@gmail.com>

batadv_orig_ifinfo_new() returns NULL only when its GFP_ATOMIC
allocation fails, which is a normal runtime condition and not a bug.
batadv_v_ogm_forward() already handles this the same way, simply
checking for NULL without a WARN_ON.

Do the same in batadv_iv_ogm_update_seqnos() to avoid a spurious
WARN_ON splat when the allocation fails (e.g. under fault injection).

Fixes: 7351a4822d42 ("batman-adv: split out router from orig_node")
Reported-by: syzbot+a079f084653164a652d1@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a079f084653164a652d1
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bat_iv_ogm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 89b499f7a37dd..158d2b8f04036 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1410,7 +1410,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 		return BATADV_NO_DUP;
 
 	orig_ifinfo = batadv_orig_ifinfo_new(orig_node, if_outgoing);
-	if (WARN_ON(!orig_ifinfo)) {
+	if (!orig_ifinfo) {
 		batadv_orig_node_put(orig_node);
 		return 0;
 	}
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net v2 02/14] batman-adv: Close OGM aggregation before transmission
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure Simon Wunderlich
@ 2026-10-30 13:50 ` Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net 03/14] batman-adv: mcast: validate tracker padding before forwarding Simon Wunderlich
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Chengfeng Ye, stable, Sven Eckelmann,
	Simon Wunderlich

From: Chengfeng Ye <nicoyip.dev@gmail.com>

The OGM send worker leaves its forwarding packet on forw_bat_list until
after batadv_iv_ogm_emit() returns. Aggregation holds forw_bat_list_lock,
but emission reads and clones the packet without that lock.

CPU 0 can therefore start emitting a queued packet while CPU 1 takes the
list lock, finds the same packet and appends another OGM. The sender can
observe the new packet length before the corresponding direct-link flag
is set, or clone the skb while its length and payload are being updated.
This can transmit an OGM with incorrect flags or inconsistent data.

KCSAN reported:

  BUG: KCSAN: data-race in batadv_iv_ogm_queue_add / batadv_iv_send_outstanding_bat_ogm_packet
  write to 0xffff888100fedcf8 of 2 bytes by interrupt on cpu 1:
  read to 0xffff888100fedcf8 of 2 bytes by task 70 on cpu 2:
  value changed: 0x00c0 -> 0x00d8

Mark the aggregate as full under forw_bat_list_lock before emission. This
waits for any ongoing append and prevents further aggregation. Emission
uses packet_len to walk the OGMs, so all queued packets are still sent.
Keep the packet on the list so interface purging can find the worker,
wait for it to finish and free the packet.

Fixes: 9b4aec647a92 ("batman-adv: fix rare race conditions on interface removal")
Cc: stable@vger.kernel.org # 4.10.x: 434becf57bdc ("batman-adv: Limit number of aggregated packets directly")
Cc: stable@vger.kernel.org # 4.10.x
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
[ sven: drop assisted tag, document new meaning of num_packets ]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bat_iv_ogm.c | 5 +++++
 net/batman-adv/types.h      | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 158d2b8f04036..85e99506a53f5 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1909,6 +1909,11 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
 		goto out;
 	}
 
+	/* Mark aggregate as full before forcing emit. */
+	spin_lock_bh(&bat_priv->forw_bat_list_lock);
+	forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
+	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
+
 	batadv_iv_ogm_emit(forw_packet);
 
 	/* we have to have at least one packet in the queue to determine the
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index c19caa84e6920..b3e6dbb360dae 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -2047,7 +2047,13 @@ struct batadv_forw_packet {
 	/** @direct_link_flags: direct link flags for aggregated OGM packets */
 	DECLARE_BITMAP(direct_link_flags, BATADV_MAX_AGGREGATION_PACKETS);
 
-	/** @num_packets: counter for aggregated OGMv1 packets */
+	/**
+	 * @num_packets: counter for aggregated OGMv1 packets. Set to
+	 *  BATADV_MAX_AGGREGATION_PACKETS (independent of the real number of
+	 *  aggregated packets) by the send worker to prevent further
+	 *  aggregation while the packet is emitted. It must therefore not be
+	 *  used to iterate over the aggregated packets
+	 */
 	u8 num_packets;
 
 	/** @delayed_work: work queue callback item for packet sending */
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net 03/14] batman-adv: mcast: validate tracker padding before forwarding
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net v2 02/14] batman-adv: Close OGM aggregation before transmission Simon Wunderlich
@ 2026-10-30 13:50 ` Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net 04/14] batman-adv: tpmeter: fix u32 truncation of reported bytes Simon Wunderlich
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Kyle Zeng, stable, Sven Eckelmann,
	Simon Wunderlich

From: Kyle Zeng <kylebot@openai.com>

A multicast tracker with an even number of destinations needs two
padding bytes after its destination list. batadv_mcast_forw_packet()
currently only checks that the count field and the addresses fit in
the tracker TVLV.

If an unpadded even tracker is compacted to an odd number of
destinations, batadv_mcast_forw_shrink_update_headers() includes those
missing padding bytes in the amount to remove. The subsequent memmove
can then write two bytes past the old packet tail. With no tailroom,
this corrupts skb_shared_info.

Include the required padding in the existing tracker length check.
This rejects the malformed tracker before either received or locally
generated packets can reach the compactor.

Fixes: 2dfe644a1ce0 ("batman-adv: mcast: shrink tracker packet after scrubbing")
Cc: stable@vger.kernel.org
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/multicast_forw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/multicast_forw.c b/net/batman-adv/multicast_forw.c
index ac97cd8b81dc9..9ffed1f237b6b 100644
--- a/net/batman-adv/multicast_forw.c
+++ b/net/batman-adv/multicast_forw.c
@@ -950,8 +950,11 @@ static int batadv_mcast_forw_packet(struct batadv_priv *bat_priv,
 
 	dest = (u8 *)mcast_tracker + sizeof(*mcast_tracker);
 
-	/* check if full tracker tvlv is within skb length */
+	/* check if full tracker tvlv, including padding, is within skb length */
 	tvlv_len = sizeof(*mcast_tracker) + ETH_ALEN * num_dests;
+	if (!(num_dests % 2))
+		tvlv_len += 2;
+
 	if (tvlv_len > skb_network_header_len(skb))
 		return -EINVAL;
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net 04/14] batman-adv: tpmeter: fix u32 truncation of reported bytes
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (2 preceding siblings ...)
  2026-10-30 13:50 ` [PATCH net 03/14] batman-adv: mcast: validate tracker padding before forwarding Simon Wunderlich
@ 2026-10-30 13:50 ` Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net 05/14] batman-adv: dat: initialize orig_node of unfilled candidate slots Simon Wunderlich
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable, Sashiko,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

When more than 4 GB are transferred by the throughput meter,
batadv_tp_batctl_notify() will cut of the upper 32 bit of the u64 counter
and thus under-report the actual transferred bytes.

The data type of the temporary storage variable must therefore still
continue to use u64.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-08bb3be4-6c6a-452d-b4e4-606940c6cfa5
Fixes: 33a3bb4a3345 ("batman-adv: throughput meter implementation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/tp_meter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index 022eb13e8ec47..56feb02d2b7ef 100644
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -221,7 +221,7 @@ static void batadv_tp_batctl_notify(enum batadv_tp_meter_reason reason,
 				    unsigned long start_time, u64 total_sent,
 				    u32 cookie)
 {
-	u32 total_bytes;
+	u64 total_bytes;
 	u32 test_time;
 	u8 result;
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net 05/14] batman-adv: dat: initialize orig_node of unfilled candidate slots
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (3 preceding siblings ...)
  2026-10-30 13:50 ` [PATCH net 04/14] batman-adv: tpmeter: fix u32 truncation of reported bytes Simon Wunderlich
@ 2026-10-30 13:50 ` Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net 06/14] batman-adv: fix skb leak when hard_iface is being released Simon Wunderlich
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable, Sashiko,
	Linus Lüssing, Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

batadv_dat_select_candidates() allocates the candidate array without
zeroing it. batadv_choose_next_candidate() marks each slot as
BATADV_DAT_CANDIDATE_NOT_FOUND but only sets orig_node when an eligible
originator was found. In the following rounds,
batadv_is_orig_node_eligible() compares res[j].orig_node of all previous
slots against the evaluated originator. For slots without candidates, it
reads uninitialized memory.

The orig_node has to be cleared together with the
BATADV_DAT_CANDIDATE_NOT_FOUND type.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-b25d9aa4-10bb-427c-a145-ccaadc2a1bb9
Fixes: 785ea1144182 ("batman-adv: Distributed ARP Table - create DHT helper functions")
Acked-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/distributed-arp-table.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 74050bffedbce..0260ed7e828b2 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -653,6 +653,7 @@ static void batadv_choose_next_candidate(struct batadv_priv *bat_priv,
 	 * NOT_FOUND
 	 */
 	cands[select].type = BATADV_DAT_CANDIDATE_NOT_FOUND;
+	cands[select].orig_node = NULL;
 
 	/* iterate over the originator list and find the node with the closest
 	 * dat_address which has not been selected yet
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net 06/14] batman-adv: fix skb leak when hard_iface is being released
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (4 preceding siblings ...)
  2026-10-30 13:50 ` [PATCH net 05/14] batman-adv: dat: initialize orig_node of unfilled candidate slots Simon Wunderlich
@ 2026-10-30 13:50 ` Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net 07/14] batman-adv: stop originator purge worker before freeing TT Simon Wunderlich
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

batadv_batman_skb_recv() owns the skb it is given. When the hard_iface
refcount already dropped to zero, the function jumped to err_out, which
only returned NET_RX_DROP and skipped the kfree_skb() under err_free.

To avoid a leak, free the skb before returning in this case.

Cc: stable@vger.kernel.org
Fixes: 4fe56e60ac1b ("batman-adv: Check hard_iface refcnt when receiving skb")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 6c54a8d4be3ec..ced76942f33c5 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -484,8 +484,10 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 	 * shut down otherwise the packet may trigger de-reference errors
 	 * further down in the receive path.
 	 */
-	if (!kref_get_unless_zero(&hard_iface->refcount))
-		goto err_out;
+	if (!kref_get_unless_zero(&hard_iface->refcount)) {
+		kfree_skb(skb);
+		return NET_RX_DROP;
+	}
 
 	skb = skb_share_check(skb, GFP_ATOMIC);
 
@@ -541,7 +543,6 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 	kfree_skb(skb);
 err_put:
 	batadv_hardif_put(hard_iface);
-err_out:
 	return NET_RX_DROP;
 }
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net 07/14] batman-adv: stop originator purge worker before freeing TT
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (5 preceding siblings ...)
  2026-10-30 13:50 ` [PATCH net 06/14] batman-adv: fix skb leak when hard_iface is being released Simon Wunderlich
@ 2026-10-30 13:50 ` Simon Wunderlich
  2026-10-30 13:50 ` [PATCH net v2 08/14] batman-adv: tt: only transmit local ROAM flag together with DEL Simon Wunderlich
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable, Sashiko,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

batadv_mesh_free() releases the translation tables via batadv_tt_free()
before batadv_originator_free() stops orig_work. The purge worker can
therefore still run while the TT is torn down. When an originator times
out, batadv_purge_orig_ref() calls batadv_tt_global_del_orig(), which
loads tt.global_hash and walks all buckets while taking their locks:

  CPU0                          CPU1
  batadv_tt_free()              batadv_purge_orig()
                                  batadv_tt_global_del_orig()
                                    hash = ...tt.global_hash;
    batadv_tt_global_table_free()
      batadv_hash_destroy(hash);
                                    spin_lock_bh(&hash->list_locks[i]);

Disable orig_work at the beginning of batadv_mesh_free(), directly after
the mesh was marked as deactivating. The second
disable_delayed_work_sync() in batadv_originator_free() is harmless because
it is free'd anyway.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260708091821.314516-1-sw%40simonwunderlich.de?part=3
Fixes: 9d31b3ce8168 ("batman-adv: fix potential TT client + orig-node memory leak")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index ced76942f33c5..3b9fa861be7be 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -295,6 +295,11 @@ void batadv_mesh_free(struct net_device *mesh_iface)
 
 	WRITE_ONCE(bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
 
+	/* the originator purge worker accesses (among others) the TT tables
+	 * and must therefore be stopped before any of them are freed
+	 */
+	disable_delayed_work_sync(&bat_priv->orig_work);
+
 	batadv_purge_outstanding_packets(bat_priv, NULL);
 	batadv_tp_stop_all(bat_priv);
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net v2 08/14] batman-adv: tt: only transmit local ROAM flag together with DEL
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (6 preceding siblings ...)
  2026-10-30 13:50 ` [PATCH net 07/14] batman-adv: stop originator purge worker before freeing TT Simon Wunderlich
@ 2026-10-30 13:50 ` Simon Wunderlich
  2026-10-30 13:51 ` [PATCH net 09/14] batman-adv: tt: clear ROAM flag when re-adding a pending local client Simon Wunderlich
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:50 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

BATADV_TT_CLIENT_ROAM is part of BATADV_TT_REMOTE_MASK but on the wire it
only has a defined meaning together with BATADV_TT_CLIENT_DEL: the
originator no longer serves the client because it roamed to another node.
Such a DEL is only relevant for events (non-full table requests).

An ADD carrying ROAM is processed (by accident) by the receiver like a
ROAMING_ADV: a newly created global entry is marked as roaming and is
excluded from the global CRC. But the sender includes it in its local CRC.

But such a behavior doesn't make sense for non-ROAM_ADV because a new TTVN
is meant as announced commit of the local TT table of a node and should
therefore not be used to create a temporary (roam) global entry in a node.

At the moment, such an ADD with the BATADV_TT_CLIENT_ROAM flag set can
happen due to two mechanisms:

batadv_tt_local_valid() copies all flags of the local entry into a full
table response. It is not synchronized with batadv_tt_local_mark_removed().
An entry which was marked as ROAM|PENDING by a ROAMING_ADV is still part of
the current TTVN and is sent with the ROAM flag set. The receivers then
create a roaming global entry for it and detect a CRC mismatch, which
causes them to request the full table again.

The same can happen for an ADD change which is queued for an entry which
(still) has the ROAM flag set. For example when only the dynamic WIFI or
ISOLA flags were updated.

It is not triggered by batadv_send_other_tt_response() because
batadv_tt_global_valid() is already taking care of filtering out
BATADV_TT_CLIENT_ROAM entries.

For the two problematic announcement mechanism, only send the ROAM flag of
local entries together with a DEL.

This only fixes the ROAM marking of the global entry and the resulting CRC
mismatch. When the requester of the full table is the node which sent the
ROAMING_ADV, the still announced entry will (independent of the ROAM flag)
cause batadv_tt_global_purge_local() to remove the requester's own (new)
local entry for the client. This is not addressed here.

Cc: stable@vger.kernel.org
Fixes: 27b37ebfa2d2 ("batman-adv: correctly pass the client flag on tt_response")
Fixes: 068ee6e204e1 ("batman-adv: roaming handling mechanism redesign")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 3349376a9087d..39b61426f1680 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -643,9 +643,17 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 	ether_addr_copy(tt_change_node->change.addr, common->addr);
 	tt_change_node->change.vid = htons(common->vid);
 
-	tt_change_node->change.flags = flags;
 	del_op_requested = flags & BATADV_TT_CLIENT_DEL;
 
+	/* the ROAM flag is only meaningful for the receiver in combination
+	 * with a DEL. An ADD with ROAM would be interpreted as a roaming
+	 * advertisement
+	 */
+	if (!del_op_requested)
+		flags &= ~BATADV_TT_CLIENT_ROAM;
+
+	tt_change_node->change.flags = flags;
+
 	/* check for ADD+DEL, DEL+ADD, ADD+ADD or DEL+DEL events */
 	spin_lock_bh(&bat_priv->tt.changes_list_lock);
 	changes = READ_ONCE(bat_priv->tt.local_changes);
@@ -3342,8 +3350,12 @@ static bool batadv_tt_local_valid(void *entry_ptr,
 	if (tt_flags & BATADV_TT_CLIENT_NEW)
 		return false;
 
+	/* the entry is (still) announced. A ROAM flag of an entry which is
+	 * pending to be removed must not be transmitted. Otherwise, the
+	 * receiver would handle it like a roaming advertisement
+	 */
 	if (flags)
-		*flags = tt_flags;
+		*flags = tt_flags & ~BATADV_TT_CLIENT_ROAM;
 
 	return true;
 }
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net 09/14] batman-adv: tt: clear ROAM flag when re-adding a pending local client
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (7 preceding siblings ...)
  2026-10-30 13:50 ` [PATCH net v2 08/14] batman-adv: tt: only transmit local ROAM flag together with DEL Simon Wunderlich
@ 2026-10-30 13:51 ` Simon Wunderlich
  2026-10-30 13:51 ` [PATCH net 10/14] batman-adv: tt: send ROAM_ADV on each roamed back client Simon Wunderlich
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:51 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

When a ROAMING_ADV is received for an announced local client,
batadv_tt_local_mark_removed() sets BATADV_TT_CLIENT_ROAM, queues a
DEL|ROAM change and marks the entry as BATADV_TT_CLIENT_PENDING. If the
client sends a frame again before the next TTVN commit,
batadv_tt_local_add_existing() only clears the PENDING flag and returns
early. The ADD cancels the queued DEL, but the entry keeps the ROAM flag:

* batadv_is_my_client() reports the client as not served by this node
* batadv_tt_local_client_is_roaming() reroutes unicast frames for it to
  the node which sent the ROAMING_ADV

The ROAM flag is only removed with the next frame of the client.

Since BATADV_TT_CLIENT_ROAM is set together with BATADV_TT_CLIENT_PENDING,
it has to be cleared also at the same time with BATADV_TT_CLIENT_PENDING
when the client roamed back.

Cc: stable@vger.kernel.org
Fixes: 068ee6e204e1 ("batman-adv: roaming handling mechanism redesign")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 39b61426f1680..e13c2828da475 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -894,6 +894,7 @@ static bool batadv_tt_local_add_existing(struct batadv_priv *bat_priv,
 					 bool *roamed_back)
 {
 	struct batadv_tt_common_entry *common = &tt_local->common;
+	bool readded = false;
 
 	tt_local->last_seen = jiffies;
 
@@ -908,8 +909,7 @@ static bool batadv_tt_local_add_existing(struct batadv_priv *bat_priv,
 			 * flag can be reset like it was never enqueued
 			 */
 			common->flags &= ~BATADV_TT_CLIENT_PENDING;
-
-			return true;
+			readded = true;
 		}
 
 		if (common->flags & BATADV_TT_CLIENT_ROAM) {
@@ -926,7 +926,7 @@ static bool batadv_tt_local_add_existing(struct batadv_priv *bat_priv,
 		}
 	}
 
-	return false;
+	return readded;
 }
 
 /**
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net 10/14] batman-adv: tt: send ROAM_ADV on each roamed back client
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (8 preceding siblings ...)
  2026-10-30 13:51 ` [PATCH net 09/14] batman-adv: tt: clear ROAM flag when re-adding a pending local client Simon Wunderlich
@ 2026-10-30 13:51 ` Simon Wunderlich
  2026-10-30 13:51 ` [PATCH net 11/14] batman-adv: tt: ignore ROAM flag of received ADD changes Simon Wunderlich
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:51 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

When a ROAMING_ADV is received for an announced local client,
batadv_roam_tvlv_unicast_handler_v1() creates a global entry with the
BATADV_TT_CLIENT_ROAM flag set.

If the client sends a frame again before the next TTVN commit,
batadv_tt_local_add_roam() is expected to announce this return via a
ROAM_ADV frame. But this doesn't happen because the global entry (created
by the first ROAMING_ADV) carries BATADV_TT_CLIENT_ROAM until the new
originator announces the client.

Let batadv_tt_local_add_roam() always handle a roamed back client -
independent of the ROAM flag of the global entry:

* trigger sending of the ROAM_ADV frame(s)
* remove the (with the roaming) associated global entry

Cc: stable@vger.kernel.org
Fixes: 068ee6e204e1 ("batman-adv: roaming handling mechanism redesign")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index e13c2828da475..0df51523dcf0c 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -819,14 +819,21 @@ static void batadv_tt_local_add_roam(struct batadv_priv *bat_priv,
 	if (!tt_global)
 		return;
 
-	/* Check whether it is a roaming, but don't do anything if the roaming
-	 * process has already been handled
+	/* Handle newly roamed client - but not roamed back client.
+	 *
+	 * A client which roamed back must always be reclaimed from the
+	 * originator(s) which sent the ROAMING_ADV. The global entry created by
+	 * this ROAMING_ADV might still carry BATADV_TT_CLIENT_ROAM because the
+	 * new originator did not yet announce the client.
 	 */
-	scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
-		if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
-			return;
+	if (!roamed_back) {
+		scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
+			/* Check whether it is a roaming, but don't do anything
+			 * if the roaming process has already been handled.
+			 */
+			if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
+				return;
 
-		if (!roamed_back) {
 			/* The global entry has to be marked as ROAMING and has to be
 			 * kept for consistency purpose.
 			 *
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net 11/14] batman-adv: tt: ignore ROAM flag of received ADD changes
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (9 preceding siblings ...)
  2026-10-30 13:51 ` [PATCH net 10/14] batman-adv: tt: send ROAM_ADV on each roamed back client Simon Wunderlich
@ 2026-10-30 13:51 ` Simon Wunderlich
  2026-10-30 13:51 ` [PATCH net v2 12/14] batman-adv: tt: only update flags for TT events Simon Wunderlich
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:51 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

A received TT change (OGM diff or full table response) with the ROAM flag
but without DEL is handled by batadv_tt_global_add() like a ROAMING_ADV:

* a newly created global entry gets BATADV_TT_CLIENT_ROAM and is
  therefore excluded from the global CRC. The sender has it in its local
  CRC and the receiver has to request the full table again
* batadv_tt_global_purge_local() removes the local entry of the
  receiver. The receiver's own client is considered as roamed to the sender
  and frames for it are rerouted

The ROAM flag only has a meaning together with DEL. Drop it on reception to
keep it from populating the global table with a temporary ROAMING entry
which is not part of the CRC - and therefore causing a CRC mismatch.

Cc: stable@vger.kernel.org
Fixes: 068ee6e204e1 ("batman-adv: roaming handling mechanism redesign")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 0df51523dcf0c..247cce95c09f4 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -3941,21 +3941,30 @@ static void _batadv_tt_update_changes(struct batadv_priv *bat_priv,
 				      u16 tt_num_changes, u8 ttvn)
 {
 	int roams;
+	u8 flags;
 	int i;
 
 	for (i = 0; i < tt_num_changes; i++) {
-		if ((tt_change + i)->flags & BATADV_TT_CLIENT_DEL) {
-			roams = (tt_change + i)->flags & BATADV_TT_CLIENT_ROAM;
+		flags = (tt_change + i)->flags;
+
+		if (flags & BATADV_TT_CLIENT_DEL) {
+			roams = flags & BATADV_TT_CLIENT_ROAM;
 			batadv_tt_global_del(bat_priv, orig_node,
 					     (tt_change + i)->addr,
 					     ntohs((tt_change + i)->vid),
 					     "tt removed by changes",
 					     roams);
 		} else {
+			/* an announced client cannot have roamed away from
+			 * orig_node. Only a ROAMING_ADV is allowed to add a
+			 * global entry with the ROAM flag
+			 */
+			flags &= ~BATADV_TT_CLIENT_ROAM;
+
 			if (!batadv_tt_global_add(bat_priv, orig_node,
 						  (tt_change + i)->addr,
 						  ntohs((tt_change + i)->vid),
-						  (tt_change + i)->flags, ttvn))
+						  flags, ttvn))
 				/* In case of problem while storing a
 				 * global_entry, we stop the updating
 				 * procedure without committing the
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net v2 12/14] batman-adv: tt: only update flags for TT events
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (10 preceding siblings ...)
  2026-10-30 13:51 ` [PATCH net 11/14] batman-adv: tt: ignore ROAM flag of received ADD changes Simon Wunderlich
@ 2026-10-30 13:51 ` Simon Wunderlich
  2026-11-01 13:52   ` netdev-bot+sashiko
  2026-10-30 13:51 ` [PATCH net v2 13/14] batman-adv: tt: don't drop local events queued during TVLV update Simon Wunderlich
  2026-10-30 13:51 ` [PATCH net v2 14/14] batman-adv: tt: limit local table to single TVLV response size Simon Wunderlich
  13 siblings, 1 reply; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:51 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

In the first implementation of the translation table events, it was only
possible to announce:

* ADD: new locally detected client
* DEL: disappeared local client

If an ADD event was queued and then a DEL event was queued in the same
timeslot, it is valid to destroy both events because nothing actually
changed for remode nodes

But with commit 3c4f7ab60cc2 ("batman-adv: improve the TT component to
support runtime flag changes") and subsequent changes, it could also happen
that there are extra ADD events:

* ADD: _NEW_ locally detected client
* ADD: already existing client with new flags
* DEL: disappeared local client

The first and the last type of event could in theory still be removed
together from the (to be announced) events queue. But if the "update flags"
ADD is dropped together with the DEL, it would still leave global
translation table of a remote node in the incorrect state - believing that
the client didn't disappear.

If a detected client was previously announced, a DEL event must only
overwrite the "update flags" ADD but the DEL itself must still continue to
exist in the (to be anounced) events queue. For now, adjustment of the
announced flags is used for everything instead of dropping already queued
up events completely.

Similarly, a DEL followed by an ADD caused both entries to be deleted. But
the new ADD could announce flags changes which were not yet present for the
client before it was marked for DELetion. The new ADD must therefore only
overwrite the DEL - but still be kept.

Cc: stable@vger.kernel.org
Fixes: 3c4f7ab60cc2 ("batman-adv: improve the TT component to support runtime flag changes")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 247cce95c09f4..fa5ead248f192 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -622,6 +622,9 @@ static u16 batadv_tt_flags_get(struct batadv_tt_common_entry *common)
  * @bat_priv: the bat priv with all the mesh interface information
  * @common: the TT entry involved in the event
  * @flags: flags of the TT entry combined with the event flags
+ *
+ * Only a single change per client is queued. It describes the latest state of
+ * the client and a new event therefore replaces an already queued one.
  */
 static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 				    const struct batadv_tt_common_entry *common,
@@ -631,7 +634,6 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 	struct batadv_tt_change_node *entry;
 	struct batadv_tt_change_node *safe;
 	bool del_op_requested;
-	bool del_op_entry;
 	size_t changes;
 
 	tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
@@ -665,25 +667,12 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 		if (entry->change.vid != tt_change_node->change.vid)
 			continue;
 
-		del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
-		if (del_op_requested != del_op_entry) {
-			/* DEL+ADD in the same orig interval have no effect and
-			 * can be removed to avoid silly behaviour on the
-			 * receiver side. The  other way around (ADD+DEL) can
-			 * happen in case of roaming of  a client still in the
-			 * NEW state. Roaming of NEW clients is now possible due
-			 * to automatically recognition of "temporary" clients
-			 */
-			list_del(&entry->list);
-			kmem_cache_free(batadv_tt_change_cache, entry);
-			changes--;
-		} else {
-			/* this is a second add or del in the same originator
-			 * interval. It could mean that flags have been changed
-			 * (e.g. double add): update them
-			 */
-			entry->change.flags = flags;
-		}
+		/* the other nodes may know the client with different
+		 * flags. A DEL+ADD must therefore still announce the
+		 * current flags and an ADD+DEL must still remove the
+		 * client. ADD+ADD and DEL+DEL only update the flags
+		 */
+		entry->change.flags = flags;
 
 		kmem_cache_free(batadv_tt_change_cache, tt_change_node);
 		goto update_changes;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net v2 13/14] batman-adv: tt: don't drop local events queued during TVLV update
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (11 preceding siblings ...)
  2026-10-30 13:51 ` [PATCH net v2 12/14] batman-adv: tt: only update flags for TT events Simon Wunderlich
@ 2026-10-30 13:51 ` Simon Wunderlich
  2026-10-30 13:51 ` [PATCH net v2 14/14] batman-adv: tt: limit local table to single TVLV response size Simon Wunderlich
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:51 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, stable, Sashiko,
	Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

batadv_tt_tvlv_container_update() read the number of local changes without
holding changes_list_lock. batadv_tt_local_event() is not serialized by
the commit_lock and can queue new events before the changes_list is
flushed:

  CPU0                          CPU1
  batadv_tt_tvlv_container_update()
    num = ...local_changes;
                                batadv_tt_local_event()
                                  list_add_tail(...);
                                  ...local_changes++;
    spin_lock_bh(&...changes_list_lock);
    /* only num events copied but all freed */

The additional events were freed without ever being sent. Other nodes will
only notice this via a CRC mismatch and must request the full table.

The number of changes and the changes_list must therefore be taken
atomically. Events queued afterwards then stay in the tt.changes_list for
the next TT round.

The changes can no longer be kept for the next round when the TVLV buffer
allocation fails. But they would be sent with a skipped TTVN anyway, so the
other nodes have to request the full table in this case either way. The
last changeset is only dropped in this case when the TTVN was incremented.
Without new changes, it still describes the current TTVN.

Previously, the unconsumed changes caused a retry of the TVLV container
update in the next commit round. Since the changes are now consumed, the
next round would only take the path without changes and wait for
BATADV_TT_OGM_APPEND_MAX rounds before updating the container again. Until
then, the outdated container with the previous TTVN would still be
announced. The ogm_append_cnt is therefore reset on failure to retry the
update in the next OGM interval.

This doesn't make the CRC consistent with the announced changeset. The
local table modifications are still not serialized with the commit and
batadv_tt_local_transition_new() can still make a client part of the CRC
for which the event is only part of the next TT round. Such an
inconsistency is detected by the CRC check of the other nodes and solved
via a full table request.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-fc286ddd-5bed-4683-b842-ee7a307eeb19
Fixes: a73105b8d4c7 ("batman-adv: improved client announcement mechanism")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 61 +++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index fa5ead248f192..f0100ccf93deb 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1323,22 +1323,24 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv,
  * batadv_tt_tvlv_container_update() - update the translation table tvlv
  *  container after local tt changes have been committed
  * @bat_priv: the bat priv with all the mesh interface information
+ * @changes_list: list of committed changes which will be consumed
+ * @tt_diff_entries_num: number of entries in @changes_list
  */
-static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
+static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv,
+					    struct list_head *changes_list,
+					    int tt_diff_entries_num)
 {
 	struct batadv_tvlv_tt_change *tt_change;
 	struct batadv_tt_change_node *entry;
 	struct batadv_tvlv_tt_data *tt_data;
 	struct batadv_tt_change_node *safe;
 	int tt_diff_entries_count = 0;
-	int tt_diff_entries_num = 0;
 	bool drop_changes = false;
 	size_t tt_extra_len = 0;
 	int tt_change_len = 0;
 	int tt_diff_len;
 	u16 tvlv_len;
 
-	tt_diff_entries_num = READ_ONCE(bat_priv->tt.local_changes);
 	tt_diff_len = batadv_tt_len(tt_diff_entries_num);
 
 	/* if we have too many changes for one packet don't send any
@@ -1356,19 +1358,39 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
 
 	tvlv_len = batadv_tt_prepare_tvlv_local_data(bat_priv, &tt_data,
 						     &tt_change, &tt_diff_len);
-	if (!tvlv_len)
+	if (!tvlv_len) {
+		/* the changes are lost and the other nodes have to request the
+		 * full table. Don't answer TT requests for the new TTVN with
+		 * the stale changeset of the previous TTVN. Without changes, the
+		 * TTVN was not incremented and the last changeset is still valid
+		 */
+		if (drop_changes || tt_diff_entries_num > 0) {
+			list_for_each_entry_safe(entry, safe, changes_list,
+						 list) {
+				list_del(&entry->list);
+				kmem_cache_free(batadv_tt_change_cache, entry);
+			}
+
+			spin_lock_bh(&bat_priv->tt.last_changeset_lock);
+			kfree(bat_priv->tt.last_changeset);
+			bat_priv->tt.last_changeset = NULL;
+			bat_priv->tt.last_changeset_len = 0;
+			spin_unlock_bh(&bat_priv->tt.last_changeset_lock);
+		}
+
+		/* announce the already committed TTVN in the next OGM interval
+		 * instead of waiting for BATADV_TT_OGM_APPEND_MAX idle rounds
+		 */
+		atomic_set(&bat_priv->tt.ogm_append_cnt, 0);
 		return;
+	}
 
 	tt_data->flags = BATADV_TT_OGM_DIFF;
 
 	if (!drop_changes && tt_diff_len == 0)
 		goto container_register;
 
-	spin_lock_bh(&bat_priv->tt.changes_list_lock);
-	WRITE_ONCE(bat_priv->tt.local_changes, 0);
-
-	list_for_each_entry_safe(entry, safe, &bat_priv->tt.changes_list,
-				 list) {
+	list_for_each_entry_safe(entry, safe, changes_list, list) {
 		if (tt_diff_entries_count < tt_diff_entries_num) {
 			memcpy(tt_change + tt_diff_entries_count,
 			       &entry->change,
@@ -1378,7 +1400,6 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
 		list_del(&entry->list);
 		kmem_cache_free(batadv_tt_change_cache, entry);
 	}
-	spin_unlock_bh(&bat_priv->tt.changes_list_lock);
 
 	tt_extra_len = batadv_tt_len(tt_diff_entries_num -
 				     tt_diff_entries_count);
@@ -4429,11 +4450,25 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
  */
 static void batadv_tt_local_commit_changes_nolock(struct batadv_priv *bat_priv)
 {
+	LIST_HEAD(changes_list);
+	int changes_num;
+
 	lockdep_assert_held(&bat_priv->tt.commit_lock);
 
-	if (READ_ONCE(bat_priv->tt.local_changes) == 0) {
+	/* atomically get the list of changes. The TTVN decision and the
+	 * announced changeset must cover the same events. Events queued
+	 * afterwards stay in the tt.changes_list for the next TT round
+	 */
+	spin_lock_bh(&bat_priv->tt.changes_list_lock);
+	list_splice_init(&bat_priv->tt.changes_list, &changes_list);
+	changes_num = READ_ONCE(bat_priv->tt.local_changes);
+	WRITE_ONCE(bat_priv->tt.local_changes, 0);
+	spin_unlock_bh(&bat_priv->tt.changes_list_lock);
+
+	if (changes_num == 0) {
 		if (!batadv_atomic_dec_not_zero(&bat_priv->tt.ogm_append_cnt))
-			batadv_tt_tvlv_container_update(bat_priv);
+			batadv_tt_tvlv_container_update(bat_priv, &changes_list,
+							changes_num);
 		return;
 	}
 
@@ -4450,7 +4485,7 @@ static void batadv_tt_local_commit_changes_nolock(struct batadv_priv *bat_priv)
 
 	/* reset the sending counter */
 	atomic_set(&bat_priv->tt.ogm_append_cnt, BATADV_TT_OGM_APPEND_MAX);
-	batadv_tt_tvlv_container_update(bat_priv);
+	batadv_tt_tvlv_container_update(bat_priv, &changes_list, changes_num);
 }
 
 /**
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net v2 14/14] batman-adv: tt: limit local table to single TVLV response size
  2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
                   ` (12 preceding siblings ...)
  2026-10-30 13:51 ` [PATCH net v2 13/14] batman-adv: tt: don't drop local events queued during TVLV update Simon Wunderlich
@ 2026-10-30 13:51 ` Simon Wunderlich
  2026-11-01 13:52   ` netdev-bot+sashiko
  13 siblings, 1 reply; 19+ messages in thread
From: Simon Wunderlich @ 2026-10-30 13:51 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, b.a.t.m.a.n, Sven Eckelmann, Sashiko,
	stable+noautosel, Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

The reservation of local TT entries and VLANs only limits the local table
by (int) packet_size_max. But the full table response is sent as a single
unicast TVLV packet whose tvlv_len (covering the TVLV header and the TT
TVLV) is only 16 bit wide. The reservation must therefore be capped at the
minimum of both the packet_size_max and the maximum theoretic table
response.

The size limit was already missing the 16 bit tvlv_len cap since the
original packet_size_max check was introduced. But this check was only
reachable with fragmentation disabled and a hard interface MTU larger than
the maximum unicast TVLV packet. In this case,
batadv_tt_prepare_tvlv_local_data() refuses to build the full table
response and no oversized packet is created. It is therefore mostly
affecting the commit 85fb027f611b ("batman-adv: tt: reject VLAN/TT entries
before reaching size limit"), which should prevent this kind of
overallocation.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de?part=8
Fixes: a19d3d85e1b8 ("batman-adv: limit local translation table max size")
Cc: <stable+noautosel@kernel.org> # requires 85fb027f611b, old behavior only affects MTU > 64KiB without fragmentation
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 69 ++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 22 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index f0100ccf93deb..46304069b3b75 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -26,8 +26,10 @@
 #include <linux/jhash.h>
 #include <linux/jiffies.h>
 #include <linux/kref.h>
+#include <linux/limits.h>
 #include <linux/list.h>
 #include <linux/lockdep.h>
+#include <linux/minmax.h>
 #include <linux/net.h>
 #include <linux/netdevice.h>
 #include <linux/netlink.h>
@@ -253,7 +255,8 @@ static int batadv_tt_len(int changes_num)
  *
  * Return: local translation table size in bytes.
  */
-static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
+static int batadv_tt_local_transmit_size(unsigned int num_vlan,
+					 unsigned int num_entries)
 {
 	int hdr_size;
 
@@ -266,6 +269,25 @@ static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
 	return hdr_size + batadv_tt_len(num_entries);
 }
 
+/**
+ * batadv_tt_local_transmit_size_max() - get the maximum size of a full local table
+ *  response
+ * @bat_priv: the bat priv with all the mesh interface information
+ *
+ * The full table response is sent as a single unicast TVLV packet. Its
+ * tvlv_len, which covers the TVLV header and the TT TVLV, is only 16 bit wide.
+ * This limits the response size even when packet_size_max is larger.
+ *
+ * Return: maximum size of a full local table response in bytes
+ */
+static int batadv_tt_local_transmit_size_max(struct batadv_priv *bat_priv)
+{
+	int tvlv_size_max = sizeof(struct batadv_unicast_tvlv_packet) + U16_MAX;
+	int packet_size_max = READ_ONCE(bat_priv->packet_size_max);
+
+	return min(packet_size_max, tvlv_size_max);
+}
+
 /**
  * batadv_tt_local_reserve() - reserve room in the transmittable local table
  * @bat_priv: the bat priv with all the mesh interface information
@@ -290,22 +312,23 @@ static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
 static bool batadv_tt_local_reserve(struct batadv_priv *bat_priv, u16 num_vlan,
 				    u16 num_entries, int *table_size)
 {
-	int packet_size_max = READ_ONCE(bat_priv->packet_size_max);
+	int size_max = batadv_tt_local_transmit_size_max(bat_priv);
+	unsigned int entries;
+	unsigned int vlans;
 
 	scoped_guard(spinlock_bh, &bat_priv->tt.reserve_lock) {
-		bat_priv->tt.reserved_vlans += num_vlan;
-		bat_priv->tt.reserved_entries += num_entries;
+		vlans = bat_priv->tt.reserved_vlans + num_vlan;
+		entries = bat_priv->tt.reserved_entries + num_entries;
 
-		*table_size = batadv_tt_local_transmit_size(bat_priv->tt.reserved_vlans,
-							    bat_priv->tt.reserved_entries);
-		if (*table_size <= packet_size_max)
-			return true;
+		*table_size = batadv_tt_local_transmit_size(vlans, entries);
+		if (*table_size > size_max)
+			return false;
 
-		bat_priv->tt.reserved_vlans -= num_vlan;
-		bat_priv->tt.reserved_entries -= num_entries;
+		bat_priv->tt.reserved_vlans = vlans;
+		bat_priv->tt.reserved_entries = entries;
 	}
 
-	return false;
+	return true;
 }
 
 /**
@@ -339,9 +362,10 @@ static bool batadv_tt_local_reserve_entry(struct batadv_priv *bat_priv,
 		return true;
 
 	net_ratelimited_function(batadv_info, bat_priv->mesh_iface,
-				 "Local translation table size (%i) exceeds maximum packet size (%i); Ignoring new local tt entry: %pM\n",
+				 "Local translation table size (%i) exceeds maximum table response size (%i); Ignoring new local tt entry: %pM\n",
 				 table_size,
-				 READ_ONCE(bat_priv->packet_size_max), addr);
+				 batadv_tt_local_transmit_size_max(bat_priv),
+				 addr);
 
 	return false;
 }
@@ -374,8 +398,9 @@ bool batadv_tt_local_reserve_vlan(struct batadv_priv *bat_priv,
 		return true;
 
 	net_ratelimited_function(batadv_info, bat_priv->mesh_iface,
-				 "Local translation table size (%i) exceeds maximum packet size (%i); Ignoring new VLAN: %d\n",
-				 table_size, READ_ONCE(bat_priv->packet_size_max),
+				 "Local translation table size (%i) exceeds maximum table response size (%i); Ignoring new VLAN: %d\n",
+				 table_size,
+				 batadv_tt_local_transmit_size_max(bat_priv),
 				 batadv_print_vid(vid));
 
 	return false;
@@ -724,9 +749,9 @@ static u16 batadv_tt_entries(u16 tt_len)
  */
 static int batadv_tt_local_table_transmit_size(struct batadv_priv *bat_priv)
 {
+	unsigned int tt_local_entries = 0;
 	struct batadv_meshif_vlan *vlan;
-	u16 tt_local_entries = 0;
-	u16 num_vlan = 0;
+	unsigned int num_vlan = 0;
 
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(vlan, &bat_priv->meshif_vlan_list, list) {
@@ -4727,16 +4752,16 @@ void batadv_tt_local_resize_to_mtu(struct net_device *mesh_iface)
 	struct batadv_priv *bat_priv = netdev_priv(mesh_iface);
 	int timeout = BATADV_TT_LOCAL_TIMEOUT / 2;
 	bool reduced = false;
-	int packet_size_max;
 	int table_size;
+	int size_max;
 
-	packet_size_max = READ_ONCE(bat_priv->packet_size_max);
+	size_max = batadv_tt_local_transmit_size_max(bat_priv);
 
 	spin_lock_bh(&bat_priv->tt.commit_lock);
 
 	while (timeout) {
 		table_size = batadv_tt_local_table_transmit_size(bat_priv);
-		if (packet_size_max >= table_size)
+		if (size_max >= table_size)
 			break;
 
 		batadv_tt_local_purge(bat_priv, timeout);
@@ -4745,8 +4770,8 @@ void batadv_tt_local_resize_to_mtu(struct net_device *mesh_iface)
 		timeout /= 2;
 		reduced = true;
 		net_ratelimited_function(batadv_info, mesh_iface,
-					 "Forced to purge local tt entries to fit new maximum fragment MTU (%i)\n",
-					 packet_size_max);
+					 "Forced to purge local tt entries to fit new maximum table response size (%i)\n",
+					 size_max);
 	}
 
 	/* commit these changes immediately, to avoid synchronization problem
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure
  2026-10-30 13:50 ` [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure Simon Wunderlich
@ 2026-10-30 13:54   ` netdev-bot+sinfo
  2026-10-30 22:00   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 19+ messages in thread
From: netdev-bot+sinfo @ 2026-10-30 13:54 UTC (permalink / raw)
  To: Simon Wunderlich
  Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, b.a.t.m.a.n, Deepanshu Kartikey,
	syzbot+a079f084653164a652d1, Sven Eckelmann

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure
  2026-10-30 13:50 ` [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure Simon Wunderlich
  2026-10-30 13:54   ` netdev-bot+sinfo
@ 2026-10-30 22:00   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 19+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-30 22:00 UTC (permalink / raw)
  To: Simon Wunderlich
  Cc: netdev, davem, edumazet, kuba, pabeni, horms, b.a.t.m.a.n,
	kartikey406, syzbot+a079f084653164a652d1, sven

Hello:

This series was applied to netdev/net.git (main)
by Sven Eckelmann <sven@narfation.org>:

On Fri, 30 Oct 2026 14:50:52 +0100 you wrote:
> From: Deepanshu Kartikey <kartikey406@gmail.com>
> 
> batadv_orig_ifinfo_new() returns NULL only when its GFP_ATOMIC
> allocation fails, which is a normal runtime condition and not a bug.
> batadv_v_ogm_forward() already handles this the same way, simply
> checking for NULL without a WARN_ON.
> 
> [...]

Here is the summary with links:
  - [net,01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure
    https://git.kernel.org/netdev/net/c/dfa953ba5366
  - [net,v2,02/14] batman-adv: Close OGM aggregation before transmission
    https://git.kernel.org/netdev/net/c/f91f64a91682
  - [net,03/14] batman-adv: mcast: validate tracker padding before forwarding
    https://git.kernel.org/netdev/net/c/ea01fff30f63
  - [net,04/14] batman-adv: tpmeter: fix u32 truncation of reported bytes
    https://git.kernel.org/netdev/net/c/1025b76d1328
  - [net,05/14] batman-adv: dat: initialize orig_node of unfilled candidate slots
    https://git.kernel.org/netdev/net/c/4eb19a389d74
  - [net,06/14] batman-adv: fix skb leak when hard_iface is being released
    https://git.kernel.org/netdev/net/c/522e4aece2e9
  - [net,07/14] batman-adv: stop originator purge worker before freeing TT
    https://git.kernel.org/netdev/net/c/f89eda5a9ccf
  - [net,v2,08/14] batman-adv: tt: only transmit local ROAM flag together with DEL
    https://git.kernel.org/netdev/net/c/10f24f058417
  - [net,09/14] batman-adv: tt: clear ROAM flag when re-adding a pending local client
    https://git.kernel.org/netdev/net/c/cc2186d78547
  - [net,10/14] batman-adv: tt: send ROAM_ADV on each roamed back client
    https://git.kernel.org/netdev/net/c/aff4d5d10f2d
  - [net,11/14] batman-adv: tt: ignore ROAM flag of received ADD changes
    https://git.kernel.org/netdev/net/c/c5d68abe6e7f
  - [net,v2,12/14] batman-adv: tt: only update flags for TT events
    https://git.kernel.org/netdev/net/c/ad1fd828aa39
  - [net,v2,13/14] batman-adv: tt: don't drop local events queued during TVLV update
    https://git.kernel.org/netdev/net/c/ed7a0dfdd370
  - [net,v2,14/14] batman-adv: tt: limit local table to single TVLV response size
    https://git.kernel.org/netdev/net/c/a137095419b4

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net v2 12/14] batman-adv: tt: only update flags for TT events
  2026-10-30 13:51 ` [PATCH net v2 12/14] batman-adv: tt: only update flags for TT events Simon Wunderlich
@ 2026-11-01 13:52   ` netdev-bot+sashiko
  0 siblings, 0 replies; 19+ messages in thread
From: netdev-bot+sashiko @ 2026-11-01 13:52 UTC (permalink / raw)
  To: sw
  Cc: netdev, davem, edumazet, kuba, pabeni, horms, b.a.t.m.a.n, sven,
	stable, sashiko-bot

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

Critical: 0 · High: 0 · Medium: 1 · Low: 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).

- [Medium] The patch now treats each new TT event as the client's latest
  state and writes its flags over any queued change for the same…
- [Low] Claimed mismatch between the kernel-doc and the new behaviour for
  NEW clients.

--

batman-adv: tt: only update flags for TT events

This commit changes __batadv_tt_local_event(). DEL+ADD and ADD+DEL pairs
for the same client no longer cancel each other. Instead, a new event
overwrites the flags of the change that is already queued. This keeps a
DEL that follows a flag-update ADD, and it keeps an ADD with new flags
that follows a DEL.

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index 247cce95c09f4..fa5ead248f192 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c

[ ... ]

> @@ -665,25 +667,12 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
>  		if (entry->change.vid != tt_change_node->change.vid)
>  			continue;
>  
> -		del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
> -		if (del_op_requested != del_op_entry) {
> -			/* DEL+ADD in the same orig interval have no effect and
> -			 * can be removed to avoid silly behaviour on the
> -			 * receiver side. The  other way around (ADD+DEL) can
> -			 * happen in case of roaming of  a client still in the
> -			 * NEW state. Roaming of NEW clients is now possible due
> -			 * to automatically recognition of "temporary" clients
> -			 */
> -			list_del(&entry->list);
> -			kmem_cache_free(batadv_tt_change_cache, entry);
> -			changes--;

[Severity: Low]

This isn't a bug, but for a client still in the NEW state, the ADD+DEL
pair used to cancel. It is now kept as an announced DEL:

batadv_tt_local_remove()
  batadv_tt_local_mark_removed()   <- breaks out for BATADV_TT_CLIENT_NEW
  batadv_tt_local_remove_now()
    batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);

When a NEW client is added and removed within one OGM interval, this
causes an extra TTVN increment. It also puts a DEL entry in the TT diff
for a client that was never announced.

Receivers seem to treat that DEL as a no-op, because batadv_tt_global_del()
bails out when batadv_tt_global_hash_find() finds nothing. The commit
message also says the extra event is accepted for now ("The first and the
last type of event could in theory still be removed together ...").

Is the extra announcement intended for the NEW-client case, or would it be
worth keeping the cancellation there?

> -		} else {
> -			/* this is a second add or del in the same originator
> -			 * interval. It could mean that flags have been changed
> -			 * (e.g. double add): update them
> -			 */
> -			entry->change.flags = flags;
> -		}
> +		/* the other nodes may know the client with different
> +		 * flags. A DEL+ADD must therefore still announce the
> +		 * current flags and an ADD+DEL must still remove the
> +		 * client. ADD+ADD and DEL+DEL only update the flags
> +		 */
> +		entry->change.flags = flags;

[Severity: Medium]

Does this overwrite rely on events reaching changes_list_lock in the same
order as the state changes they describe?

batadv_tt_local_event() takes its flags snapshot under flags_lock. It
releases that lock before __batadv_tt_local_event() takes
changes_list_lock:

net/batman-adv/translation-table.c:batadv_tt_local_event() {
	...
	flags = batadv_tt_flags_get(common) | event_flags;
	__batadv_tt_local_event(bat_priv, common, flags);
}

The ADD callers in batadv_tt_local_add() hold neither the bucket list_lock
nor flags_lock here. The DEL paths queue their event either under those
locks (batadv_tt_local_set_pending()) or after removing the entry from the
hash (batadv_tt_local_remove_now()). Nothing orders either DEL path against
a concurrent ADD.

Here is the sequence for a new client:

CPU A (TX path)
batadv_interface_tx()
  batadv_tt_local_add()
    batadv_tt_local_create()
    batadv_hash_add()             <- entry is now visible
    batadv_tt_local_event(ADD)
      batadv_tt_flags_get()
                                  CPU B (RX path)
                                  batadv_tt_global_add()
                                    batadv_tt_global_purge_local()
                                      batadv_tt_local_remove()
                                        batadv_tt_local_mark_removed()
                                          <- NEW, not set pending
                                        batadv_tt_local_remove_now()
                                          batadv_hash_remove()
                                          batadv_tt_local_event(DEL)
      __batadv_tt_local_event()
        entry->change.flags = flags;  <- queued DEL becomes ADD

Before this patch the DEL+ADD pair cancelled. That gave the right result
in this case, because the client was never announced and is no longer in
the local hash.

With this change, can an ADD be announced for a client that is no longer
in the local table?

The same window exists for an existing client. batadv_tt_local_add() can
take the ADD snapshot after batadv_tt_local_update_flags(), and
batadv_tt_local_set_pending() can then queue DEL|... before the ADD gets
changes_list_lock. The DEL was also lost in this ordering before the
patch. Now the stale ADD replaces it as the latest state.

In both cases, remote global tables would keep a client that the local
node removes, through batadv_tt_local_purge_pending_clients() at the next
commit in the second case. This lasts until a CRC mismatch triggers a full
table request.

As far as I can tell, the later patches in this series don't change
batadv_tt_local_event() or __batadv_tt_local_event(), so the window is
still there at the end of the series.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261030135105.2555887-2-sw%40simonwunderlich.de

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net v2 14/14] batman-adv: tt: limit local table to single TVLV response size
  2026-10-30 13:51 ` [PATCH net v2 14/14] batman-adv: tt: limit local table to single TVLV response size Simon Wunderlich
@ 2026-11-01 13:52   ` netdev-bot+sashiko
  0 siblings, 0 replies; 19+ messages in thread
From: netdev-bot+sashiko @ 2026-11-01 13:52 UTC (permalink / raw)
  To: sw
  Cc: netdev, davem, edumazet, kuba, pabeni, horms, b.a.t.m.a.n, sven,
	stable, sashiko-bot

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] The commit message describes the pre-patch behaviour wrongly.
- [Low] Two kernel-doc comments no longer match the code after this
  patch.

Pre-existing issues:
- [Low] This problem wasn't introduced by this patch, but the same 16-bit
  tvlv_len limit is still not enforced when a node answers a TT_REQUEST…
- [Low] This problem wasn't introduced by this patch, but the size limit
  is read before the lock is taken.

--

batman-adv: tt: limit local table to single TVLV response size

This patch adds batadv_tt_local_transmit_size_max(). It caps the local TT
reservation and the resize-to-MTU limit at the smaller of packet_size_max
and the largest full table response that fits in a single unicast TVLV
packet with a 16 bit tvlv_len. The reservation logic now computes the new
totals in locals and writes the counters only when the reservation
succeeds.

> The size limit was already missing the 16 bit tvlv_len cap since the
> original packet_size_max check was introduced. But this check was only
> reachable with fragmentation disabled and a hard interface MTU larger than
> the maximum unicast TVLV packet. In this case,
> batadv_tt_prepare_tvlv_local_data() refuses to build the full table
> response and no oversized packet is created.

[Severity: Low]

Does this describe the old behaviour accurately?

batadv_tt_prepare_tvlv_local_data() only bounds the TT value with a u16,
and does not count the 4 byte struct batadv_tvlv_hdr:

	if (check_add_overflow(*tt_len, change_offset, &tvlv_len)) {

batadv_tvlv_unicast_send() then adds the header and stores the result
without a range check:

	tvlv_len = sizeof(*tvlv_hdr) + tvlv_value_len;
	...
	unicast_tvlv_packet->tvlv_len = htons(tvlv_len);

All TT components are multiples of 4, so a 65532 byte TT value is
reachable. For example, 1 VLAN and 5460 entries gives 4 + 8 + 65520.

Before this patch, the reservation allowed that table whenever
packet_size_max >= 65556. The full table response would then be built and
sent with tvlv_len = 65536, which htons() truncates to 0. So an oversized,
malformed packet does seem to have been created. The receiver in
batadv_recv_unicast_tvlv() then just processes no TVLVs.

The code change handles this for the local table: table_size includes the
TVLV header and is capped at sizeof(struct batadv_unicast_tvlv_packet) +
U16_MAX.

The stable+noautosel reasoning depends on how the old behaviour is
described. Could the commit message be made more precise here?

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index f0100ccf93deb..46304069b3b75 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c

[ ... ]

> @@ -266,6 +269,25 @@ static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
>  	return hdr_size + batadv_tt_len(num_entries);
>  }
>  
> +/**
> + * batadv_tt_local_transmit_size_max() - get the maximum size of a full local table
> + *  response
> + * @bat_priv: the bat priv with all the mesh interface information
> + *
> + * The full table response is sent as a single unicast TVLV packet. Its
> + * tvlv_len, which covers the TVLV header and the TT TVLV, is only 16 bit wide.
> + * This limits the response size even when packet_size_max is larger.
> + *
> + * Return: maximum size of a full local table response in bytes
> + */
> +static int batadv_tt_local_transmit_size_max(struct batadv_priv *bat_priv)
> +{
> +	int tvlv_size_max = sizeof(struct batadv_unicast_tvlv_packet) + U16_MAX;
> +	int packet_size_max = READ_ONCE(bat_priv->packet_size_max);
> +
> +	return min(packet_size_max, tvlv_size_max);
> +}

[Severity: Low]

This is a pre-existing issue and was not introduced by this patch. The
same 16 bit tvlv_len limit does not seem to be enforced when a node
answers a TT_REQUEST on behalf of another originator.
batadv_send_other_tt_response() only checks this:

	/* Don't send the response, if larger than fragmented packet. */
	tt_len = sizeof(struct batadv_unicast_tvlv_packet) + tvlv_len;
	if (tt_len > READ_ONCE(bat_priv->packet_size_max)) {

That check leaves out the 4 byte batadv_tvlv_hdr. It also has no cap at
sizeof(struct batadv_unicast_tvlv_packet) + U16_MAX.
batadv_tt_prepare_tvlv_global_data() has the same u16-only
check_add_overflow() as the local variant.

A remote originator's global entries come from OGM TT diffs. The local
reserve_lock accounting does not limit them.

Take fragmentation disabled and a hard interface MTU >= 65552. A relayed
table with a 65532 byte TT value (1 VLAN and 5460 entries) passes both
checks. batadv_tvlv_unicast_send() then sends it with tvlv_len truncated
from 65536 to 0.

Only the local reservation and resize paths use the new
batadv_tt_local_transmit_size_max(). Would a check in
batadv_tvlv_unicast_send() that rejects tvlv_value_len >
U16_MAX - sizeof(struct batadv_tvlv_hdr) cover every caller?

> @@ -290,22 +312,23 @@ static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
>  static bool batadv_tt_local_reserve(struct batadv_priv *bat_priv, u16 num_vlan,
>  				    u16 num_entries, int *table_size)
>  {
> -	int packet_size_max = READ_ONCE(bat_priv->packet_size_max);
> +	int size_max = batadv_tt_local_transmit_size_max(bat_priv);
> +	unsigned int entries;
> +	unsigned int vlans;
>  
>  	scoped_guard(spinlock_bh, &bat_priv->tt.reserve_lock) {

[Severity: Low]

This isn't a bug introduced by this patch, but the limit is still read
before reserve_lock is taken. The old code had the same READ_ONCE() before
the guard. batadv_tt_local_resize_to_mtu() also reads the limit before
taking commit_lock.

Can a concurrent MTU decrease leave the local table larger than the new
packet_size_max?

CPU0                                CPU1
batadv_tt_local_add()
 batadv_tt_local_reserve_entry()
  batadv_tt_local_reserve()
   size_max = <old, larger limit>
                                    batadv_update_min_mtu()
                                     batadv_hardif_min_mtu()
                                      WRITE_ONCE(packet_size_max, ...)
                                     batadv_tt_local_resize_to_mtu()
                                      /* table fits, nothing purged */
   scoped_guard(reserve_lock)
    reservation committed against
    the stale size_max
 entry inserted

The purge decision uses batadv_tt_local_table_transmit_size(), which only
counts vlan->tt.num_entries. It does not see in-flight reservations
(tt.reserved_entries and tt.reserved_vlans) or entries that still carry
BATADV_TT_CLIENT_NEW.

Nothing checks the table again until entries age out, so full table
responses may be dropped until then.

[ ... ]

> @@ -4727,16 +4752,16 @@ void batadv_tt_local_resize_to_mtu(struct net_device *mesh_iface)
>  	struct batadv_priv *bat_priv = netdev_priv(mesh_iface);
>  	int timeout = BATADV_TT_LOCAL_TIMEOUT / 2;
>  	bool reduced = false;
> -	int packet_size_max;
>  	int table_size;
> +	int size_max;
>  
> -	packet_size_max = READ_ONCE(bat_priv->packet_size_max);
> +	size_max = batadv_tt_local_transmit_size_max(bat_priv);
>  
>  	spin_lock_bh(&bat_priv->tt.commit_lock);
>  
>  	while (timeout) {
>  		table_size = batadv_tt_local_table_transmit_size(bat_priv);
> -		if (packet_size_max >= table_size)
> +		if (size_max >= table_size)
>  			break;

[Severity: Low]

Should the kernel-doc be updated to match? The kernel-doc for
batadv_tt_local_resize_to_mtu() still says:

 * batadv_tt_local_resize_to_mtu() - resize the local translation table fit the
 *  maximum packet size that can be transported through the mesh

The loop now compares against batadv_tt_local_transmit_size_max(), and the
log message below already says "maximum table response size".

The kernel-doc for batadv_tt_local_reserve() also still describes the old
add-then-drop handling:

 * Add the requested number of VLANs and local TT entries to the reservation
 * counters and check whether the local translation table would then still fit
 * in a single full table response. The reservation is dropped again when it
 * would not.

The new body computes the totals in vlans and entries and writes the
counters only on success. Nothing is dropped any more.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261030135105.2555887-2-sw%40simonwunderlich.de

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2026-11-01 13:52 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 13:50 [PATCH net 00/14] pull request: batman-adv 2026-10-30 Simon Wunderlich
2026-10-30 13:50 ` [PATCH net 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure Simon Wunderlich
2026-10-30 13:54   ` netdev-bot+sinfo
2026-10-30 22:00   ` patchwork-bot+netdevbpf
2026-10-30 13:50 ` [PATCH net v2 02/14] batman-adv: Close OGM aggregation before transmission Simon Wunderlich
2026-10-30 13:50 ` [PATCH net 03/14] batman-adv: mcast: validate tracker padding before forwarding Simon Wunderlich
2026-10-30 13:50 ` [PATCH net 04/14] batman-adv: tpmeter: fix u32 truncation of reported bytes Simon Wunderlich
2026-10-30 13:50 ` [PATCH net 05/14] batman-adv: dat: initialize orig_node of unfilled candidate slots Simon Wunderlich
2026-10-30 13:50 ` [PATCH net 06/14] batman-adv: fix skb leak when hard_iface is being released Simon Wunderlich
2026-10-30 13:50 ` [PATCH net 07/14] batman-adv: stop originator purge worker before freeing TT Simon Wunderlich
2026-10-30 13:50 ` [PATCH net v2 08/14] batman-adv: tt: only transmit local ROAM flag together with DEL Simon Wunderlich
2026-10-30 13:51 ` [PATCH net 09/14] batman-adv: tt: clear ROAM flag when re-adding a pending local client Simon Wunderlich
2026-10-30 13:51 ` [PATCH net 10/14] batman-adv: tt: send ROAM_ADV on each roamed back client Simon Wunderlich
2026-10-30 13:51 ` [PATCH net 11/14] batman-adv: tt: ignore ROAM flag of received ADD changes Simon Wunderlich
2026-10-30 13:51 ` [PATCH net v2 12/14] batman-adv: tt: only update flags for TT events Simon Wunderlich
2026-11-01 13:52   ` netdev-bot+sashiko
2026-10-30 13:51 ` [PATCH net v2 13/14] batman-adv: tt: don't drop local events queued during TVLV update Simon Wunderlich
2026-10-30 13:51 ` [PATCH net v2 14/14] batman-adv: tt: limit local table to single TVLV response size Simon Wunderlich
2026-11-01 13:52   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox