Message-ID: <e2164d2e-10dc-4990-9c1b-70667a594fd1@nvidia.com>
Date: Mon, 27 Jan 2025 19:26:34 +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().

Hi Kuniyuki,

any update on this?
do you need further info?

Thanks,
Yael

On 20/01/2025 20:55, Yael Chemla wrote:
> 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