Message-ID: <dc118ae9-4ec0-4180-b7aa-90eba5283010@nvidia.com>
Date: Mon, 20 Jan 2025 20:55:07 +0200
From: Yael Chemla <ychemla@...dia.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: davem@...emloft.net, edumazet@...gle.com, horms@...nel.org,
 kuba@...nel.org, kuni1840@...il.com, netdev@...r.kernel.org,
 pabeni@...hat.com
Subject: Re: [PATCH v1 net-next 4/4] net: Hold rtnl_net_lock() in
 (un)?register_netdevice_notifier_dev_net().

On 16/01/2025 4:54, Kuniyuki Iwashima wrote:
> Thanks for the report!
> 
> From: Yael Chemla <ychemla@...dia.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 ?
> 
BUG: KASAN: slab-use-after-free in notifier_call_chain 
(/usr/work/linux/kernel/notifier.c:75 (discriminator 2))
Read of size 8 at addr ffff88810cefb4c8 by task test-bridge-lag/21127

Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
Call Trace:
  <TASK>
dump_stack_lvl (/usr/work/linux/lib/dump_stack.c:123)
print_report (/usr/work/linux/mm/kasan/report.c:379 
/usr/work/linux/mm/kasan/report.c:489)
? __virt_addr_valid (/usr/work/linux/./arch/x86/include/asm/preempt.h:84 
(discriminator 13) /usr/work/linux/./include/linux/rcupdate.h:964 
(discriminator 13) /usr/work/linux/./include/linux/mmzone.h:2058 
(discriminator 13) /usr/work/linux/arch/x86/mm/physaddr.c:65 
(discriminator 13))
kasan_report (/usr/work/linux/mm/kasan/report.c:604)
? notifier_call_chain (/usr/work/linux/kernel/notifier.c:75 
(discriminator 2))
? notifier_call_chain (/usr/work/linux/kernel/notifier.c:75 
(discriminator 2))
notifier_call_chain (/usr/work/linux/kernel/notifier.c:75 (discriminator 2))
call_netdevice_notifiers_info (/usr/work/linux/net/core/dev.c:2011)
unregister_netdevice_many_notify (/usr/work/linux/net/core/dev.c:11551)
? mark_held_locks (/usr/work/linux/kernel/locking/lockdep.c:4321 
(discriminator 1))
? __mutex_lock (/usr/work/linux/kernel/locking/mutex.c:689 
(discriminator 2) /usr/work/linux/kernel/locking/mutex.c:735 
(discriminator 2))
? lockdep_hardirqs_on_prepare 
(/usr/work/linux/kernel/locking/lockdep.c:4347 
/usr/work/linux/kernel/locking/lockdep.c:4406)
? dev_ingress_queue_create (/usr/work/linux/net/core/dev.c:11492)
? __mutex_lock (/usr/work/linux/kernel/locking/mutex.c:689 
(discriminator 2) /usr/work/linux/kernel/locking/mutex.c:735 
(discriminator 2))
? __mutex_lock (/usr/work/linux/./arch/x86/include/asm/preempt.h:84 
(discriminator 13) /usr/work/linux/kernel/locking/mutex.c:715 
(discriminator 13) /usr/work/linux/kernel/locking/mutex.c:735 
(discriminator 13))
? unregister_netdev (/usr/work/linux/./include/linux/netdevice.h:3236 
/usr/work/linux/net/core/dev.c:11633)
? mutex_lock_io_nested (/usr/work/linux/kernel/locking/mutex.c:734)
? __mutex_unlock_slowpath 
(/usr/work/linux/./arch/x86/include/asm/atomic64_64.h:101 
/usr/work/linux/./include/linux/atomic/atomic-arch-fallback.h:4329 
/usr/work/linux/./include/linux/atomic/atomic-long.h:1506 
/usr/work/linux/./include/linux/atomic/atomic-instrumented.h:4481 
/usr/work/linux/kernel/locking/mutex.c:913)
unregister_netdevice_queue (/usr/work/linux/net/core/dev.c:11487)
? unregister_netdevice_many (/usr/work/linux/net/core/dev.c:11476)
unregister_netdev (/usr/work/linux/net/core/dev.c:11635)
mlx5e_remove 
(/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/en_main.c:6552 
/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/en_main.c:6579) 
mlx5_core
auxiliary_bus_remove (/usr/work/linux/drivers/base/auxiliary.c:230)
device_release_driver_internal (/usr/work/linux/drivers/base/dd.c:1275 
/usr/work/linux/drivers/base/dd.c:1296)
? kobject_put (/usr/work/linux/./arch/x86/include/asm/atomic.h:93 
(discriminator 4) 
/usr/work/linux/./include/linux/atomic/atomic-arch-fallback.h:949 
(discriminator 4) 
/usr/work/linux/./include/linux/atomic/atomic-instrumented.h:401 
(discriminator 4) /usr/work/linux/./include/linux/refcount.h:264 
(discriminator 4) /usr/work/linux/./include/linux/refcount.h:307 
(discriminator 4) /usr/work/linux/./include/linux/refcount.h:325 
(discriminator 4) /usr/work/linux/./include/linux/kref.h:64 
(discriminator 4) /usr/work/linux/lib/kobject.c:737 (discriminator 4))
bus_remove_device (/usr/work/linux/./include/linux/kobject.h:193 
/usr/work/linux/drivers/base/base.h:73 
/usr/work/linux/drivers/base/bus.c:583)
device_del (/usr/work/linux/drivers/base/power/power.h:142 
/usr/work/linux/drivers/base/core.c:3855)
? mlx5_core_is_eth_enabled 
(/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/devlink.h:65) 
mlx5_core
? __device_link_del (/usr/work/linux/drivers/base/core.c:3809)
? is_ib_enabled 
(/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/devlink.h:58) 
mlx5_core
mlx5_rescan_drivers_locked 
(/usr/work/linux/./include/linux/auxiliary_bus.h:241 
/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/dev.c:333 
/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/dev.c:535 
/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/dev.c:549) mlx5_core
mlx5_unregister_device 
(/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/dev.c:468) 
mlx5_core
mlx5_uninit_one (/usr/work/linux/./include/linux/instrumented.h:68 
/usr/work/linux/./include/asm-generic/bitops/instrumented-non-atomic.h:141 
/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/main.c:1563) 
mlx5_core
remove_one 
(/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/main.c:965 
/usr/work/linux/drivers/net/ethernet/mellanox/mlx5/core/main.c:2019) 
mlx5_core
pci_device_remove (/usr/work/linux/./include/linux/pm_runtime.h:129 
/usr/work/linux/drivers/pci/pci-driver.c:475)
device_release_driver_internal (/usr/work/linux/drivers/base/dd.c:1275 
/usr/work/linux/drivers/base/dd.c:1296)
unbind_store (/usr/work/linux/drivers/base/bus.c:245)
kernfs_fop_write_iter (/usr/work/linux/fs/kernfs/file.c:338)
vfs_write (/usr/work/linux/fs/read_write.c:587 (discriminator 1) 
/usr/work/linux/fs/read_write.c:679 (discriminator 1))
? do_user_addr_fault (/usr/work/linux/./include/linux/rcupdate.h:337 
/usr/work/linux/./include/linux/rcupdate.h:849 
/usr/work/linux/./include/linux/mm.h:740 
/usr/work/linux/arch/x86/mm/fault.c:1340)
? kernel_write (/usr/work/linux/fs/read_write.c:660)
? lock_downgrade (/usr/work/linux/kernel/locking/lockdep.c:5857)
ksys_write (/usr/work/linux/fs/read_write.c:732)
? __x64_sys_read (/usr/work/linux/fs/read_write.c:721)
? do_user_addr_fault 
(/usr/work/linux/./arch/x86/include/asm/preempt.h:84 (discriminator 13) 
/usr/work/linux/./include/linux/rcupdate.h:98 (discriminator 13) 
/usr/work/linux/./include/linux/rcupdate.h:882 (discriminator 13) 
/usr/work/linux/./include/linux/mm.h:742 (discriminator 13) 
/usr/work/linux/arch/x86/mm/fault.c:1340 (discriminator 13))
do_syscall_64 (/usr/work/linux/arch/x86/entry/common.c:52 (discriminator 
1) /usr/work/linux/arch/x86/entry/common.c:83 (discriminator 1))
entry_SYSCALL_64_after_hwframe 
(/usr/work/linux/arch/x86/entry/entry_64.S:130)
RIP: 0033:0x7f6a4d5018b7

>>
>> 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 ?
> 

we set up a network configuration which includes two VFs isolated using 
two namespaces (there’s also bridge in this topology), we pass some 
traffic between VFs. At the end of test (cleanup) we delete network 
namespaces, wait for 0.5 sec and unbind VFs of NIC.

note that when I extended the timeout after deleting the namespaces the 
issue doesn’t reproduce.

> I guess the VF is moved to init_net ?
> 

this should be the behavior in case deletion of the namespaces happens 
before the unbind of VFs.

>>
>>
>> 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@...zon.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()
> 
> ?

When checking dev_net before and after taking the lock the issue won’t 
reproduce.
note that when issue reproduce we arrive to 
unregister_netdevice_notifier_dev_net with an invalid net pointer 
(verified it with prints of its value, and it's not the same consistent 
value as is throughout rest of the test).
we suspect the issue related to the async ns deletion.

Powered by blists - more mailing lists