[PATCH net v5 0/2] net,bpf: fix null-ptr-deref in xdp_master_redirect() for bonding and add selftest

[PATCH net v5 0/2] net,bpf: fix null-ptr-deref in xdp_master_redirect() for bonding and add selftest

[Jiayuan Chen]

syzkaller reported a kernel panic [1] with the following crash stack:

Call Trace:
BUG: unable to handle page fault for address: ffff8ebd08580000
PF: supervisor write access in kernel mode
PF: error_code(0x0002) - not-present page
PGD 11f201067 P4D 11f201067 PUD 0
Oops: Oops: 0002 [#1] SMP PTI
CPU: 2 UID: 0 PID: 451 Comm: test_progs Not tainted 6.19.0+ #161 PREEMPT_RT
RIP: 0010:bond_rr_gen_slave_id+0x90/0xd0
RSP: 0018:ffffd3f4815f3448 EFLAGS: 00010246
RAX: 0000000000000001 RBX: 0000000000000001 RCX: ffff8ebc8728b17e
RDX: 0000000000000000 RSI: ffffd3f4815f3538 RDI: ffff8ebc8abcce40
RBP: ffffd3f4815f3460 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffd3f4815f3538
R13: ffff8ebc8abcce40 R14: ffff8ebc8728b17f R15: ffff8ebc8728b170
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff8ebd08580000 CR3: 000000010a808006 CR4: 0000000000770ef0
PKRU: 55555554
Call Trace:
 <TASK>
 bond_xdp_get_xmit_slave+0xc0/0x240
 xdp_master_redirect+0x74/0xc0
 bpf_prog_run_generic_xdp+0x2f2/0x3f0
 do_xdp_generic+0x1fd/0x3d0
 __netif_receive_skb_core.constprop.0+0x30d/0x1220
 __netif_receive_skb_list_core+0xfc/0x250
 netif_receive_skb_list_internal+0x20c/0x3d0
 ? eth_type_trans+0x137/0x160
 netif_receive_skb_list+0x25/0x140
 xdp_test_run_batch.constprop.0+0x65b/0x6e0
 bpf_test_run_xdp_live+0x1ec/0x3b0
 bpf_prog_test_run_xdp+0x49d/0x6e0
 __sys_bpf+0x446/0x27b0
 __x64_sys_bpf+0x1a/0x30
 x64_sys_call+0x146c/0x26e0
 do_syscall_64+0xd3/0x1510
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Problem Description

bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
when the bond mode is round-robin. If the bond device was never brought
up, rr_tx_counter remains NULL.

The XDP redirect path can reach this code even when the bond is not up:
bpf_master_redirect_enabled_key is a global static key, so when any bond
device has native XDP attached, the XDP_TX -> xdp_master_redirect()
interception is enabled for all bond slaves system-wide.

Solution

Patch 1: Add a NULL check with unlikely() in bond_rr_gen_slave_id() before
dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was never
opened), fall back to get_random_u32() for slave selection. The existing
allocation in bond_open() is kept, with WRITE_ONCE() added to pair with
the READ_ONCE() in the NULL check.
Patch 2: Add a selftest that reproduces the above scenario.

Changes since v4:
https://lore.kernel.org/netdev/20260304074301.35482-1-jiayuan.chen@linux.dev/
- Reverted unconditional alloc in bond_init(); instead add a NULL check
  with unlikely()/READ_ONCE() in bond_rr_gen_slave_id() and WRITE_ONCE()
  in bond_open(), avoiding memory waste for non-RR modes
  (Suggested by Nikolay Aleksandrov, patch by Jay Vosburgh)

Changes since v3:
https://lore.kernel.org/netdev/20260228021918.141002-1-jiayuan.chen@linux.dev/T/#t
- Added code comment and commit log explaining why rr_tx_counter is
  allocated unconditionally for all modes (Suggested by Jay Vosburgh)

Changes since v2:
https://lore.kernel.org/netdev/20260227092254.272603-1-jiayuan.chen@linux.dev/T/#t
- Moved allocation from bond_create_init() helper into bond_init()
  (ndo_init), which is the natural single point covering both creation
  paths and also handles post-creation mode changes to round-robin

Changes since v1:
https://lore.kernel.org/netdev/20260224112545.37888-1-jiayuan.chen@linux.dev/T/#t
- Moved the guard for NULL rr_tx_counter from xdp_master_redirect()
  into the bonding subsystem itself
  (Suggested by Sebastian Andrzej Siewior <bigeasy@linutronix.de>)

[1] https://syzkaller.appspot.com/bug?extid=80e046b8da2820b6ba73

Jiayuan Chen (2):
  bonding: fix null-ptr-deref in bond_rr_gen_slave_id()
  selftests/bpf: add test for xdp_master_redirect with bond not up

 drivers/net/bonding/bond_main.c               |   9 +-
 .../selftests/bpf/prog_tests/xdp_bonding.c    | 101 +++++++++++++++++-
 2 files changed, 106 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Jiayuan Chen]

From: Jiayuan Chen <jiayuan.chen@shopee.com>

bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
when the bond mode is round-robin. If the bond device was never brought
up, rr_tx_counter remains NULL, causing a null-ptr-deref.

The XDP redirect path can reach this code even when the bond is not up:
bpf_master_redirect_enabled_key is a global static key, so when any bond
device has native XDP attached, the XDP_TX -> xdp_master_redirect()
interception is enabled for all bond slaves system-wide. This allows the
path xdp_master_redirect() -> bond_xdp_get_xmit_slave() ->
bond_xdp_xmit_roundrobin_slave_get() -> bond_rr_gen_slave_id() to be
reached on a bond that was never opened.

Fix this by adding a NULL check with unlikely() in bond_rr_gen_slave_id()
before dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was
never opened), fall back to get_random_u32() for slave selection. The
allocation in bond_open() is kept, with WRITE_ONCE() added to safely
publish the pointer to the XDP read side. A plain read suffices for the
!bond->rr_tx_counter guard in bond_open() itself, as bond_open() runs
under RTNL lock and is the only writer of rr_tx_counter.

Fixes: 879af96ffd72 ("net, core: Add support for XDP redirection to slave device")
Reported-by: syzbot+80e046b8da2820b6ba73@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/698f84c6.a70a0220.2c38d7.00cc.GAE@google.com/T/
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
---
 drivers/net/bonding/bond_main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 444519078da3..b8ec87625ce3 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4290,9 +4290,11 @@ static int bond_open(struct net_device *bond_dev)
 	struct slave *slave;
 
 	if (BOND_MODE(bond) == BOND_MODE_ROUNDROBIN && !bond->rr_tx_counter) {
-		bond->rr_tx_counter = alloc_percpu(u32);
-		if (!bond->rr_tx_counter)
+		u32 __percpu *rr_tx_tmp = alloc_percpu(u32);
+
+		if (!rr_tx_tmp)
 			return -ENOMEM;
+		WRITE_ONCE(bond->rr_tx_counter, rr_tx_tmp);
 	}
 
 	/* reset slave->backup and slave->inactive */
@@ -4883,6 +4885,9 @@ static u32 bond_rr_gen_slave_id(struct bonding *bond)
 	struct reciprocal_value reciprocal_packets_per_slave;
 	int packets_per_slave = bond->params.packets_per_slave;
 
+	if (unlikely(!READ_ONCE(bond->rr_tx_counter)))
+		return get_random_u32();
+
 	switch (packets_per_slave) {
 	case 0:
 		slave_id = get_random_u32();
-- 
2.43.0

[PATCH net v5 2/2] selftests/bpf: add test for xdp_master_redirect with bond not up

[Jiayuan Chen]

From: Jiayuan Chen <jiayuan.chen@shopee.com>

Add a selftest that reproduces the null-ptr-deref in
bond_rr_gen_slave_id() when XDP redirect targets a bond device in
round-robin mode that was never brought up. The test verifies the fix
by ensuring no crash occurs.

Test setup:
- bond0: active-backup mode, UP, with native XDP (enables
  bpf_master_redirect_enabled_key globally)
- bond1: round-robin mode, never UP
- veth1: slave of bond1, with generic XDP (XDP_TX)
- BPF_PROG_TEST_RUN with live frames triggers the redirect path

Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
---
 .../selftests/bpf/prog_tests/xdp_bonding.c    | 101 +++++++++++++++++-
 1 file changed, 99 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
index e8ea26464349..0d4ec1e5b401 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
@@ -191,13 +191,18 @@ static int bonding_setup(struct skeletons *skeletons, int mode, int xmit_policy,
 	return -1;
 }
 
-static void bonding_cleanup(struct skeletons *skeletons)
+static void link_cleanup(struct skeletons *skeletons)
 {
-	restore_root_netns();
 	while (skeletons->nlinks) {
 		skeletons->nlinks--;
 		bpf_link__destroy(skeletons->links[skeletons->nlinks]);
 	}
+}
+
+static void bonding_cleanup(struct skeletons *skeletons)
+{
+	restore_root_netns();
+	link_cleanup(skeletons);
 	ASSERT_OK(system("ip link delete bond1"), "delete bond1");
 	ASSERT_OK(system("ip link delete veth1_1"), "delete veth1_1");
 	ASSERT_OK(system("ip link delete veth1_2"), "delete veth1_2");
@@ -493,6 +498,95 @@ static void test_xdp_bonding_nested(struct skeletons *skeletons)
 	system("ip link del bond_nest2");
 }
 
+/*
+ * Test that XDP redirect via xdp_master_redirect() does not crash when
+ * the bond master device is not up. When bond is in round-robin mode but
+ * never opened, rr_tx_counter is NULL.
+ */
+static void test_xdp_bonding_redirect_no_up(struct skeletons *skeletons)
+{
+	struct nstoken *nstoken = NULL;
+	int xdp_pass_fd, xdp_tx_fd;
+	int veth1_ifindex;
+	int err;
+	char pkt[ETH_HLEN + 1];
+	struct xdp_md ctx_in = {};
+
+	DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts,
+			    .data_in = &pkt,
+			    .data_size_in = sizeof(pkt),
+			    .ctx_in = &ctx_in,
+			    .ctx_size_in = sizeof(ctx_in),
+			    .flags = BPF_F_TEST_XDP_LIVE_FRAMES,
+			    .repeat = 1,
+			    .batch_size = 1,
+		);
+
+	/* We can't use bonding_setup() because bond will be active */
+	SYS(out, "ip netns add ns_rr_no_up");
+	nstoken = open_netns("ns_rr_no_up");
+	if (!ASSERT_OK_PTR(nstoken, "open ns_rr_no_up"))
+		goto out;
+
+	/* bond0: active-backup, UP with slave veth0.
+	 * Attaching native XDP to bond0 enables bpf_master_redirect_enabled_key
+	 * globally.
+	 */
+	SYS(out, "ip link add bond0 type bond mode active-backup");
+	SYS(out, "ip link add veth0 type veth peer name veth0p");
+	SYS(out, "ip link set veth0 master bond0");
+	SYS(out, "ip link set bond0 up");
+	SYS(out, "ip link set veth0p up");
+
+	/* bond1: round-robin, never UP -> rr_tx_counter stays NULL */
+	SYS(out, "ip link add bond1 type bond mode balance-rr");
+	SYS(out, "ip link add veth1 type veth peer name veth1p");
+	SYS(out, "ip link set veth1 master bond1");
+
+	veth1_ifindex = if_nametoindex("veth1");
+	if (!ASSERT_GT(veth1_ifindex, 0, "veth1_ifindex"))
+		goto out;
+
+	/* Attach native XDP to bond0 -> enables global redirect key */
+	if (xdp_attach(skeletons, skeletons->xdp_tx->progs.xdp_tx, "bond0"))
+		goto out;
+
+	/* Attach generic XDP (XDP_TX) to veth1.
+	 * When packets arrive at veth1 via netif_receive_skb, do_xdp_generic()
+	 * runs this program. XDP_TX + bond slave triggers xdp_master_redirect().
+	 */
+	xdp_tx_fd = bpf_program__fd(skeletons->xdp_tx->progs.xdp_tx);
+	if (!ASSERT_GE(xdp_tx_fd, 0, "xdp_tx prog_fd"))
+		goto out;
+
+	err = bpf_xdp_attach(veth1_ifindex, xdp_tx_fd,
+			     XDP_FLAGS_SKB_MODE, NULL);
+	if (!ASSERT_OK(err, "attach generic XDP to veth1"))
+		goto out;
+
+	/* Run BPF_PROG_TEST_RUN with XDP_PASS live frames on veth1.
+	 * XDP_PASS frames become SKBs with skb->dev = veth1, entering
+	 * netif_receive_skb -> do_xdp_generic -> xdp_master_redirect.
+	 * Without the fix, bond_rr_gen_slave_id() dereferences NULL
+	 * rr_tx_counter and crashes.
+	 */
+	xdp_pass_fd = bpf_program__fd(skeletons->xdp_dummy->progs.xdp_dummy_prog);
+	if (!ASSERT_GE(xdp_pass_fd, 0, "xdp_pass prog_fd"))
+		goto out;
+
+	memset(pkt, 0, sizeof(pkt));
+	ctx_in.data_end = sizeof(pkt);
+	ctx_in.ingress_ifindex = veth1_ifindex;
+
+	err = bpf_prog_test_run_opts(xdp_pass_fd, &opts);
+	ASSERT_OK(err, "xdp_pass test_run should not crash");
+
+out:
+	link_cleanup(skeletons);
+	close_netns(nstoken);
+	SYS_NOFAIL("ip netns del ns_rr_no_up");
+}
+
 static void test_xdp_bonding_features(struct skeletons *skeletons)
 {
 	LIBBPF_OPTS(bpf_xdp_query_opts, query_opts);
@@ -738,6 +832,9 @@ void serial_test_xdp_bonding(void)
 	if (test__start_subtest("xdp_bonding_redirect_multi"))
 		test_xdp_bonding_redirect_multi(&skeletons);
 
+	if (test__start_subtest("xdp_bonding_redirect_no_up"))
+		test_xdp_bonding_redirect_no_up(&skeletons);
+
 out:
 	xdp_dummy__destroy(skeletons.xdp_dummy);
 	xdp_tx__destroy(skeletons.xdp_tx);
-- 
2.43.0

Re: [PATCH net v5 0/2] net,bpf: fix null-ptr-deref in xdp_master_redirect() for bonding and add selftest

[Eric Dumazet]

On Mon, Mar 9, 2026 at 4:07 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> syzkaller reported a kernel panic [1] with the following crash stack:
>
> Call Trace:
> BUG: unable to handle page fault for address: ffff8ebd08580000
> PF: supervisor write access in kernel mode
> PF: error_code(0x0002) - not-present page
> PGD 11f201067 P4D 11f201067 PUD 0
> Oops: Oops: 0002 [#1] SMP PTI
> CPU: 2 UID: 0 PID: 451 Comm: test_progs Not tainted 6.19.0+ #161 PREEMPT_RT
> RIP: 0010:bond_rr_gen_slave_id+0x90/0xd0
> RSP: 0018:ffffd3f4815f3448 EFLAGS: 00010246
> RAX: 0000000000000001 RBX: 0000000000000001 RCX: ffff8ebc8728b17e
> RDX: 0000000000000000 RSI: ffffd3f4815f3538 RDI: ffff8ebc8abcce40
> RBP: ffffd3f4815f3460 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: ffffd3f4815f3538
> R13: ffff8ebc8abcce40 R14: ffff8ebc8728b17f R15: ffff8ebc8728b170
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffff8ebd08580000 CR3: 000000010a808006 CR4: 0000000000770ef0
> PKRU: 55555554
> Call Trace:
>  <TASK>
>  bond_xdp_get_xmit_slave+0xc0/0x240
>  xdp_master_redirect+0x74/0xc0
>  bpf_prog_run_generic_xdp+0x2f2/0x3f0
>  do_xdp_generic+0x1fd/0x3d0
>  __netif_receive_skb_core.constprop.0+0x30d/0x1220
>  __netif_receive_skb_list_core+0xfc/0x250
>  netif_receive_skb_list_internal+0x20c/0x3d0
>  ? eth_type_trans+0x137/0x160
>  netif_receive_skb_list+0x25/0x140
>  xdp_test_run_batch.constprop.0+0x65b/0x6e0
>  bpf_test_run_xdp_live+0x1ec/0x3b0
>  bpf_prog_test_run_xdp+0x49d/0x6e0
>  __sys_bpf+0x446/0x27b0
>  __x64_sys_bpf+0x1a/0x30
>  x64_sys_call+0x146c/0x26e0
>  do_syscall_64+0xd3/0x1510
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e

Please, can you always provide symbols in such traces ?
You can use scripts/decode_stacktrace.sh to make the trace really
nice, instead of ugly.

>
> Problem Description
>
> bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
> check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
> when the bond mode is round-robin. If the bond device was never brought
> up, rr_tx_counter remains NULL.
>
> The XDP redirect path can reach this code even when the bond is not up:
> bpf_master_redirect_enabled_key is a global static key, so when any bond
> device has native XDP attached, the XDP_TX -> xdp_master_redirect()
> interception is enabled for all bond slaves system-wide.
>
> Solution
>
> Patch 1: Add a NULL check with unlikely() in bond_rr_gen_slave_id() before
> dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was never
> opened), fall back to get_random_u32() for slave selection. The existing
> allocation in bond_open() is kept, with WRITE_ONCE() added to pair with
> the READ_ONCE() in the NULL check.
> Patch 2: Add a selftest that reproduces the above scenario.
>
> Changes since v4:
> https://lore.kernel.org/netdev/20260304074301.35482-1-jiayuan.chen@linux.dev/
> - Reverted unconditional alloc in bond_init(); instead add a NULL check
>   with unlikely()/READ_ONCE() in bond_rr_gen_slave_id() and WRITE_ONCE()
>   in bond_open(), avoiding memory waste for non-RR modes
>   (Suggested by Nikolay Aleksandrov, patch by Jay Vosburgh)
>
> Changes since v3:
> https://lore.kernel.org/netdev/20260228021918.141002-1-jiayuan.chen@linux.dev/T/#t
> - Added code comment and commit log explaining why rr_tx_counter is
>   allocated unconditionally for all modes (Suggested by Jay Vosburgh)
>
> Changes since v2:
> https://lore.kernel.org/netdev/20260227092254.272603-1-jiayuan.chen@linux.dev/T/#t
> - Moved allocation from bond_create_init() helper into bond_init()
>   (ndo_init), which is the natural single point covering both creation
>   paths and also handles post-creation mode changes to round-robin
>
> Changes since v1:
> https://lore.kernel.org/netdev/20260224112545.37888-1-jiayuan.chen@linux.dev/T/#t
> - Moved the guard for NULL rr_tx_counter from xdp_master_redirect()
>   into the bonding subsystem itself
>   (Suggested by Sebastian Andrzej Siewior <bigeasy@linutronix.de>)
>
> [1] https://syzkaller.appspot.com/bug?extid=80e046b8da2820b6ba73
>
> Jiayuan Chen (2):
>   bonding: fix null-ptr-deref in bond_rr_gen_slave_id()
>   selftests/bpf: add test for xdp_master_redirect with bond not up
>
>  drivers/net/bonding/bond_main.c               |   9 +-
>  .../selftests/bpf/prog_tests/xdp_bonding.c    | 101 +++++++++++++++++-
>  2 files changed, 106 insertions(+), 4 deletions(-)
>
> --
> 2.43.0
>

Re: [PATCH net v5 0/2] net,bpf: fix null-ptr-deref in xdp_master_redirect() for bonding and add selftest

[Jiayuan Chen]

March 9, 2026 at 15:46, "Eric Dumazet" <edumazet@google.com mailto:edumazet@google.com?to=%22Eric%20Dumazet%22%20%3Cedumazet%40google.com%3E > wrote:


> 
> On Mon, Mar 9, 2026 at 4:07 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> 
> > 
> > syzkaller reported a kernel panic [1] with the following crash stack:
> > 
> >  Call Trace:
> >  BUG: unable to handle page fault for address: ffff8ebd08580000
> >  PF: supervisor write access in kernel mode
> >  PF: error_code(0x0002) - not-present page
> >  PGD 11f201067 P4D 11f201067 PUD 0
> >  Oops: Oops: 0002 [#1] SMP PTI
> >  CPU: 2 UID: 0 PID: 451 Comm: test_progs Not tainted 6.19.0+ #161 PREEMPT_RT
> >  RIP: 0010:bond_rr_gen_slave_id+0x90/0xd0
> >  RSP: 0018:ffffd3f4815f3448 EFLAGS: 00010246
> >  RAX: 0000000000000001 RBX: 0000000000000001 RCX: ffff8ebc8728b17e
> >  RDX: 0000000000000000 RSI: ffffd3f4815f3538 RDI: ffff8ebc8abcce40
> >  RBP: ffffd3f4815f3460 R08: 0000000000000000 R09: 0000000000000000
> >  R10: 0000000000000000 R11: 0000000000000000 R12: ffffd3f4815f3538
> >  R13: ffff8ebc8abcce40 R14: ffff8ebc8728b17f R15: ffff8ebc8728b170
> >  CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >  CR2: ffff8ebd08580000 CR3: 000000010a808006 CR4: 0000000000770ef0
> >  PKRU: 55555554
> >  Call Trace:
> >  <TASK>
> >  bond_xdp_get_xmit_slave+0xc0/0x240
> >  xdp_master_redirect+0x74/0xc0
> >  bpf_prog_run_generic_xdp+0x2f2/0x3f0
> >  do_xdp_generic+0x1fd/0x3d0
> >  __netif_receive_skb_core.constprop.0+0x30d/0x1220
> >  __netif_receive_skb_list_core+0xfc/0x250
> >  netif_receive_skb_list_internal+0x20c/0x3d0
> >  ? eth_type_trans+0x137/0x160
> >  netif_receive_skb_list+0x25/0x140
> >  xdp_test_run_batch.constprop.0+0x65b/0x6e0
> >  bpf_test_run_xdp_live+0x1ec/0x3b0
> >  bpf_prog_test_run_xdp+0x49d/0x6e0
> >  __sys_bpf+0x446/0x27b0
> >  __x64_sys_bpf+0x1a/0x30
> >  x64_sys_call+0x146c/0x26e0
> >  do_syscall_64+0xd3/0x1510
> >  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > 
> Please, can you always provide symbols in such traces ?
> You can use scripts/decode_stacktrace.sh to make the trace really
> nice, instead of ugly.
> 


Hi Eric,

Thank you for the suggestion. I didn't include the fully decoded stack 
trace in the cover letter because the syzkaller report already contains 
the complete information. You can find it here if needed:

https://syzkaller.appspot.com/text?tag=CrashReport&x=15448952580000
https://syzkaller.appspot.com/bug?extid=80e046b8da2820b6ba73

Re: [PATCH net v5 0/2] net,bpf: fix null-ptr-deref in xdp_master_redirect() for bonding and add selftest

[Eric Dumazet]

On Mon, Mar 9, 2026 at 10:41 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> March 9, 2026 at 15:46, "Eric Dumazet" <edumazet@google.com mailto:edumazet@google.com?to=%22Eric%20Dumazet%22%20%3Cedumazet%40google.com%3E > wrote:
>
>
> >
> > On Mon, Mar 9, 2026 at 4:07 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> >
> > >
> > > syzkaller reported a kernel panic [1] with the following crash stack:
> > >
> > >  Call Trace:
> > >  BUG: unable to handle page fault for address: ffff8ebd08580000
> > >  PF: supervisor write access in kernel mode
> > >  PF: error_code(0x0002) - not-present page
> > >  PGD 11f201067 P4D 11f201067 PUD 0
> > >  Oops: Oops: 0002 [#1] SMP PTI
> > >  CPU: 2 UID: 0 PID: 451 Comm: test_progs Not tainted 6.19.0+ #161 PREEMPT_RT
> > >  RIP: 0010:bond_rr_gen_slave_id+0x90/0xd0
> > >  RSP: 0018:ffffd3f4815f3448 EFLAGS: 00010246
> > >  RAX: 0000000000000001 RBX: 0000000000000001 RCX: ffff8ebc8728b17e
> > >  RDX: 0000000000000000 RSI: ffffd3f4815f3538 RDI: ffff8ebc8abcce40
> > >  RBP: ffffd3f4815f3460 R08: 0000000000000000 R09: 0000000000000000
> > >  R10: 0000000000000000 R11: 0000000000000000 R12: ffffd3f4815f3538
> > >  R13: ffff8ebc8abcce40 R14: ffff8ebc8728b17f R15: ffff8ebc8728b170
> > >  CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > >  CR2: ffff8ebd08580000 CR3: 000000010a808006 CR4: 0000000000770ef0
> > >  PKRU: 55555554
> > >  Call Trace:
> > >  <TASK>
> > >  bond_xdp_get_xmit_slave+0xc0/0x240
> > >  xdp_master_redirect+0x74/0xc0
> > >  bpf_prog_run_generic_xdp+0x2f2/0x3f0
> > >  do_xdp_generic+0x1fd/0x3d0
> > >  __netif_receive_skb_core.constprop.0+0x30d/0x1220
> > >  __netif_receive_skb_list_core+0xfc/0x250
> > >  netif_receive_skb_list_internal+0x20c/0x3d0
> > >  ? eth_type_trans+0x137/0x160
> > >  netif_receive_skb_list+0x25/0x140
> > >  xdp_test_run_batch.constprop.0+0x65b/0x6e0
> > >  bpf_test_run_xdp_live+0x1ec/0x3b0
> > >  bpf_prog_test_run_xdp+0x49d/0x6e0
> > >  __sys_bpf+0x446/0x27b0
> > >  __x64_sys_bpf+0x1a/0x30
> > >  x64_sys_call+0x146c/0x26e0
> > >  do_syscall_64+0xd3/0x1510
> > >  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > >
> > Please, can you always provide symbols in such traces ?
> > You can use scripts/decode_stacktrace.sh to make the trace really
> > nice, instead of ugly.
> >
>
>
> Hi Eric,
>
> Thank you for the suggestion. I didn't include the fully decoded stack
> trace in the cover letter because the syzkaller report already contains
> the complete information. You can find it here if needed:
>
> https://syzkaller.appspot.com/text?tag=CrashReport&x=15448952580000
> https://syzkaller.appspot.com/bug?extid=80e046b8da2820b6ba73

Exactly.

Either copy the syzbot stack traces when they have the symbols,
or do not copy them if they don't have them, a link to them is just good enough.

Re: [PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Nikolay Aleksandrov]

On Mon, Mar 09, 2026 at 11:06:58AM +0800, Jiayuan Chen wrote:
> From: Jiayuan Chen <jiayuan.chen@shopee.com>
> 
> bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
> check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
> when the bond mode is round-robin. If the bond device was never brought
> up, rr_tx_counter remains NULL, causing a null-ptr-deref.
> 
> The XDP redirect path can reach this code even when the bond is not up:
> bpf_master_redirect_enabled_key is a global static key, so when any bond
> device has native XDP attached, the XDP_TX -> xdp_master_redirect()
> interception is enabled for all bond slaves system-wide. This allows the
> path xdp_master_redirect() -> bond_xdp_get_xmit_slave() ->
> bond_xdp_xmit_roundrobin_slave_get() -> bond_rr_gen_slave_id() to be
> reached on a bond that was never opened.
> 
> Fix this by adding a NULL check with unlikely() in bond_rr_gen_slave_id()
> before dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was
> never opened), fall back to get_random_u32() for slave selection. The
> allocation in bond_open() is kept, with WRITE_ONCE() added to safely
> publish the pointer to the XDP read side. A plain read suffices for the
> !bond->rr_tx_counter guard in bond_open() itself, as bond_open() runs
> under RTNL lock and is the only writer of rr_tx_counter.
> 
> Fixes: 879af96ffd72 ("net, core: Add support for XDP redirection to slave device")
> Reported-by: syzbot+80e046b8da2820b6ba73@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/698f84c6.a70a0220.2c38d7.00cc.GAE@google.com/T/
> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> ---
>  drivers/net/bonding/bond_main.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 

This is Jay's patch + the unlikely change, looks good to me.
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

Cheers,
 Nik

> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 444519078da3..b8ec87625ce3 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4290,9 +4290,11 @@ static int bond_open(struct net_device *bond_dev)
>  	struct slave *slave;
>  
>  	if (BOND_MODE(bond) == BOND_MODE_ROUNDROBIN && !bond->rr_tx_counter) {
> -		bond->rr_tx_counter = alloc_percpu(u32);
> -		if (!bond->rr_tx_counter)
> +		u32 __percpu *rr_tx_tmp = alloc_percpu(u32);
> +
> +		if (!rr_tx_tmp)
>  			return -ENOMEM;
> +		WRITE_ONCE(bond->rr_tx_counter, rr_tx_tmp);
>  	}
>  
>  	/* reset slave->backup and slave->inactive */
> @@ -4883,6 +4885,9 @@ static u32 bond_rr_gen_slave_id(struct bonding *bond)
>  	struct reciprocal_value reciprocal_packets_per_slave;
>  	int packets_per_slave = bond->params.packets_per_slave;
>  
> +	if (unlikely(!READ_ONCE(bond->rr_tx_counter)))
> +		return get_random_u32();
> +
>  	switch (packets_per_slave) {
>  	case 0:
>  		slave_id = get_random_u32();
> -- 
> 2.43.0
> 

Re: [PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Eric Dumazet]

On Tue, Mar 10, 2026 at 12:49 PM Nikolay Aleksandrov
<razor@blackwall.org> wrote:
>
> On Mon, Mar 09, 2026 at 11:06:58AM +0800, Jiayuan Chen wrote:
> > From: Jiayuan Chen <jiayuan.chen@shopee.com>
> >
> > bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
> > check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
> > when the bond mode is round-robin. If the bond device was never brought
> > up, rr_tx_counter remains NULL, causing a null-ptr-deref.
> >
> > The XDP redirect path can reach this code even when the bond is not up:
> > bpf_master_redirect_enabled_key is a global static key, so when any bond
> > device has native XDP attached, the XDP_TX -> xdp_master_redirect()
> > interception is enabled for all bond slaves system-wide. This allows the
> > path xdp_master_redirect() -> bond_xdp_get_xmit_slave() ->
> > bond_xdp_xmit_roundrobin_slave_get() -> bond_rr_gen_slave_id() to be
> > reached on a bond that was never opened.
> >
> > Fix this by adding a NULL check with unlikely() in bond_rr_gen_slave_id()
> > before dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was
> > never opened), fall back to get_random_u32() for slave selection. The
> > allocation in bond_open() is kept, with WRITE_ONCE() added to safely
> > publish the pointer to the XDP read side. A plain read suffices for the
> > !bond->rr_tx_counter guard in bond_open() itself, as bond_open() runs
> > under RTNL lock and is the only writer of rr_tx_counter.
> >
> > Fixes: 879af96ffd72 ("net, core: Add support for XDP redirection to slave device")
> > Reported-by: syzbot+80e046b8da2820b6ba73@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/all/698f84c6.a70a0220.2c38d7.00cc.GAE@google.com/T/
> > Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> > ---
> >  drivers/net/bonding/bond_main.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
>
> This is Jay's patch + the unlikely change, looks good to me.
> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

Orthogonal to this patch  :

 get_random_u32() typical cost is around 10 to 20 ns, I really wonder
if this makes sense
for the packets_per_slave == 0 or 1 case to haves this kind of
randomness in the first place.

Perhaps we could use a

static DEFINE_PER_CPU(u32, rr_tx_counter)

And :
 slave_id = this_cpu_inc_return(rr_tx_counter);

Re: [PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Eric Dumazet]

On Tue, Mar 10, 2026 at 1:00 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Mar 10, 2026 at 12:49 PM Nikolay Aleksandrov
> <razor@blackwall.org> wrote:
> >
> > On Mon, Mar 09, 2026 at 11:06:58AM +0800, Jiayuan Chen wrote:
> > > From: Jiayuan Chen <jiayuan.chen@shopee.com>
> > >
> > > bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
> > > check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
> > > when the bond mode is round-robin. If the bond device was never brought
> > > up, rr_tx_counter remains NULL, causing a null-ptr-deref.
> > >
> > > The XDP redirect path can reach this code even when the bond is not up:
> > > bpf_master_redirect_enabled_key is a global static key, so when any bond
> > > device has native XDP attached, the XDP_TX -> xdp_master_redirect()
> > > interception is enabled for all bond slaves system-wide. This allows the
> > > path xdp_master_redirect() -> bond_xdp_get_xmit_slave() ->
> > > bond_xdp_xmit_roundrobin_slave_get() -> bond_rr_gen_slave_id() to be
> > > reached on a bond that was never opened.
> > >
> > > Fix this by adding a NULL check with unlikely() in bond_rr_gen_slave_id()
> > > before dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was
> > > never opened), fall back to get_random_u32() for slave selection. The
> > > allocation in bond_open() is kept, with WRITE_ONCE() added to safely
> > > publish the pointer to the XDP read side. A plain read suffices for the
> > > !bond->rr_tx_counter guard in bond_open() itself, as bond_open() runs
> > > under RTNL lock and is the only writer of rr_tx_counter.
> > >
> > > Fixes: 879af96ffd72 ("net, core: Add support for XDP redirection to slave device")
> > > Reported-by: syzbot+80e046b8da2820b6ba73@syzkaller.appspotmail.com
> > > Closes: https://lore.kernel.org/all/698f84c6.a70a0220.2c38d7.00cc.GAE@google.com/T/
> > > Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> > > ---
> > >  drivers/net/bonding/bond_main.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> >
> > This is Jay's patch + the unlikely change, looks good to me.
> > Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
>
> Orthogonal to this patch  :
>
>  get_random_u32() typical cost is around 10 to 20 ns, I really wonder
> if this makes sense
> for the packets_per_slave == 0 or 1 case to haves this kind of
> randomness in the first place.
>
> Perhaps we could use a
>
> static DEFINE_PER_CPU(u32, rr_tx_counter)
>
> And :
>  slave_id = this_cpu_inc_return(rr_tx_counter);

I also have mixed feelings about this patch.

We probably should detect that the device is not ready before hitting
something deeper in the stack.

Sure, a NULL deref is avoided, bu what happens next ?

We send a packet while the device is not UP, I am pretty sure this
violates at least some RCU rules in device dismantling.

Re: [PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Nikolay Aleksandrov]

On Tue, Mar 10, 2026 at 01:07:15PM +0100, Eric Dumazet wrote:
> On Tue, Mar 10, 2026 at 1:00 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Tue, Mar 10, 2026 at 12:49 PM Nikolay Aleksandrov
> > <razor@blackwall.org> wrote:
> > >
> > > On Mon, Mar 09, 2026 at 11:06:58AM +0800, Jiayuan Chen wrote:
> > > > From: Jiayuan Chen <jiayuan.chen@shopee.com>
> > > >
> > > > bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
> > > > check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
> > > > when the bond mode is round-robin. If the bond device was never brought
> > > > up, rr_tx_counter remains NULL, causing a null-ptr-deref.
> > > >
> > > > The XDP redirect path can reach this code even when the bond is not up:
> > > > bpf_master_redirect_enabled_key is a global static key, so when any bond
> > > > device has native XDP attached, the XDP_TX -> xdp_master_redirect()
> > > > interception is enabled for all bond slaves system-wide. This allows the
> > > > path xdp_master_redirect() -> bond_xdp_get_xmit_slave() ->
> > > > bond_xdp_xmit_roundrobin_slave_get() -> bond_rr_gen_slave_id() to be
> > > > reached on a bond that was never opened.
> > > >
> > > > Fix this by adding a NULL check with unlikely() in bond_rr_gen_slave_id()
> > > > before dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was
> > > > never opened), fall back to get_random_u32() for slave selection. The
> > > > allocation in bond_open() is kept, with WRITE_ONCE() added to safely
> > > > publish the pointer to the XDP read side. A plain read suffices for the
> > > > !bond->rr_tx_counter guard in bond_open() itself, as bond_open() runs
> > > > under RTNL lock and is the only writer of rr_tx_counter.
> > > >
> > > > Fixes: 879af96ffd72 ("net, core: Add support for XDP redirection to slave device")
> > > > Reported-by: syzbot+80e046b8da2820b6ba73@syzkaller.appspotmail.com
> > > > Closes: https://lore.kernel.org/all/698f84c6.a70a0220.2c38d7.00cc.GAE@google.com/T/
> > > > Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> > > > ---
> > > >  drivers/net/bonding/bond_main.c | 9 +++++++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > >
> > > This is Jay's patch + the unlikely change, looks good to me.
> > > Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
> >
> > Orthogonal to this patch  :
> >
> >  get_random_u32() typical cost is around 10 to 20 ns, I really wonder
> > if this makes sense
> > for the packets_per_slave == 0 or 1 case to haves this kind of
> > randomness in the first place.
> >
> > Perhaps we could use a
> >
> > static DEFINE_PER_CPU(u32, rr_tx_counter)
> >
> > And :
> >  slave_id = this_cpu_inc_return(rr_tx_counter);
> 
> I also have mixed feelings about this patch.
> 
> We probably should detect that the device is not ready before hitting
> something deeper in the stack.
> 
> Sure, a NULL deref is avoided, bu what happens next ?
> 
> We send a packet while the device is not UP, I am pretty sure this
> violates at least some RCU rules in device dismantling.

IIRC when the redirect continues, the packet should get dropped if the device is
not up (checks at a few places), but that's outside of bond's jurisdiction and
after the slave id is needed in xdp master redirect's path unfortunately.
I'm not sure it can reach much further, it just has the master dev's slave id
generation in its path.

In any case we shouldn't crash in the slave id generation in the bonding,
that ndo's only job is to return a slave id.

Re: [PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Paolo Abeni]

On 3/10/26 1:39 PM, Nikolay Aleksandrov wrote:
> On Tue, Mar 10, 2026 at 01:07:15PM +0100, Eric Dumazet wrote:
>> On Tue, Mar 10, 2026 at 1:00 PM Eric Dumazet <edumazet@google.com> wrote:
>>>
>>> On Tue, Mar 10, 2026 at 12:49 PM Nikolay Aleksandrov
>>> <razor@blackwall.org> wrote:
>>>>
>>>> On Mon, Mar 09, 2026 at 11:06:58AM +0800, Jiayuan Chen wrote:
>>>>> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>>>>>
>>>>> bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
>>>>> check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
>>>>> when the bond mode is round-robin. If the bond device was never brought
>>>>> up, rr_tx_counter remains NULL, causing a null-ptr-deref.
>>>>>
>>>>> The XDP redirect path can reach this code even when the bond is not up:
>>>>> bpf_master_redirect_enabled_key is a global static key, so when any bond
>>>>> device has native XDP attached, the XDP_TX -> xdp_master_redirect()
>>>>> interception is enabled for all bond slaves system-wide. This allows the
>>>>> path xdp_master_redirect() -> bond_xdp_get_xmit_slave() ->
>>>>> bond_xdp_xmit_roundrobin_slave_get() -> bond_rr_gen_slave_id() to be
>>>>> reached on a bond that was never opened.
>>>>>
>>>>> Fix this by adding a NULL check with unlikely() in bond_rr_gen_slave_id()
>>>>> before dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was
>>>>> never opened), fall back to get_random_u32() for slave selection. The
>>>>> allocation in bond_open() is kept, with WRITE_ONCE() added to safely
>>>>> publish the pointer to the XDP read side. A plain read suffices for the
>>>>> !bond->rr_tx_counter guard in bond_open() itself, as bond_open() runs
>>>>> under RTNL lock and is the only writer of rr_tx_counter.
>>>>>
>>>>> Fixes: 879af96ffd72 ("net, core: Add support for XDP redirection to slave device")
>>>>> Reported-by: syzbot+80e046b8da2820b6ba73@syzkaller.appspotmail.com
>>>>> Closes: https://lore.kernel.org/all/698f84c6.a70a0220.2c38d7.00cc.GAE@google.com/T/
>>>>> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
>>>>> ---
>>>>>  drivers/net/bonding/bond_main.c | 9 +++++++--
>>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>
>>>>
>>>> This is Jay's patch + the unlikely change, looks good to me.
>>>> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
>>>
>>> Orthogonal to this patch  :
>>>
>>>  get_random_u32() typical cost is around 10 to 20 ns, I really wonder
>>> if this makes sense
>>> for the packets_per_slave == 0 or 1 case to haves this kind of
>>> randomness in the first place.
>>>
>>> Perhaps we could use a
>>>
>>> static DEFINE_PER_CPU(u32, rr_tx_counter)
>>>
>>> And :
>>>  slave_id = this_cpu_inc_return(rr_tx_counter);
>>
>> I also have mixed feelings about this patch.
>>
>> We probably should detect that the device is not ready before hitting
>> something deeper in the stack.
>>
>> Sure, a NULL deref is avoided, bu what happens next ?
>>
>> We send a packet while the device is not UP, I am pretty sure this
>> violates at least some RCU rules in device dismantling.
> 
> IIRC when the redirect continues, the packet should get dropped if the device is
> not up (checks at a few places), but that's outside of bond's jurisdiction and
> after the slave id is needed in xdp master redirect's path unfortunately.
> I'm not sure it can reach much further, it just has the master dev's slave id
> generation in its path.
> 
> In any case we shouldn't crash in the slave id generation in the bonding,
> that ndo's only job is to return a slave id.

I'm sorry for the back and forth, but I share Eric's concern. I think
the approach suggested by Daniel:

https://lore.kernel.org/netdev/4d15be93-b497-4499-996d-9f3a67a2abc6@iogearbox.net/

or the initial patch form:

https://lore.kernel.org/netdev/20260224112545.37888-1-jiayuan.chen@linux.dev/T/#m7c67bb12f85bc88d583788fb6e41113c46208ae7

would be better. To respond to old concerns raised there: the check is
IMHO bond-specific, as control moves from the lower interface to the
upper bonding device, and the code is under an RCU critical section, the
device can't go away before the xmit is completed.

/P

Re: [PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Jiayuan Chen]

On 3/12/26 6:36 PM, Paolo Abeni wrote:
> On 3/10/26 1:39 PM, Nikolay Aleksandrov wrote:
>> On Tue, Mar 10, 2026 at 01:07:15PM +0100, Eric Dumazet wrote:
>>> On Tue, Mar 10, 2026 at 1:00 PM Eric Dumazet <edumazet@google.com> wrote:
>>>> On Tue, Mar 10, 2026 at 12:49 PM Nikolay Aleksandrov
>>>> <razor@blackwall.org> wrote:
>>>>> On Mon, Mar 09, 2026 at 11:06:58AM +0800, Jiayuan Chen wrote:
>>>>>> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>>>>>>
>>>>>> bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
>>>>>> check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
>>>>>> when the bond mode is round-robin. If the bond device was never brought
>>>>>> up, rr_tx_counter remains NULL, causing a null-ptr-deref.
>>>>>>
>>>>>> The XDP redirect path can reach this code even when the bond is not up:
>>>>>> bpf_master_redirect_enabled_key is a global static key, so when any bond
>>>>>> device has native XDP attached, the XDP_TX -> xdp_master_redirect()
>>>>>> interception is enabled for all bond slaves system-wide. This allows the
>>>>>> path xdp_master_redirect() -> bond_xdp_get_xmit_slave() ->
>>>>>> bond_xdp_xmit_roundrobin_slave_get() -> bond_rr_gen_slave_id() to be
>>>>>> reached on a bond that was never opened.
>>>>>>
>>>>>> Fix this by adding a NULL check with unlikely() in bond_rr_gen_slave_id()
>>>>>> before dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was
>>>>>> never opened), fall back to get_random_u32() for slave selection. The
>>>>>> allocation in bond_open() is kept, with WRITE_ONCE() added to safely
>>>>>> publish the pointer to the XDP read side. A plain read suffices for the
>>>>>> !bond->rr_tx_counter guard in bond_open() itself, as bond_open() runs
>>>>>> under RTNL lock and is the only writer of rr_tx_counter.
>>>>>>
>>>>>> Fixes: 879af96ffd72 ("net, core: Add support for XDP redirection to slave device")
>>>>>> Reported-by: syzbot+80e046b8da2820b6ba73@syzkaller.appspotmail.com
>>>>>> Closes: https://lore.kernel.org/all/698f84c6.a70a0220.2c38d7.00cc.GAE@google.com/T/
>>>>>> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
>>>>>> ---
>>>>>>   drivers/net/bonding/bond_main.c | 9 +++++++--
>>>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>
>>>>> This is Jay's patch + the unlikely change, looks good to me.
>>>>> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
>>>> Orthogonal to this patch  :
>>>>
>>>>   get_random_u32() typical cost is around 10 to 20 ns, I really wonder
>>>> if this makes sense
>>>> for the packets_per_slave == 0 or 1 case to haves this kind of
>>>> randomness in the first place.
>>>>
>>>> Perhaps we could use a
>>>>
>>>> static DEFINE_PER_CPU(u32, rr_tx_counter)
>>>>
>>>> And :
>>>>   slave_id = this_cpu_inc_return(rr_tx_counter);
>>> I also have mixed feelings about this patch.
>>>
>>> We probably should detect that the device is not ready before hitting
>>> something deeper in the stack.
>>>
>>> Sure, a NULL deref is avoided, bu what happens next ?
>>>
>>> We send a packet while the device is not UP, I am pretty sure this
>>> violates at least some RCU rules in device dismantling.
>> IIRC when the redirect continues, the packet should get dropped if the device is
>> not up (checks at a few places), but that's outside of bond's jurisdiction and
>> after the slave id is needed in xdp master redirect's path unfortunately.
>> I'm not sure it can reach much further, it just has the master dev's slave id
>> generation in its path.
>>
>> In any case we shouldn't crash in the slave id generation in the bonding,
>> that ndo's only job is to return a slave id.
> I'm sorry for the back and forth, but I share Eric's concern. I think
> the approach suggested by Daniel:
>
> https://lore.kernel.org/netdev/4d15be93-b497-4499-996d-9f3a67a2abc6@iogearbox.net/
>
> or the initial patch form:
>
> https://lore.kernel.org/netdev/20260224112545.37888-1-jiayuan.chen@linux.dev/T/#m7c67bb12f85bc88d583788fb6e41113c46208ae7
>
> would be better. To respond to old concerns raised there: the check is
> IMHO bond-specific, as control moves from the lower interface to the
> upper bonding device, and the code is under an RCU critical section, the
> device can't go away before the xmit is completed.
>
> /P


Looking at this issue holistically:


1. The XDP layer fix addresses the root cause of the current issue

2. Adding a defensive null check in bond_rr_gen_slave_id() protects
    against buggy callers - whether from XDP or future code paths. This
    aligns with the defense-in-depth principle that Nikolay and Sebastian
    highlighted.

Could we include both in v1? This way, the bond layer is robust regardless

of who calls it, preventing similar crashes from other potential code paths.


Thanks

Re: [PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Nikolay Aleksandrov]

On Thu, Mar 12, 2026 at 11:36:20AM +0100, Paolo Abeni wrote:
> On 3/10/26 1:39 PM, Nikolay Aleksandrov wrote:
> > On Tue, Mar 10, 2026 at 01:07:15PM +0100, Eric Dumazet wrote:
> >> On Tue, Mar 10, 2026 at 1:00 PM Eric Dumazet <edumazet@google.com> wrote:
> >>>
> >>> On Tue, Mar 10, 2026 at 12:49 PM Nikolay Aleksandrov
> >>> <razor@blackwall.org> wrote:
> >>>>
> >>>> On Mon, Mar 09, 2026 at 11:06:58AM +0800, Jiayuan Chen wrote:
> >>>>> From: Jiayuan Chen <jiayuan.chen@shopee.com>
> >>>>>
> >>>>> bond_rr_gen_slave_id() dereferences bond->rr_tx_counter without a NULL
> >>>>> check. rr_tx_counter is a per-CPU counter only allocated in bond_open()
> >>>>> when the bond mode is round-robin. If the bond device was never brought
> >>>>> up, rr_tx_counter remains NULL, causing a null-ptr-deref.
> >>>>>
> >>>>> The XDP redirect path can reach this code even when the bond is not up:
> >>>>> bpf_master_redirect_enabled_key is a global static key, so when any bond
> >>>>> device has native XDP attached, the XDP_TX -> xdp_master_redirect()
> >>>>> interception is enabled for all bond slaves system-wide. This allows the
> >>>>> path xdp_master_redirect() -> bond_xdp_get_xmit_slave() ->
> >>>>> bond_xdp_xmit_roundrobin_slave_get() -> bond_rr_gen_slave_id() to be
> >>>>> reached on a bond that was never opened.
> >>>>>
> >>>>> Fix this by adding a NULL check with unlikely() in bond_rr_gen_slave_id()
> >>>>> before dereferencing rr_tx_counter. When rr_tx_counter is NULL (bond was
> >>>>> never opened), fall back to get_random_u32() for slave selection. The
> >>>>> allocation in bond_open() is kept, with WRITE_ONCE() added to safely
> >>>>> publish the pointer to the XDP read side. A plain read suffices for the
> >>>>> !bond->rr_tx_counter guard in bond_open() itself, as bond_open() runs
> >>>>> under RTNL lock and is the only writer of rr_tx_counter.
> >>>>>
> >>>>> Fixes: 879af96ffd72 ("net, core: Add support for XDP redirection to slave device")
> >>>>> Reported-by: syzbot+80e046b8da2820b6ba73@syzkaller.appspotmail.com
> >>>>> Closes: https://lore.kernel.org/all/698f84c6.a70a0220.2c38d7.00cc.GAE@google.com/T/
> >>>>> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> >>>>> ---
> >>>>>  drivers/net/bonding/bond_main.c | 9 +++++++--
> >>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>>>>
> >>>>
> >>>> This is Jay's patch + the unlikely change, looks good to me.
> >>>> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
> >>>
> >>> Orthogonal to this patch  :
> >>>
> >>>  get_random_u32() typical cost is around 10 to 20 ns, I really wonder
> >>> if this makes sense
> >>> for the packets_per_slave == 0 or 1 case to haves this kind of
> >>> randomness in the first place.
> >>>
> >>> Perhaps we could use a
> >>>
> >>> static DEFINE_PER_CPU(u32, rr_tx_counter)
> >>>
> >>> And :
> >>>  slave_id = this_cpu_inc_return(rr_tx_counter);
> >>
> >> I also have mixed feelings about this patch.
> >>
> >> We probably should detect that the device is not ready before hitting
> >> something deeper in the stack.
> >>
> >> Sure, a NULL deref is avoided, bu what happens next ?
> >>
> >> We send a packet while the device is not UP, I am pretty sure this
> >> violates at least some RCU rules in device dismantling.
> > 
> > IIRC when the redirect continues, the packet should get dropped if the device is
> > not up (checks at a few places), but that's outside of bond's jurisdiction and
> > after the slave id is needed in xdp master redirect's path unfortunately.
> > I'm not sure it can reach much further, it just has the master dev's slave id
> > generation in its path.
> > 
> > In any case we shouldn't crash in the slave id generation in the bonding,
> > that ndo's only job is to return a slave id.
> 
> I'm sorry for the back and forth, but I share Eric's concern. I think
> the approach suggested by Daniel:
> 
> https://lore.kernel.org/netdev/4d15be93-b497-4499-996d-9f3a67a2abc6@iogearbox.net/
> 

That will work, I like Daniel's patch as well. It will add a test for all redirects
for master devices, but I guess that is ok. For bonding it will work because the bond
has the problem only while it was never opened (before first up).

IMO this patch still has value, because currently the code implicitly relies on
a specific sequence of events, who knows tomorrow someone may find another way
and again call that ndo while the bond is down.

> or the initial patch form:
> 
> https://lore.kernel.org/netdev/20260224112545.37888-1-jiayuan.chen@linux.dev/T/#m7c67bb12f85bc88d583788fb6e41113c46208ae7
> 
> would be better. To respond to old concerns raised there: the check is
> IMHO bond-specific, as control moves from the lower interface to the
> upper bonding device, and the code is under an RCU critical section, the
> device can't go away before the xmit is completed.

This one I don't like, it is not about going away, it is more about adding 2 new
tests for everyone and potentially 1 more cache line.

> 
> /P
> 

Cheers,
 Nik

Re: [PATCH net v5 1/2] bonding: fix null-ptr-deref in bond_rr_gen_slave_id()

[Jiayuan Chen]

On 3/12/26 7:02 PM, Jiayuan Chen wrote:
> f who calls it, preventing similar crashes from other potential code 
> paths.




Sorry for the noise.

Could we include both:
1. Add link status check in xdp
2. Adding a defensive null check in bond_rr_gen_slave_id()


This way, the bond layer is robust regardless of who calls it,
preventing similar crashes from other potential code paths.