Bug #432
closedbat_v: Leak of aggregated skbs
0%
Description
See
- https://sashiko.dev/#/patchset/20260511-yuantan098-bugfixes-v1-0-6adf6965f2b3@narfation.org?part=5
- https://sashiko.dev/#/patchset/20260607-drop-hardif-list-v4-0-ade2931db295@narfation.org?part=6
Could a concurrent interface disable cause a memory leak later in this
function?If a thread is preempted after passing the mesh_iface check but before
acquiring aggr_list.lock, a concurrent batadv_hardif_disable_interface() could
lock and flush the aggr_list and permanently cancel the aggr_wq worker.When the preempted thread resumes, it takes the lock and queues the skb to the
aggr_list. Since the work queue is permanently cancelled and the list is not
explicitly purged upon destruction (batadv_hardif_release() only calls
kfree_rcu()), does the queued skb leak indefinitely?
Updated by Sven Eckelmann 5 months ago
@Antonio Quartulli @Marek Lindner @Linus Lüssing I don't know who is currently developing B.A.T.M.A.N. V but this has to be fixed by one of you.
Updated by Linus Lüssing 5 months ago
The report seems valid, this is how I think I can reliably reproduce it.
Some added mdelay()'s:
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index 9e693cf0f761..943b0aa5d3ca 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -242,6 +242,7 @@ static void batadv_v_ogm_queue_on_if(struct batadv_priv *bat_priv,
struct sk_buff *skb,
struct batadv_hard_iface *hard_iface)
{
+ //printk("~~~ %s:%i: start\n", __func__, __LINE__);
if (hard_iface->mesh_iface != bat_priv->mesh_iface) {
kfree_skb(skb);
return;
@@ -252,10 +253,28 @@ static void batadv_v_ogm_queue_on_if(struct batadv_priv *bat_priv,
return;
}
+// if (hard_iface->if_status == BATADV_IF_NOT_IN_USE) {
+// printk("~~~ %s:%i: before locking, sleeping\n", __func__, __LINE__);
+ // delay until after batadv_v_ogm_iface_disable()
+ mdelay(50);
+ mdelay(50);
+ mdelay(50);
+ mdelay(50);
+ mdelay(50);
+// printk("~~~ %s:%i: locking\n", __func__, __LINE__);
+// }
+
spin_lock_bh(&hard_iface->bat_v.aggr_list.lock);
if (!batadv_v_ogm_queue_left(skb, hard_iface))
batadv_v_ogm_aggr_send(bat_priv, hard_iface);
+ if (hard_iface->if_status == BATADV_IF_NOT_IN_USE)
+ printk("~~~ %s:%i: in lock, hard-iface not in use?\n", __func__, __LINE__);
+ if (hard_iface->mesh_iface != bat_priv->mesh_iface)
+ printk("~~~ %s:%i: in lock, hard_iface->mesh_iface != bat_priv->mesh_iface\n", __func__, __LINE__);
+
+
+
hard_iface->bat_v.aggr_len += batadv_v_ogm_len(skb);
__skb_queue_tail(&hard_iface->bat_v.aggr_list, skb);
spin_unlock_bh(&hard_iface->bat_v.aggr_list.lock);
@@ -430,9 +449,11 @@ void batadv_v_ogm_iface_disable(struct batadv_hard_iface *hard_iface)
{
cancel_delayed_work_sync(&hard_iface->bat_v.aggr_wq);
+printk("~~~ %s:%i: before lock\n", __func__, __LINE__);
spin_lock_bh(&hard_iface->bat_v.aggr_list.lock);
batadv_v_ogm_aggr_list_free(hard_iface);
spin_unlock_bh(&hard_iface->bat_v.aggr_list.lock);
+printk("~~~ %s:%i: after lock\n", __func__, __LINE__);
}
/**
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index ca9f5c37fb54..fe95edeff1f5 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -857,7 +857,9 @@ void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface)
batadv_hardif_put(new_if);
}
+printk("~~~ %s:%i: before iface.disable\n", __func__, __LINE__);
bat_priv->algo_ops->iface.disable(hard_iface);
+printk("~~~ %s:%i: after iface.disable\n", __func__, __LINE__);
hard_iface->if_status = BATADV_IF_NOT_IN_USE;
/* delete all references to this hard_iface */
@@ -872,7 +874,9 @@ void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface)
if (batadv_hardif_cnt(hard_iface->mesh_iface) <= 1)
batadv_gw_check_client_stop(bat_priv);
+printk("~~~ %s:%i: before mesh_iface = NULL\n", __func__, __LINE__);
hard_iface->mesh_iface = NULL;
+printk("~~~ %s:%i: after mesh_iface = NULL\n", __func__, __LINE__);
batadv_hardif_put(hard_iface);
out:
And on a console in a VM running 7.1.0-rc4+:
ip link add dev brtest type bridge; ip link set up dev brtest
NUM_NODES=100; for i in `seq 1 $NUM_NODES`; do ip link add dev veth$i-in type veth peer name veth$i-out; ip link set dev veth$i-out master brtest up; ip link set dev brtest up; done; dmesg -C; echo clear > /sys/kernel/debug/kmemleak
echo clear > /sys/kernel/debug/kmemleak; sleep 5; insmod net/batman-adv/batman-adv.ko; batctl routing_algo BATMAN_V; for i in `seq 1 $NUM_NODES`; do ip link add dev bat$i type batadv; batctl meshif bat$i it 40; ip link set dev veth$i-in master bat$i up; ip link set dev bat$i up; done; sleep 5; rmmod batman-adv; sleep 30; echo scan > /sys/kernel/debug/kmemleak; sleep 5; cat /sys/kernel/debug/kmemleak
Result:
root@linus-work:/mnt/net/mesh/batman/batman-adv# cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff8e850a212a40 (size 704):
comm "kworker/u32:3", pid 3692, jiffies 4295664365
hex dump (first 32 bytes):
6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
backtrace (crc 67b3cd19):
kmem_cache_alloc_node_noprof+0x4af/0x700
kmalloc_reserve+0x86/0x100
__alloc_skb+0xf4/0x240
__netdev_alloc_skb+0x3a/0x330
0xffffffffc0d8bfd7
process_one_work+0x228/0x600
worker_thread+0x1ee/0x3d0
kthread+0xf4/0x130
ret_from_fork+0x38e/0x420
ret_from_fork_asm+0x1a/0x30
unreferenced object 0xffff8e8505920ac0 (size 232):
comm "kworker/u32:3", pid 3692, jiffies 4295664365
hex dump (first 32 bytes):
88 da f0 39 85 8e ff ff 88 da f0 39 85 8e ff ff ...9.......9....
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc 781e300e):
kmem_cache_alloc_noprof+0x5be/0x6a0
skb_clone+0x63/0x1e0
0xffffffffc0d8c102
process_one_work+0x228/0x600
worker_thread+0x1ee/0x3d0
kthread+0xf4/0x130
ret_from_fork+0x38e/0x420
ret_from_fork_asm+0x1a/0x30
unreferenced object 0xffff8e8509861c40 (size 704):
comm "kworker/u32:3", pid 3692, jiffies 4295664491
hex dump (first 32 bytes):
6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
backtrace (crc d541e123):
kmem_cache_alloc_node_noprof+0x4af/0x700
kmalloc_reserve+0x86/0x100
__alloc_skb+0xf4/0x240
__netdev_alloc_skb+0x3a/0x330
0xffffffffc0d8bfd7
process_one_work+0x228/0x600
worker_thread+0x1ee/0x3d0
kthread+0xf4/0x130
ret_from_fork+0x38e/0x420
ret_from_fork_asm+0x1a/0x30
unreferenced object 0xffff8e850e88a1c0 (size 232):
comm "kworker/u32:3", pid 3692, jiffies 4295664491
hex dump (first 32 bytes):
88 aa f0 39 85 8e ff ff 88 aa f0 39 85 8e ff ff ...9.......9....
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc dbcc6002):
kmem_cache_alloc_noprof+0x5be/0x6a0
skb_clone+0x63/0x1e0
0xffffffffc0d8c102
process_one_work+0x228/0x600
worker_thread+0x1ee/0x3d0
kthread+0xf4/0x130
ret_from_fork+0x38e/0x420
ret_from_fork_asm+0x1a/0x30
unreferenced object 0xffff8e850a5dd440 (size 704):
comm "kworker/u32:0", pid 6197, jiffies 4295690210
hex dump (first 32 bytes):
6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
backtrace (crc 513a5524):
kmem_cache_alloc_node_noprof+0x4af/0x700
kmalloc_reserve+0x86/0x100
__alloc_skb+0xf4/0x240
__netdev_alloc_skb+0x3a/0x330
0xffffffffc0d8bfd7
process_one_work+0x228/0x600
worker_thread+0x1ee/0x3d0
kthread+0xf4/0x130
ret_from_fork+0x38e/0x420
ret_from_fork_asm+0x1a/0x30
unreferenced object 0xffff8e850abff540 (size 232):
comm "kworker/u32:0", pid 6197, jiffies 4295690210
hex dump (first 32 bytes):
88 86 08 21 85 8e ff ff 88 86 08 21 85 8e ff ff ...!.......!....
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc b2b116e):
kmem_cache_alloc_noprof+0x5be/0x6a0
skb_clone+0x63/0x1e0
0xffffffffc0d8c102
process_one_work+0x228/0x600
worker_thread+0x1ee/0x3d0
kthread+0xf4/0x130
ret_from_fork+0x38e/0x420
ret_from_fork_asm+0x1a/0x30
[...]
Will try to come up with a fix for it.
Updated by Sven Eckelmann 4 months ago
- Target version set to 2026.2
Updated by Sven Eckelmann 4 months ago
- Status changed from In Progress to Resolved