| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMArcTV3xfOJ1-GoJriUyTAniPkAeSYYRTd85gxJc+nfiBce7w@mail.gmail.com> Date: Wed, 7 May 2025 13:24:55 +0900 From: Taehee Yoo <ap420073@...il.com> To: Jakub Kicinski <kuba@...nel.org> Cc: davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com, andrew+netdev@...n.ch, horms@...nel.org, almasrymina@...gle.com, sdf@...ichev.me, netdev@...r.kernel.org, asml.silence@...il.com, dw@...idwei.uk, skhawaja@...gle.com, willemb@...gle.com, jdamato@...tly.com Subject: Re: [PATCH net v2] net: devmem: fix kernel panic when socket close after module unload On Wed, May 7, 2025 at 7:07 AM Jakub Kicinski <kuba@...nel.org> wrote: > Hi Jakub, Thanks a lot for the review! > On Tue, 6 May 2025 14:08:58 +0000 Taehee Yoo wrote: > > Kernel panic occurs when a devmem TCP socket is closed after NIC module > > is unloaded. > > > > This is Devmem TCP unregistration scenarios. number is an order. > > (a)socket close (b)pp destroy (c)uninstall result > > 1 2 3 OK > > 1 3 2 (d)Impossible > > 2 1 3 OK > > 3 1 2 (e)Kernel panic > > 2 3 1 (d)Impossible > > 3 2 1 (d)Impossible > > > > (a) netdev_nl_sock_priv_destroy() is called when devmem TCP socket is > > closed. > > (b) page_pool_destroy() is called when the interface is down. > > (c) mp_ops->uninstall() is called when an interface is unregistered. > > (d) There is no scenario in mp_ops->uninstall() is called before > > page_pool_destroy(). > > Because unregister_netdevice_many_notify() closes interfaces first > > and then calls mp_ops->uninstall(). > > (e) netdev_nl_sock_priv_destroy() accesses struct net_device to acquire > > netdev_lock(). > > But if the interface module has already been removed, net_device > > pointer is invalid, so it causes kernel panic. > > > > In summary, there are only 3 possible scenarios. > > A. sk close -> pp destroy -> uninstall. > > B. pp destroy -> sk close -> uninstall. > > C. pp destroy -> uninstall -> sk close. > > > > Case C is a kernel panic scenario. > > > > In order to fix this problem, It makes mp_dmabuf_devmem_uninstall() set > > binding->dev to NULL. > > It indicates an bound net_device was unregistered. > > > > It makes netdev_nl_sock_priv_destroy() do not acquire netdev_lock() > > if binding->dev is NULL. > > > > It inverts socket/netdev lock order like below: > > netdev_lock(); > > mutex_lock(&priv->lock); > > mutex_unlock(&priv->lock); > > netdev_unlock(); > > > > Because of inversion of locking ordering, mp_dmabuf_devmem_uninstall() > > acquires socket lock from now on. > > > > Tests: > > Scenario A: > > ./ncdevmem -s 192.168.1.4 -c 192.168.1.2 -f $interface -l -p 8000 \ > > -v 7 -t 1 -q 1 & > > pid=$! > > sleep 10 > > kill $pid > > ip link set $interface down > > modprobe -rv $module > > > > Scenario B: > > ./ncdevmem -s 192.168.1.4 -c 192.168.1.2 -f $interface -l -p 8000 \ > > -v 7 -t 1 -q 1 & > > pid=$! > > sleep 10 > > ip link set $interface down > > kill $pid > > modprobe -rv $module > > > > Scenario C: > > ./ncdevmem -s 192.168.1.4 -c 192.168.1.2 -f $interface -l -p 8000 \ > > -v 7 -t 1 -q 1 & > > pid=$! > > sleep 10 > > modprobe -rv $module > > sleep 5 > > kill $pid > > > > Splat looks like: > > Oops: general protection fault, probably for non-canonical address 0xdffffc001fffa9f7: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN NOPTI > > KASAN: probably user-memory-access in range [0x00000000fffd4fb8-0x00000000fffd4fbf] > > CPU: 0 UID: 0 PID: 2041 Comm: ncdevmem Tainted: G B W 6.15.0-rc1+ #2 PREEMPT(undef) 0947ec89efa0fd68838b78e36aa1617e97ff5d7f > > Tainted: [B]=BAD_PAGE, [W]=WARN > > RIP: 0010:__mutex_lock (./include/linux/sched.h:2244 kernel/locking/mutex.c:400 kernel/locking/mutex.c:443 kernel/locking/mutex.c:605 kernel/locking/mutex.c:746) > > Code: ea 03 80 3c 02 00 0f 85 4f 13 00 00 49 8b 1e 48 83 e3 f8 74 6a 48 b8 00 00 00 00 00 fc ff df 48 8d 7b 34 48 89 fa 48 c1 ea 03 <0f> b6 f > > RSP: 0018:ffff88826f7ef730 EFLAGS: 00010203 > > RAX: dffffc0000000000 RBX: 00000000fffd4f88 RCX: ffffffffaa9bc811 > > RDX: 000000001fffa9f7 RSI: 0000000000000008 RDI: 00000000fffd4fbc > > RBP: ffff88826f7ef8b0 R08: 0000000000000000 R09: ffffed103e6aa1a4 > > R10: 0000000000000007 R11: ffff88826f7ef442 R12: fffffbfff669f65e > > R13: ffff88812a830040 R14: ffff8881f3550d20 R15: 00000000fffd4f88 > > FS: 0000000000000000(0000) GS:ffff888866c05000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 0000563bed0cb288 CR3: 00000001a7c98000 CR4: 00000000007506f0 > > PKRU: 55555554 > > Call Trace: > > <TASK> > > ... > > netdev_nl_sock_priv_destroy (net/core/netdev-genl.c:953 (discriminator 3)) > > genl_release (net/netlink/genetlink.c:653 net/netlink/genetlink.c:694 net/netlink/genetlink.c:705) > > ... > > netlink_release (net/netlink/af_netlink.c:737) > > ... > > __sock_release (net/socket.c:647) > > sock_close (net/socket.c:1393) > > I'll look at the code later today, but it will need a respin for sure: > > net/core/netdev-genl.c: In function ‘netdev_nl_bind_rx_doit’: > net/core/netdev-genl.c:878:61: error: passing argument 3 of ‘net_devmem_bind_dmabuf’ from incompatible pointer type [-Werror=incompatible-pointer-types] > 878 | binding = net_devmem_bind_dmabuf(netdev, dmabuf_fd, priv, info->extack); > | ^~~~ > | | > | struct netdev_nl_sock * > In file included from ../net/core/netdev-genl.c:16: > net/core/devmem.h:133:48: note: expected ‘struct netlink_ext_ack *’ but argument is of type ‘struct netdev_nl_sock *’ > 133 | struct netlink_ext_ack *extack) > | ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~ > net/core/netdev-genl.c:878:19: error: too many arguments to function ‘net_devmem_bind_dmabuf’ > 878 | binding = net_devmem_bind_dmabuf(netdev, dmabuf_fd, priv, info->extack); > | ^~~~~~~~~~~~~~~~~~~~~~ > In file included from ../net/core/netdev-genl.c:16: > net/core/devmem.h:132:1: note: declared here > 132 | net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, > | ^~~~~~~~~~~~~~~~~~~~~~ > > This is on the kunit build so guessing some compat was missed. Ah, yes, I missed updating a prototype for the netmem disabled case in the devmem.h I will fix it in the next version! Thanks a lot! Taehee Yoo
Powered by blists - more mailing lists