[PATCH v1 net] netdevsim: Fix wild pointer access in nsim_queue_free().

[PATCH v1 net] netdevsim: Fix wild pointer access in nsim_queue_free().

[Kuniyuki Iwashima]

syzbot reported the splat below. [0]

When nsim_queue_uninit() is called from nsim_init_netdevsim(),
register_netdevice() has not been called, thus dev->dstats has
not been allocated.

Let's not call dev_dstats_rx_dropped_add() in such a case.

[0]
BUG: unable to handle page fault for address: ffff88809782c020
 PF: supervisor write access in kernel mode
 PF: error_code(0x0002) - not-present page
PGD 1b401067 P4D 1b401067 PUD 0
Oops: Oops: 0002 [#1] SMP KASAN NOPTI
CPU: 3 UID: 0 PID: 8476 Comm: syz.1.251 Not tainted 6.16.0-syzkaller-06699-ge8d780dcd957 #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:local_add arch/x86/include/asm/local.h:33 [inline]
RIP: 0010:u64_stats_add include/linux/u64_stats_sync.h:89 [inline]
RIP: 0010:dev_dstats_rx_dropped_add include/linux/netdevice.h:3027 [inline]
RIP: 0010:nsim_queue_free+0xba/0x120 drivers/net/netdevsim/netdev.c:714
Code: 07 77 6c 4a 8d 3c ed 20 7e f1 8d 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 75 46 4a 03 1c ed 20 7e f1 8d <4c> 01 63 20 be 00 02 00 00 48 8d 3d 00 00 00 00 e8 61 2f 58 fa 48
RSP: 0018:ffffc900044af150 EFLAGS: 00010286
RAX: dffffc0000000000 RBX: ffff88809782c000 RCX: 00000000000079c3
RDX: 1ffffffff1be2fc7 RSI: ffffffff8c15f380 RDI: ffffffff8df17e38
RBP: ffff88805f59d000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000
R13: 0000000000000003 R14: ffff88806ceb3d00 R15: ffffed100dfd308e
FS:  0000000000000000(0000) GS:ffff88809782c000(0063) knlGS:00000000f505db40
CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: ffff88809782c020 CR3: 000000006fc6a000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 nsim_queue_uninit drivers/net/netdevsim/netdev.c:993 [inline]
 nsim_init_netdevsim drivers/net/netdevsim/netdev.c:1049 [inline]
 nsim_create+0xd0a/0x1260 drivers/net/netdevsim/netdev.c:1101
 __nsim_dev_port_add+0x435/0x7d0 drivers/net/netdevsim/dev.c:1438
 nsim_dev_port_add_all drivers/net/netdevsim/dev.c:1494 [inline]
 nsim_dev_reload_create drivers/net/netdevsim/dev.c:1546 [inline]
 nsim_dev_reload_up+0x5b8/0x860 drivers/net/netdevsim/dev.c:1003
 devlink_reload+0x322/0x7c0 net/devlink/dev.c:474
 devlink_nl_reload_doit+0xe31/0x1410 net/devlink/dev.c:584
 genl_family_rcv_msg_doit+0x206/0x2f0 net/netlink/genetlink.c:1115
 genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
 genl_rcv_msg+0x55c/0x800 net/netlink/genetlink.c:1210
 netlink_rcv_skb+0x155/0x420 net/netlink/af_netlink.c:2552
 genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
 netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
 netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1346
 netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
 sock_sendmsg_nosec net/socket.c:714 [inline]
 __sock_sendmsg net/socket.c:729 [inline]
 ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
 ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
 __sys_sendmsg+0x16d/0x220 net/socket.c:2700
 do_syscall_32_irqs_on arch/x86/entry/syscall_32.c:83 [inline]
 __do_fast_syscall_32+0x7c/0x3a0 arch/x86/entry/syscall_32.c:306
 do_fast_syscall_32+0x32/0x80 arch/x86/entry/syscall_32.c:331
 entry_SYSENTER_compat_after_hwframe+0x84/0x8e
RIP: 0023:0xf708e579
Code: b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 8d b4 26 00 00 00 00 8d b4 26 00 00 00 00
RSP: 002b:00000000f505d55c EFLAGS: 00000296 ORIG_RAX: 0000000000000172
RAX: ffffffffffffffda RBX: 0000000000000007 RCX: 0000000080000080
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000296 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
 </TASK>
Modules linked in:
CR2: ffff88809782c020

Fixes: 2a68a22304f9 ("netdevsim: account dropped packet length in stats on queue free")
Reported-by: syzbot+8aa80c6232008f7b957d@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/688bb9ca.a00a0220.26d0e1.0050.GAE@google.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/netdevsim/netdev.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 39fe28af48b9..5cbc005136d8 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -710,9 +710,13 @@ static struct nsim_rq *nsim_queue_alloc(void)
 static void nsim_queue_free(struct net_device *dev, struct nsim_rq *rq)
 {
 	hrtimer_cancel(&rq->napi_timer);
-	local_bh_disable();
-	dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);
-	local_bh_enable();
+
+	if (likely(dev->reg_state != NETREG_UNINITIALIZED)) {
+		local_bh_disable();
+		dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);
+		local_bh_enable();
+	}
+
 	skb_queue_purge_reason(&rq->skb_queue, SKB_DROP_REASON_QUEUE_PURGE);
 	kfree(rq);
 }
-- 
2.50.1.565.gc32cd1483b-goog

Re: [PATCH v1 net] netdevsim: Fix wild pointer access in nsim_queue_free().

[Eric Dumazet]

On Thu, Jul 31, 2025 at 11:48 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> syzbot reported the splat below. [0]
>
> When nsim_queue_uninit() is called from nsim_init_netdevsim(),
> register_netdevice() has not been called, thus dev->dstats has
> not been allocated.
>
> Let's not call dev_dstats_rx_dropped_add() in such a case.
>
>
> Fixes: 2a68a22304f9 ("netdevsim: account dropped packet length in stats on queue free")
> Reported-by: syzbot+8aa80c6232008f7b957d@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/688bb9ca.a00a0220.26d0e1.0050.GAE@google.com/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  drivers/net/netdevsim/netdev.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index 39fe28af48b9..5cbc005136d8 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -710,9 +710,13 @@ static struct nsim_rq *nsim_queue_alloc(void)
>  static void nsim_queue_free(struct net_device *dev, struct nsim_rq *rq)
>  {
>         hrtimer_cancel(&rq->napi_timer);
> -       local_bh_disable();
> -       dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);
> -       local_bh_enable();
> +
> +       if (likely(dev->reg_state != NETREG_UNINITIALIZED)) {

I find this test about reg_state a bit fragile...

I probably would have made dev_dstats_rx_dropped_add() a bit stronger,
it is not used in a fast path.

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5e5de4b0a433c6613224b53750921ab9f5a39c85..0b7ad5ae4b85d480aee8531e821027e6ebe7119b
100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3021,11 +3021,13 @@ static inline void
dev_dstats_rx_dropped(struct net_device *dev)
 static inline void dev_dstats_rx_dropped_add(struct net_device *dev,
                                             unsigned int packets)
 {
-       struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
+       if (dev->dstats) {
+               struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);

-       u64_stats_update_begin(&dstats->syncp);
-       u64_stats_add(&dstats->rx_drops, packets);
-       u64_stats_update_end(&dstats->syncp);
+               u64_stats_update_begin(&dstats->syncp);
+               u64_stats_add(&dstats->rx_drops, packets);
+               u64_stats_update_end(&dstats->syncp);
+       }
 }

 static inline void dev_dstats_tx_add(struct net_device *dev,

Re: [PATCH v1 net] netdevsim: Fix wild pointer access in nsim_queue_free().

[Kuniyuki Iwashima]

On Fri, Aug 1, 2025 at 12:35 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Jul 31, 2025 at 11:48 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > syzbot reported the splat below. [0]
> >
> > When nsim_queue_uninit() is called from nsim_init_netdevsim(),
> > register_netdevice() has not been called, thus dev->dstats has
> > not been allocated.
> >
> > Let's not call dev_dstats_rx_dropped_add() in such a case.
> >
> >
> > Fixes: 2a68a22304f9 ("netdevsim: account dropped packet length in stats on queue free")
> > Reported-by: syzbot+8aa80c6232008f7b957d@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/netdev/688bb9ca.a00a0220.26d0e1.0050.GAE@google.com/
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  drivers/net/netdevsim/netdev.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> > index 39fe28af48b9..5cbc005136d8 100644
> > --- a/drivers/net/netdevsim/netdev.c
> > +++ b/drivers/net/netdevsim/netdev.c
> > @@ -710,9 +710,13 @@ static struct nsim_rq *nsim_queue_alloc(void)
> >  static void nsim_queue_free(struct net_device *dev, struct nsim_rq *rq)
> >  {
> >         hrtimer_cancel(&rq->napi_timer);
> > -       local_bh_disable();
> > -       dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);
> > -       local_bh_enable();
> > +
> > +       if (likely(dev->reg_state != NETREG_UNINITIALIZED)) {
>
> I find this test about reg_state a bit fragile...
>
> I probably would have made dev_dstats_rx_dropped_add() a bit stronger,
> it is not used in a fast path.

I thought I should avoid local_bh_disable() too, but yes,
it's unlikely and in the slow path.

I'll use the blow diff in v2.

Thank you Eric!

>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 5e5de4b0a433c6613224b53750921ab9f5a39c85..0b7ad5ae4b85d480aee8531e821027e6ebe7119b
> 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3021,11 +3021,13 @@ static inline void
> dev_dstats_rx_dropped(struct net_device *dev)
>  static inline void dev_dstats_rx_dropped_add(struct net_device *dev,
>                                              unsigned int packets)
>  {
> -       struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
> +       if (dev->dstats) {
> +               struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
>
> -       u64_stats_update_begin(&dstats->syncp);
> -       u64_stats_add(&dstats->rx_drops, packets);
> -       u64_stats_update_end(&dstats->syncp);
> +               u64_stats_update_begin(&dstats->syncp);
> +               u64_stats_add(&dstats->rx_drops, packets);
> +               u64_stats_update_end(&dstats->syncp);
> +       }
>  }
>
>  static inline void dev_dstats_tx_add(struct net_device *dev,

Re: [PATCH v1 net] netdevsim: Fix wild pointer access in nsim_queue_free().

[Jakub Kicinski]

On Fri, 1 Aug 2025 09:29:49 -0700 Kuniyuki Iwashima wrote:
> > >         hrtimer_cancel(&rq->napi_timer);
> > > -       local_bh_disable();
> > > -       dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);
> > > -       local_bh_enable();
> > > +
> > > +       if (likely(dev->reg_state != NETREG_UNINITIALIZED)) {  
> >
> > I find this test about reg_state a bit fragile...
> >
> > I probably would have made dev_dstats_rx_dropped_add() a bit stronger,
> > it is not used in a fast path.  
> 
> I thought I should avoid local_bh_disable() too, but yes,
> it's unlikely and in the slow path.
> 
> I'll use the blow diff in v2.

Option 2 :

	if (rq->skb_queue.qlen)
		dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);

since there can't be any packets, yet. Up to you.

Re: [PATCH v1 net] netdevsim: Fix wild pointer access in nsim_queue_free().

[Vivek BalachandharTN]

Hi Kuniyu,

Thanks for working on this fix.

Could you share the steps or setup you used to reproduce the issue?  
I’d like to replicate it locally to better understand the problem.

Thanks,
Vivek BalachandharTN