Skip to contentSkip to conversation

net: bonding: fix use-after-free in bond_xmit_broadcast()

PATCH In review Kevin Berry 3 participants 15 messages Apr 28 – Jun 16 1 message not in the archive

Subsystems (3)
Network drivers Stable branch Bonding driver
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Kevin Berry

author
#1 lore raw

PATCH v2 1/1 net: bonding: fix use-after-free in bond_xmit_broadcast()

From: Xiang Mei <[email protected]>

bond_xmit_broadcast() reuses the original skb for the last slave (determined by bond_is_last_slave()) and clones it for others. Concurrent slave enslave/release can mutate the slave list during RCU-protected iteration, changing which slave is "last" mid-loop. This causes the original skb to be double-consumed (double-freed).

Replace the racy bond_is_last_slave() check with a simple index comparison (i + 1 == slaves_count) against the pre-snapshot slave count taken via READ_ONCE() before the loop. This preserves the zero-copy optimization for the last slave while making the "last" determination stable against concurrent list mutations.

The UAF can trigger the following crash:

==================================================================

BUG: KASAN: slab-use-after-free in skb_clone
Read of size 8 at addr ffff888100ef8d40 by task exploit/147

CPU: 1 UID: 0 PID: 147 Comm: exploit Not tainted 7.0.0-rc3+ #4 PREEMPTLAZY
Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:123)
 print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:597)
 skb_clone (include/linux/skbuff.h:1724 include/linux/skbuff.h:1792 include/linux/skbuff.h:3396 net/core/skbuff.c:2108)
 bond_xmit_broadcast (drivers/net/bonding/bond_main.c:5334)
 bond_start_xmit (drivers/net/bonding/bond_main.c:5567 drivers/net/bonding/bond_main.c:5593)
 dev_hard_start_xmit (include/linux/netdevice.h:5325 include/linux/netdevice.h:5334 net/core/dev.c:3871 net/core/dev.c:3887)
 __dev_queue_xmit (include/linux/netdevice.h:3601 net/core/dev.c:4838)
 ip6_finish_output2 (include/net/neighbour.h:540 include/net/neighbour.h:554 net/ipv6/ip6_output.c:136)
 ip6_finish_output (net/ipv6/ip6_output.c:208 net/ipv6/ip6_output.c:219)
 ip6_output (net/ipv6/ip6_output.c:250)
 ip6_send_skb (net/ipv6/ip6_output.c:1985)
 udp_v6_send_skb (net/ipv6/udp.c:1442)
 udpv6_sendmsg (net/ipv6/udp.c:1733)
 __sys_sendto (net/socket.c:730 net/socket.c:742 net/socket.c:2206)
 __x64_sys_sendto (net/socket.c:2209)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Allocated by task 147:

Freed by task 147:

The buggy address belongs to the object at ffff888100ef8c80
 which belongs to the cache skbuff_head_cache of size 224

The buggy address is located 192 bytes inside of
freed 224-byte region [ffff888100ef8c80, ffff888100ef8d60)

Memory state around the buggy address:
 ffff888100ef8c00: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888100ef8c80: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888100ef8d00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                                                    ^
 ffff888100ef8d80: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
 ffff888100ef8e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Fixes 4e5bd03ae346 ("net: bonding: fix bond_xmit_broadcast return value error bug")
Reported-by Weiming Shi <[email protected]>
Signed-off-by Xiang Mei <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Kevin Berry <[email protected]>
---
 drivers/net/bonding/bond_main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/net/bonding/bond_main.c +8 -1
index 5035cfa74f1a..9f1a189d46f1 100644
@@ -5326,14 +5326,21 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
 	struct list_head *iter;
 	bool xmit_suc = false;
 	bool skb_used = false;
+	int slaves_count, i = 0;
 
+	slaves_count = READ_ONCE(bond->slave_cnt);
 	bond_for_each_slave_rcu(bond, slave, iter) {
 		struct sk_buff *skb2;
+		bool is_last;
+
+		if (++i > slaves_count)
+			break;
+		is_last = (i == slaves_count);
 
 		if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
 			continue;
 
-		if (bond_is_last_slave(bond, slave)) {
+		if (is_last) {
 			skb2 = skb;
 			skb_used = true;
 		} else {

base-commit: c286ea5e62389897291fa742d2bb909ecc9ef2d0

Signature · 1 line
2.54.0.545.g6539524ca2-goog
Acked-by on v2 1/1
#2 lore raw

Thanks Kevin for picking this up for 6.12.y.
We can't backport the upstream patch since the upstream rewrite that introduced the bond_up_slave snapshot array isn't in this tree. Confirmed it fixes the double-free without introducing concerning side effects.

Acked-by Xiang Mei <[email protected]>
Quoted 103 lines · Kevin Berry

From: Xiang Mei <[email protected]>

bond_xmit_broadcast() reuses the original skb for the last slave (determined by bond_is_last_slave()) and clones it for others. Concurrent slave enslave/release can mutate the slave list during RCU-protected iteration, changing which slave is "last" mid-loop. This causes the original skb to be double-consumed (double-freed).

Replace the racy bond_is_last_slave() check with a simple index comparison (i + 1 == slaves_count) against the pre-snapshot slave count taken via READ_ONCE() before the loop. This preserves the zero-copy optimization for the last slave while making the "last" determination stable against concurrent list mutations.

The UAF can trigger the following crash:

==================================================================

BUG: KASAN: slab-use-after-free in skb_clone
Read of size 8 at addr ffff888100ef8d40 by task exploit/147

CPU: 1 UID: 0 PID: 147 Comm: exploit Not tainted 7.0.0-rc3+ #4 PREEMPTLAZY
Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:123)
 print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:597)
 skb_clone (include/linux/skbuff.h:1724 include/linux/skbuff.h:1792 include/linux/skbuff.h:3396 net/core/skbuff.c:2108)
 bond_xmit_broadcast (drivers/net/bonding/bond_main.c:5334)
 bond_start_xmit (drivers/net/bonding/bond_main.c:5567 drivers/net/bonding/bond_main.c:5593)
 dev_hard_start_xmit (include/linux/netdevice.h:5325 include/linux/netdevice.h:5334 net/core/dev.c:3871 net/core/dev.c:3887)
 __dev_queue_xmit (include/linux/netdevice.h:3601 net/core/dev.c:4838)
 ip6_finish_output2 (include/net/neighbour.h:540 include/net/neighbour.h:554 net/ipv6/ip6_output.c:136)
 ip6_finish_output (net/ipv6/ip6_output.c:208 net/ipv6/ip6_output.c:219)
 ip6_output (net/ipv6/ip6_output.c:250)
 ip6_send_skb (net/ipv6/ip6_output.c:1985)
 udp_v6_send_skb (net/ipv6/udp.c:1442)
 udpv6_sendmsg (net/ipv6/udp.c:1733)
 __sys_sendto (net/socket.c:730 net/socket.c:742 net/socket.c:2206)
 __x64_sys_sendto (net/socket.c:2209)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Allocated by task 147:

Freed by task 147:

The buggy address belongs to the object at ffff888100ef8c80
 which belongs to the cache skbuff_head_cache of size 224

The buggy address is located 192 bytes inside of
freed 224-byte region [ffff888100ef8c80, ffff888100ef8d60)

Memory state around the buggy address:
 ffff888100ef8c00: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888100ef8c80: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888100ef8d00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                                                    ^
 ffff888100ef8d80: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
 ffff888100ef8e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Fixes 4e5bd03ae346 ("net: bonding: fix bond_xmit_broadcast return value error bug")
Reported-by Weiming Shi <[email protected]>
Signed-off-by Xiang Mei <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Kevin Berry <[email protected]>
---
 drivers/net/bonding/bond_main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/net/bonding/bond_main.c +8 -1
index 5035cfa74f1a..9f1a189d46f1 100644
@@ -5326,14 +5326,21 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
        struct list_head *iter;
        bool xmit_suc = false;
        bool skb_used = false;
+       int slaves_count, i = 0;
 
+       slaves_count = READ_ONCE(bond->slave_cnt);
        bond_for_each_slave_rcu(bond, slave, iter) {
                struct sk_buff *skb2;
+               bool is_last;
+
+               if (++i > slaves_count)
+                       break;
+               is_last = (i == slaves_count);
 
                if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
                        continue;
 
-               if (bond_is_last_slave(bond, slave)) {
+               if (is_last) {
                        skb2 = skb;
                        skb_used = true;
                } else {

base-commit: c286ea5e62389897291fa742d2bb909ecc9ef2d0 --
2.54.0.545.g6539524ca2-goog

author
#3 lore raw

Hi all,

Sending the patch for 6.6 as well since it should be the same as the 6.12 patch. Tested by compiling.

Thanks,

Kevin

author
#4 lore raw

PATCH 1/1 net: bonding: fix use-after-free in bond_xmit_broadcast()

From: Xiang Mei <[email protected]>

commit 2884bf72fb8f03409e423397319205de48adca16 upstream.

bond_xmit_broadcast() reuses the original skb for the last slave (determined by bond_is_last_slave()) and clones it for others. Concurrent slave enslave/release can mutate the slave list during RCU-protected iteration, changing which slave is "last" mid-loop. This causes the original skb to be double-consumed (double-freed).

Replace the racy bond_is_last_slave() check with a simple index comparison (i + 1 == slaves_count) against the pre-snapshot slave count taken via READ_ONCE() before the loop. This preserves the zero-copy optimization for the last slave while making the "last" determination stable against concurrent list mutations.

The UAF can trigger the following crash:

==================================================================

BUG: KASAN: slab-use-after-free in skb_clone
Read of size 8 at addr ffff888100ef8d40 by task exploit/147

CPU: 1 UID: 0 PID: 147 Comm: exploit Not tainted 7.0.0-rc3+ #4 PREEMPTLAZY
Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:123)
 print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:597)
 skb_clone (include/linux/skbuff.h:1724 include/linux/skbuff.h:1792 include/linux/skbuff.h:3396 net/core/skbuff.c:2108)
 bond_xmit_broadcast (drivers/net/bonding/bond_main.c:5334)
 bond_start_xmit (drivers/net/bonding/bond_main.c:5567 drivers/net/bonding/bond_main.c:5593)
 dev_hard_start_xmit (include/linux/netdevice.h:5325 include/linux/netdevice.h:5334 net/core/dev.c:3871 net/core/dev.c:3887)
 __dev_queue_xmit (include/linux/netdevice.h:3601 net/core/dev.c:4838)
 ip6_finish_output2 (include/net/neighbour.h:540 include/net/neighbour.h:554 net/ipv6/ip6_output.c:136)
 ip6_finish_output (net/ipv6/ip6_output.c:208 net/ipv6/ip6_output.c:219)
 ip6_output (net/ipv6/ip6_output.c:250)
 ip6_send_skb (net/ipv6/ip6_output.c:1985)
 udp_v6_send_skb (net/ipv6/udp.c:1442)
 udpv6_sendmsg (net/ipv6/udp.c:1733)
 __sys_sendto (net/socket.c:730 net/socket.c:742 net/socket.c:2206)
 __x64_sys_sendto (net/socket.c:2209)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Allocated by task 147:

Freed by task 147:

The buggy address belongs to the object at ffff888100ef8c80
 which belongs to the cache skbuff_head_cache of size 224

The buggy address is located 192 bytes inside of
freed 224-byte region [ffff888100ef8c80, ffff888100ef8d60)

Memory state around the buggy address:
 ffff888100ef8c00: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888100ef8c80: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888100ef8d00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                                                    ^
 ffff888100ef8d80: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
 ffff888100ef8e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Fixes 4e5bd03ae346 ("net: bonding: fix bond_xmit_broadcast return value error bug")
Reported-by Weiming Shi <[email protected]>
Signed-off-by Xiang Mei <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Paolo Abeni <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
Signed-off-by Kevin Berry <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/net/bonding/bond_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/net/bonding/bond_main.c +8 -4
index 114ebaa284da..6484ba1ab14c 100644
@@ -5280,18 +5280,22 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
 				       struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave = NULL;
-	struct list_head *iter;
+	struct bond_up_slave *slaves;
 	bool xmit_suc = false;
 	bool skb_used = false;
+	int slaves_count, i;
 
-	bond_for_each_slave_rcu(bond, slave, iter) {
+	slaves = rcu_dereference(bond->all_slaves);
+
+	slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
+	for (i = 0; i < slaves_count; i++) {
+		struct slave *slave = slaves->arr[i];
 		struct sk_buff *skb2;
 
 		if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
 			continue;
 
-		if (bond_is_last_slave(bond, slave)) {
+		if (i + 1 == slaves_count) {
 			skb2 = skb;
 			skb_used = true;
 		} else {

base-commit: 258cf62a6dfde3c6a39d120a56a298f2ed6a8901

Signature · 1 line
2.54.0.563.g4f69b47b94-goog
author
#5 lore raw

Hi all,

Just a quick reminder about the patch for the 6.6 tree above. Any concerns with including that?

Thanks,

Kevin

maintainer
#6 lore raw
Quoted 3 lines · Kevin Berry

Hi all,

Just a quick reminder about the patch for the 6.6 tree above.

I don't see anything "above" here, sorry.

confused,

greg k-h

author
#7 lore raw

PATCH 1/1 net: bonding: fix use-after-free in bond_xmit_broadcast()

From: Xiang Mei <[email protected]>

commit 2884bf72fb8f03409e423397319205de48adca16 upstream.

bond_xmit_broadcast() reuses the original skb for the last slave (determined by bond_is_last_slave()) and clones it for others. Concurrent slave enslave/release can mutate the slave list during RCU-protected iteration, changing which slave is "last" mid-loop. This causes the original skb to be double-consumed (double-freed).

Replace the racy bond_is_last_slave() check with a simple index comparison (i + 1 == slaves_count) against the pre-snapshot slave count taken via READ_ONCE() before the loop. This preserves the zero-copy optimization for the last slave while making the "last" determination stable against concurrent list mutations.

The UAF can trigger the following crash:

==================================================================

BUG: KASAN: slab-use-after-free in skb_clone
Read of size 8 at addr ffff888100ef8d40 by task exploit/147

CPU: 1 UID: 0 PID: 147 Comm: exploit Not tainted 7.0.0-rc3+ #4 PREEMPTLAZY
Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:123)
 print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:597)
 skb_clone (include/linux/skbuff.h:1724 include/linux/skbuff.h:1792 include/linux/skbuff.h:3396 net/core/skbuff.c:2108)
 bond_xmit_broadcast (drivers/net/bonding/bond_main.c:5334)
 bond_start_xmit (drivers/net/bonding/bond_main.c:5567 drivers/net/bonding/bond_main.c:5593)
 dev_hard_start_xmit (include/linux/netdevice.h:5325 include/linux/netdevice.h:5334 net/core/dev.c:3871 net/core/dev.c:3887)
 __dev_queue_xmit (include/linux/netdevice.h:3601 net/core/dev.c:4838)
 ip6_finish_output2 (include/net/neighbour.h:540 include/net/neighbour.h:554 net/ipv6/ip6_output.c:136)
 ip6_finish_output (net/ipv6/ip6_output.c:208 net/ipv6/ip6_output.c:219)
 ip6_output (net/ipv6/ip6_output.c:250)
 ip6_send_skb (net/ipv6/ip6_output.c:1985)
 udp_v6_send_skb (net/ipv6/udp.c:1442)
 udpv6_sendmsg (net/ipv6/udp.c:1733)
 __sys_sendto (net/socket.c:730 net/socket.c:742 net/socket.c:2206)
 __x64_sys_sendto (net/socket.c:2209)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Allocated by task 147:

Freed by task 147:

The buggy address belongs to the object at ffff888100ef8c80
 which belongs to the cache skbuff_head_cache of size 224

The buggy address is located 192 bytes inside of
freed 224-byte region [ffff888100ef8c80, ffff888100ef8d60)

Memory state around the buggy address:
 ffff888100ef8c00: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888100ef8c80: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888100ef8d00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                                                    ^
 ffff888100ef8d80: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
 ffff888100ef8e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Fixes 4e5bd03ae346 ("net: bonding: fix bond_xmit_broadcast return value error bug")
Reported-by Weiming Shi <[email protected]>
Signed-off-by Xiang Mei <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Paolo Abeni <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
Signed-off-by Kevin Berry <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/net/bonding/bond_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/net/bonding/bond_main.c +8 -4
index 114ebaa284da..6484ba1ab14c 100644
@@ -5280,18 +5280,22 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
 				       struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave = NULL;
-	struct list_head *iter;
+	struct bond_up_slave *slaves;
 	bool xmit_suc = false;
 	bool skb_used = false;
+	int slaves_count, i;
 
-	bond_for_each_slave_rcu(bond, slave, iter) {
+	slaves = rcu_dereference(bond->all_slaves);
+
+	slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
+	for (i = 0; i < slaves_count; i++) {
+		struct slave *slave = slaves->arr[i];
 		struct sk_buff *skb2;
 
 		if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
 			continue;
 
-		if (bond_is_last_slave(bond, slave)) {
+		if (i + 1 == slaves_count) {
 			skb2 = skb;
 			skb_used = true;
 		} else {

base-commit: 258cf62a6dfde3c6a39d120a56a298f2ed6a8901

Signature · 1 line
2.54.0.563.g4f69b47b94-goog
maintainer
#8 lore raw
Quoted 71 lines · Kevin Berry

From: Xiang Mei <[email protected]>

commit 2884bf72fb8f03409e423397319205de48adca16 upstream.

bond_xmit_broadcast() reuses the original skb for the last slave (determined by bond_is_last_slave()) and clones it for others. Concurrent slave enslave/release can mutate the slave list during RCU-protected iteration, changing which slave is "last" mid-loop. This causes the original skb to be double-consumed (double-freed).

Replace the racy bond_is_last_slave() check with a simple index comparison (i + 1 == slaves_count) against the pre-snapshot slave count taken via READ_ONCE() before the loop. This preserves the zero-copy optimization for the last slave while making the "last" determination stable against concurrent list mutations.

The UAF can trigger the following crash:

==================================================================

BUG: KASAN: slab-use-after-free in skb_clone
Read of size 8 at addr ffff888100ef8d40 by task exploit/147

CPU: 1 UID: 0 PID: 147 Comm: exploit Not tainted 7.0.0-rc3+ #4 PREEMPTLAZY
Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:123)
 print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:597)
 skb_clone (include/linux/skbuff.h:1724 include/linux/skbuff.h:1792 include/linux/skbuff.h:3396 net/core/skbuff.c:2108)
 bond_xmit_broadcast (drivers/net/bonding/bond_main.c:5334)
 bond_start_xmit (drivers/net/bonding/bond_main.c:5567 drivers/net/bonding/bond_main.c:5593)
 dev_hard_start_xmit (include/linux/netdevice.h:5325 include/linux/netdevice.h:5334 net/core/dev.c:3871 net/core/dev.c:3887)
 __dev_queue_xmit (include/linux/netdevice.h:3601 net/core/dev.c:4838)
 ip6_finish_output2 (include/net/neighbour.h:540 include/net/neighbour.h:554 net/ipv6/ip6_output.c:136)
 ip6_finish_output (net/ipv6/ip6_output.c:208 net/ipv6/ip6_output.c:219)
 ip6_output (net/ipv6/ip6_output.c:250)
 ip6_send_skb (net/ipv6/ip6_output.c:1985)
 udp_v6_send_skb (net/ipv6/udp.c:1442)
 udpv6_sendmsg (net/ipv6/udp.c:1733)
 __sys_sendto (net/socket.c:730 net/socket.c:742 net/socket.c:2206)
 __x64_sys_sendto (net/socket.c:2209)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Allocated by task 147:

Freed by task 147:

The buggy address belongs to the object at ffff888100ef8c80
 which belongs to the cache skbuff_head_cache of size 224

The buggy address is located 192 bytes inside of
freed 224-byte region [ffff888100ef8c80, ffff888100ef8d60)

Memory state around the buggy address:
 ffff888100ef8c00: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888100ef8c80: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888100ef8d00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                                                    ^
 ffff888100ef8d80: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
 ffff888100ef8e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Fixes 4e5bd03ae346 ("net: bonding: fix bond_xmit_broadcast return value error bug")
Reported-by Weiming Shi <[email protected]>
Signed-off-by Xiang Mei <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Paolo Abeni <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
Signed-off-by Kevin Berry <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
Quoted 3 lines
---
 drivers/net/bonding/bond_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

What kernel tree(s) is this for?

thanks,

greg k-h

author
#9 lore raw
Quoted 1 line

What kernel tree(s) is this for?

It's for the 6.6 stable tree. I used 258cf62a6dfd as the base commit (v6.6.137), but I checked this morning and it applied cleanly to v6.6.140 as well.

Thanks,

Kevin

maintainer
#10 lore raw
In reply to Kevin Berry
Quoted 107 lines · Kevin Berry

From: Xiang Mei <[email protected]>

commit 2884bf72fb8f03409e423397319205de48adca16 upstream.

bond_xmit_broadcast() reuses the original skb for the last slave (determined by bond_is_last_slave()) and clones it for others. Concurrent slave enslave/release can mutate the slave list during RCU-protected iteration, changing which slave is "last" mid-loop. This causes the original skb to be double-consumed (double-freed).

Replace the racy bond_is_last_slave() check with a simple index comparison (i + 1 == slaves_count) against the pre-snapshot slave count taken via READ_ONCE() before the loop. This preserves the zero-copy optimization for the last slave while making the "last" determination stable against concurrent list mutations.

The UAF can trigger the following crash:

==================================================================

BUG: KASAN: slab-use-after-free in skb_clone
Read of size 8 at addr ffff888100ef8d40 by task exploit/147

CPU: 1 UID: 0 PID: 147 Comm: exploit Not tainted 7.0.0-rc3+ #4 PREEMPTLAZY
Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:123)
 print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:597)
 skb_clone (include/linux/skbuff.h:1724 include/linux/skbuff.h:1792 include/linux/skbuff.h:3396 net/core/skbuff.c:2108)
 bond_xmit_broadcast (drivers/net/bonding/bond_main.c:5334)
 bond_start_xmit (drivers/net/bonding/bond_main.c:5567 drivers/net/bonding/bond_main.c:5593)
 dev_hard_start_xmit (include/linux/netdevice.h:5325 include/linux/netdevice.h:5334 net/core/dev.c:3871 net/core/dev.c:3887)
 __dev_queue_xmit (include/linux/netdevice.h:3601 net/core/dev.c:4838)
 ip6_finish_output2 (include/net/neighbour.h:540 include/net/neighbour.h:554 net/ipv6/ip6_output.c:136)
 ip6_finish_output (net/ipv6/ip6_output.c:208 net/ipv6/ip6_output.c:219)
 ip6_output (net/ipv6/ip6_output.c:250)
 ip6_send_skb (net/ipv6/ip6_output.c:1985)
 udp_v6_send_skb (net/ipv6/udp.c:1442)
 udpv6_sendmsg (net/ipv6/udp.c:1733)
 __sys_sendto (net/socket.c:730 net/socket.c:742 net/socket.c:2206)
 __x64_sys_sendto (net/socket.c:2209)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Allocated by task 147:

Freed by task 147:

The buggy address belongs to the object at ffff888100ef8c80
 which belongs to the cache skbuff_head_cache of size 224

The buggy address is located 192 bytes inside of
freed 224-byte region [ffff888100ef8c80, ffff888100ef8d60)

Memory state around the buggy address:
 ffff888100ef8c00: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888100ef8c80: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888100ef8d00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                                                    ^
 ffff888100ef8d80: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
 ffff888100ef8e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Fixes 4e5bd03ae346 ("net: bonding: fix bond_xmit_broadcast return value error bug")
Reported-by Weiming Shi <[email protected]>
Signed-off-by Xiang Mei <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Paolo Abeni <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
Signed-off-by Kevin Berry <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/net/bonding/bond_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/net/bonding/bond_main.c +8 -4
index 114ebaa284da..6484ba1ab14c 100644
@@ -5280,18 +5280,22 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
 				       struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave = NULL;
-	struct list_head *iter;
+	struct bond_up_slave *slaves;
 	bool xmit_suc = false;
 	bool skb_used = false;
+	int slaves_count, i;
 
-	bond_for_each_slave_rcu(bond, slave, iter) {
+	slaves = rcu_dereference(bond->all_slaves);
+
+	slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
+	for (i = 0; i < slaves_count; i++) {
+		struct slave *slave = slaves->arr[i];
 		struct sk_buff *skb2;
 
 		if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
 			continue;
 
-		if (bond_is_last_slave(bond, slave)) {
+		if (i + 1 == slaves_count) {
 			skb2 = skb;
 			skb_used = true;
 		} else {
Quoted 5 lines

base-commit: 258cf62a6dfde3c6a39d120a56a298f2ed6a8901

Signature · 1 line
2.54.0.563.g4f69b47b94-goog

Does not apply at all :(

author
#11 lore raw

Hm, looks like the patch context changed because commit ce7a381697cb3 ("net: bonding: add broadcast_neighbor option for 802.3ad") was applied to the 6.6 stable tree after I sent my patch. With that, though, Xiang's original commit 2884bf72fb8f ("net: bonding: fix use-after-free in bond_xmit_broadcast()") applies cleanly to 6.6 and should fix the double-free. Would it be possible to cherry-pick that?

-Kevin

Quoted 116 lines · Greg Kroah-Hartman
Quoted 107 lines · Kevin Berry

From: Xiang Mei <[email protected]>

commit 2884bf72fb8f03409e423397319205de48adca16 upstream.

bond_xmit_broadcast() reuses the original skb for the last slave (determined by bond_is_last_slave()) and clones it for others. Concurrent slave enslave/release can mutate the slave list during RCU-protected iteration, changing which slave is "last" mid-loop. This causes the original skb to be double-consumed (double-freed).

Replace the racy bond_is_last_slave() check with a simple index comparison (i + 1 == slaves_count) against the pre-snapshot slave count taken via READ_ONCE() before the loop. This preserves the zero-copy optimization for the last slave while making the "last" determination stable against concurrent list mutations.

The UAF can trigger the following crash:

==================================================================

BUG: KASAN: slab-use-after-free in skb_clone
Read of size 8 at addr ffff888100ef8d40 by task exploit/147

CPU: 1 UID: 0 PID: 147 Comm: exploit Not tainted 7.0.0-rc3+ #4 PREEMPTLAZY
Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:123)
 print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:597)
 skb_clone (include/linux/skbuff.h:1724 include/linux/skbuff.h:1792 include/linux/skbuff.h:3396 net/core/skbuff.c:2108)
 bond_xmit_broadcast (drivers/net/bonding/bond_main.c:5334)
 bond_start_xmit (drivers/net/bonding/bond_main.c:5567 drivers/net/bonding/bond_main.c:5593)
 dev_hard_start_xmit (include/linux/netdevice.h:5325 include/linux/netdevice.h:5334 net/core/dev.c:3871 net/core/dev.c:3887)
 __dev_queue_xmit (include/linux/netdevice.h:3601 net/core/dev.c:4838)
 ip6_finish_output2 (include/net/neighbour.h:540 include/net/neighbour.h:554 net/ipv6/ip6_output.c:136)
 ip6_finish_output (net/ipv6/ip6_output.c:208 net/ipv6/ip6_output.c:219)
 ip6_output (net/ipv6/ip6_output.c:250)
 ip6_send_skb (net/ipv6/ip6_output.c:1985)
 udp_v6_send_skb (net/ipv6/udp.c:1442)
 udpv6_sendmsg (net/ipv6/udp.c:1733)
 __sys_sendto (net/socket.c:730 net/socket.c:742 net/socket.c:2206)
 __x64_sys_sendto (net/socket.c:2209)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Allocated by task 147:

Freed by task 147:

The buggy address belongs to the object at ffff888100ef8c80
 which belongs to the cache skbuff_head_cache of size 224

The buggy address is located 192 bytes inside of
freed 224-byte region [ffff888100ef8c80, ffff888100ef8d60)

Memory state around the buggy address:
 ffff888100ef8c00: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888100ef8c80: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888100ef8d00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                                                    ^
 ffff888100ef8d80: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
 ffff888100ef8e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Fixes 4e5bd03ae346 ("net: bonding: fix bond_xmit_broadcast return value error bug")
Reported-by Weiming Shi <[email protected]>
Signed-off-by Xiang Mei <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Paolo Abeni <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
Signed-off-by Kevin Berry <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/net/bonding/bond_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/net/bonding/bond_main.c +8 -4
index 114ebaa284da..6484ba1ab14c 100644
@@ -5280,18 +5280,22 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
                                     struct net_device *bond_dev)
 {
      struct bonding *bond = netdev_priv(bond_dev);
-     struct slave *slave = NULL;
-     struct list_head *iter;
+     struct bond_up_slave *slaves;
      bool xmit_suc = false;
      bool skb_used = false;
+     int slaves_count, i;
 
-     bond_for_each_slave_rcu(bond, slave, iter) {
+     slaves = rcu_dereference(bond->all_slaves);
+
+     slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
+     for (i = 0; i < slaves_count; i++) {
+             struct slave *slave = slaves->arr[i];
              struct sk_buff *skb2;
 
              if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
                      continue;
 
-             if (bond_is_last_slave(bond, slave)) {
+             if (i + 1 == slaves_count) {
                      skb2 = skb;
                      skb_used = true;
              } else {
Quoted 5 lines

base-commit: 258cf62a6dfde3c6a39d120a56a298f2ed6a8901 --
2.54.0.563.g4f69b47b94-goog

Does not apply at all :(

maintainer
#12 lore raw
Quoted 6 lines · Kevin Berry

Hm, looks like the patch context changed because commit ce7a381697cb3 ("net: bonding: add broadcast_neighbor option for 802.3ad") was applied to the 6.6 stable tree after I sent my patch. With that, though, Xiang's original commit 2884bf72fb8f ("net: bonding: fix use-after-free in bond_xmit_broadcast()") applies cleanly to 6.6 and should fix the double-free. Would it be possible to cherry-pick that?

That worked, thanks!

greg k-h

author
#13 lore raw
Quoted 3 lines

That worked, thanks!

greg k-h

Glad to hear it!

It's also worth pointing out that because the fix for 6.12 was done without the ce7a381697cb3 ("net: bonding: add broadcast_neighbor option for 802.3ad") dependency, it conflicts with the c4f050ce06c56 ("bonding: 3ad: implement proper RCU rules for port->aggregator") fix commit series that was applied to other trees. So for 6.12, I think it would make sense to:

1. Revert the fix from this thread: 3453882f36c4 ("net: bonding: fix use-after-free in bond_xmit_broadcast()"),
2. Apply the patch series for c4f050ce06c56 ("bonding: 3ad: implement proper RCU rules for port->aggregator"), namely:
- Apply 4440873f36553 ("bonding: 802.3ad replace MAC_ADDRESS_EQUAL
with __agg_has_partner")
- Apply ce7a381697cb3 ("net: bonding: add broadcast_neighbor
option for 802.3ad")
- Apply 6b6dc81ee7e8c ("bonding: add support for per-port LACP
actor priority")
- Apply 4916f2e2f3fc9 ("bonding: print churn state via netlink")
- Apply c4f050ce06c56 ("bonding: 3ad: implement proper RCU rules
for port->aggregator")
3. Apply Xiang's original fix commit: 2884bf72fb8f ("net: bonding: fix use-after-free in bond_xmit_broadcast()").

That should make things consistent between 6.1, 6.6., and 6.12 with respect to those two fixes.

Thanks,
-Kevin

maintainer
#14 lore raw
Quoted 29 lines · Kevin Berry
Quoted 3 lines

That worked, thanks!

greg k-h

Glad to hear it!

It's also worth pointing out that because the fix for 6.12 was done without the ce7a381697cb3 ("net: bonding: add broadcast_neighbor option for 802.3ad") dependency, it conflicts with the c4f050ce06c56 ("bonding: 3ad: implement proper RCU rules for port->aggregator") fix commit series that was applied to other trees. So for 6.12, I think it would make sense to:

1. Revert the fix from this thread: 3453882f36c4 ("net: bonding: fix use-after-free in bond_xmit_broadcast()"),
2. Apply the patch series for c4f050ce06c56 ("bonding: 3ad: implement proper RCU rules for port->aggregator"), namely:
- Apply 4440873f36553 ("bonding: 802.3ad replace MAC_ADDRESS_EQUAL
with __agg_has_partner")
- Apply ce7a381697cb3 ("net: bonding: add broadcast_neighbor
option for 802.3ad")
- Apply 6b6dc81ee7e8c ("bonding: add support for per-port LACP
actor priority")
- Apply 4916f2e2f3fc9 ("bonding: print churn state via netlink")
- Apply c4f050ce06c56 ("bonding: 3ad: implement proper RCU rules
for port->aggregator")
3. Apply Xiang's original fix commit: 2884bf72fb8f ("net: bonding: fix use-after-free in bond_xmit_broadcast()").

Quoted 2 lines

That should make things consistent between 6.1, 6.6., and 6.12 with respect to those two fixes.

Please send a patch series that does this to make sure I get it correct.

thanks,

greg k-h

author
#15 lore raw
Quoted 1 line

Please send a patch series that does this to make sure I get it correct.

Done, sent as a new patch series.

Thanks,
-Kevin