From dfa953ba5366b0a80c303700dab2bbcbdc5c27e9 Mon Sep 17 00:00:00 2001 From: Deepanshu Kartikey Date: Sun, 13 Sep 2026 08:49:06 +0530 Subject: [PATCH 01/14] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure 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 Signed-off-by: Sven Eckelmann --- 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 89b499f7a37d..158d2b8f0403 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; } From f91f64a91682c07e6c5f059804e709656364e8a6 Mon Sep 17 00:00:00 2001 From: Chengfeng Ye Date: Sun, 27 Sep 2026 18:16:24 +0800 Subject: [PATCH 02/14] batman-adv: Close OGM aggregation before transmission 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 [ sven: drop assisted tag, document new meaning of num_packets ] Signed-off-by: Sven Eckelmann --- 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 158d2b8f0403..85e99506a53f 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 c19caa84e692..b3e6dbb360da 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 */ From ea01fff30f631e67ee5f0a2b97855d772b3a35fc Mon Sep 17 00:00:00 2001 From: Kyle Zeng Date: Thu, 29 Oct 2026 21:21:46 -0700 Subject: [PATCH 03/14] batman-adv: mcast: validate tracker padding before forwarding 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 Signed-off-by: Sven Eckelmann --- 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 ac97cd8b81dc..9ffed1f237b6 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; From 1025b76d13288a311e8259a0eefde0854fa63c41 Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Tue, 6 Oct 2026 19:38:15 +0200 Subject: [PATCH 04/14] batman-adv: tpmeter: fix u32 truncation of reported bytes 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 Closes: https://sashiko.dev/#/bug/linux-08bb3be4-6c6a-452d-b4e4-606940c6cfa5 Fixes: 33a3bb4a3345 ("batman-adv: throughput meter implementation") Signed-off-by: Sven Eckelmann --- 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 022eb13e8ec4..56feb02d2b7e 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; From 4eb19a389d74f06901b4cd6f24b996b2e84eaefc Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Sun, 4 Oct 2026 10:10:06 +0200 Subject: [PATCH 05/14] batman-adv: dat: initialize orig_node of unfilled candidate slots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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 Signed-off-by: Sven Eckelmann --- 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 74050bffedbc..0260ed7e828b 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 From 522e4aece2e96cacbb8a6ae1eec0d4c0dd6f4fed Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Tue, 13 Oct 2026 11:23:34 +0000 Subject: [PATCH 06/14] batman-adv: fix skb leak when hard_iface is being released 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 --- 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 6c54a8d4be3e..ced76942f33c 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; } From f89eda5a9ccf82cba06b085b42ebc4886b8dfe9e Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Thu, 8 Oct 2026 18:29:16 +0200 Subject: [PATCH 07/14] batman-adv: stop originator purge worker before freeing TT 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 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 --- 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 ced76942f33c..3b9fa861be7b 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); From 10f24f058417024a2c19cb8d88a6a0678a58a21b Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Thu, 8 Oct 2026 16:30:06 +0200 Subject: [PATCH 08/14] batman-adv: tt: only transmit local ROAM flag together with DEL 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 --- 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 3349376a9087..39b61426f168 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; } From cc2186d7854785b30d423f5d43757b086b300e55 Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Wed, 7 Oct 2026 13:02:45 +0200 Subject: [PATCH 09/14] batman-adv: tt: clear ROAM flag when re-adding a pending local client 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 --- 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 39b61426f168..e13c2828da47 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; } /** From aff4d5d10f2dcb4a9b604e27414dafa1d51b6406 Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Thu, 8 Oct 2026 12:00:27 +0200 Subject: [PATCH 10/14] batman-adv: tt: send ROAM_ADV on each roamed back client 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 --- 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 e13c2828da47..0df51523dcf0 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. * From c5d68abe6e7f1dbd132a602cb696fe1fff0389eb Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Wed, 7 Oct 2026 13:03:29 +0200 Subject: [PATCH 11/14] batman-adv: tt: ignore ROAM flag of received ADD changes 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 --- 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 0df51523dcf0..247cce95c09f 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 From ad1fd828aa3944efc393b84b0d7c02483f1b5638 Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Thu, 8 Oct 2026 16:30:10 +0200 Subject: [PATCH 12/14] batman-adv: tt: only update flags for TT events 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 --- 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 247cce95c09f..fa5ead248f19 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; From ed7a0dfdd370acb48be7a281df732c1bebcc7f1d Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Thu, 8 Oct 2026 17:41:24 +0200 Subject: [PATCH 13/14] batman-adv: tt: don't drop local events queued during TVLV update 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 Closes: https://sashiko.dev/#/bug/linux-fc286ddd-5bed-4683-b842-ee7a307eeb19 Fixes: a73105b8d4c7 ("batman-adv: improved client announcement mechanism") Signed-off-by: Sven Eckelmann --- 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 fa5ead248f19..f0100ccf93de 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); } /** From a137095419b4be0b204a11af4f1cea872ea262e9 Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Sat, 3 Oct 2026 00:00:51 +0200 Subject: [PATCH 14/14] batman-adv: tt: limit local table to single TVLV response size 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 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: # requires 85fb027f611b, old behavior only affects MTU > 64KiB without fragmentation Signed-off-by: Sven Eckelmann --- 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 f0100ccf93de..46304069b3b7 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -26,8 +26,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -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