Re: [PATCH v1 net-next 4/4] net: Hold rtnl_net_lock() in (un)?register_netdevice_notifier_dev_net().

[Kuniyuki Iwashima <kuniyu@amazon.com>]

Thanks for the report!

From: Yael Chemla <ychemla@nvidia.com>
Date: Thu, 16 Jan 2025 00:16:27 +0200
> we observed in our regression tests the following issue:
> 
> BUG: KASAN: slab-use-after-free in notifier_call_chain+0x22c/0x280
> kasan_report+0xbd/0xf0
> RIP: 0033:0x7f70839018b7
> kasan_save_stack+0x1c/0x40
> kasan_save_track+0x10/0x30
> __kasan_kmalloc+0x83/0x90
> kasan_save_stack+0x1c/0x40
> kasan_save_track+0x10/0x30
> kasan_save_free_info+0x37/0x50
> __kasan_slab_free+0x33/0x40
> page dumped because: kasan: bad access detected
> BUG: KASAN: slab-use-after-free in notifier_call_chain+0x222/0x280
> kasan_report+0xbd/0xf0
> RIP: 0033:0x7f70839018b7
> kasan_save_stack+0x1c/0x40
> kasan_save_track+0x10/0x30
> __kasan_kmalloc+0x83/0x90
> kasan_save_stack+0x1c/0x40
> kasan_save_track+0x10/0x30
> kasan_save_free_info+0x37/0x50
> __kasan_slab_free+0x33/0x40
> page dumped because: kasan: bad access detected
> 
> and there are many more of that kind.

Do you have any other stack traces with more callers info ?
Also can you decode the trace with ./scripts/decode_stacktrace.sh ?

> 
> it happens after applying commit 7fb1073300a2 ("net: Hold 
> rtnl_net_lock() in (un)?register_netdevice_notifier_dev_net()")
> 
> test scenario includes configuration and traffic over two namespaces 
> associated with two different VFs.

Could you elaborate more about the test scenario, especially
how each device/netns is dismantled after the test case ?

I guess the VF is moved to init_net ?

> 
> 
> On 04/01/2025 8:37, Kuniyuki Iwashima wrote:
> > (un)?register_netdevice_notifier_dev_net() hold RTNL before triggering
> > the notifier for all netdev in the netns.
> > 
> > Let's convert the RTNL to rtnl_net_lock().
> > 
> > Note that move_netdevice_notifiers_dev_net() is assumed to be (but not
> > yet) protected by per-netns RTNL of both src and dst netns; we need to
> > convert wireless and hyperv drivers that call dev_change_net_namespace().
> > 
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> >   net/core/dev.c | 16 ++++++++++------
> >   1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index f6c6559e2548..a0dd34463901 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -1943,15 +1943,17 @@ int register_netdevice_notifier_dev_net(struct net_device *dev,
> >   					struct notifier_block *nb,
> >   					struct netdev_net_notifier *nn)
> >   {
> > +	struct net *net = dev_net(dev);
> 
> it seems to happen since the net pointer is acquired here without a lock.
> Note that KASAN issue is not triggered when executing with rtnl_lock() 
> taken before this line. and our kernel .config expands 
> rtnl_net_lock(net) to rtnl_lock() (CONFIG_DEBUG_NET_SMALL_RTNL is not set).

It sounds like the device was being moved to another netns while
unregister_netdevice_notifier_dev_net() was called.

Could you check if dev_net() is changed before/after rtnl_lock() in

  * register_netdevice_notifier_dev_net()
  * unregister_netdevice_notifier_dev_net()

?