Skip to content

Commit ce499c1

Browse files
TaeheeYooNipaLocal
authored and
NipaLocal
committed
net: devmem: fix kernel panic when socket close after module unload
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 [kernel-patches#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+ kernel-patches#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) Fixes: 1d22d30 ("net: drop rtnl_lock for queue_mgmt operations") Signed-off-by: Taehee Yoo <[email protected]> Signed-off-by: NipaLocal <nipa@local>
1 parent 3a2d820 commit ce499c1

File tree

3 files changed

+27
-9
lines changed

3 files changed

+27
-9
lines changed

net/core/devmem.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
167167

168168
struct net_devmem_dmabuf_binding *
169169
net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
170+
struct netdev_nl_sock *priv,
170171
struct netlink_ext_ack *extack)
171172
{
172173
struct net_devmem_dmabuf_binding *binding;
@@ -189,6 +190,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
189190
}
190191

191192
binding->dev = dev;
193+
binding->priv = priv;
192194

193195
err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id,
194196
binding, xa_limit_32b, &id_alloc_next,
@@ -376,12 +378,16 @@ static void mp_dmabuf_devmem_uninstall(void *mp_priv,
376378
struct netdev_rx_queue *bound_rxq;
377379
unsigned long xa_idx;
378380

381+
mutex_lock(&binding->priv->lock);
379382
xa_for_each(&binding->bound_rxqs, xa_idx, bound_rxq) {
380383
if (bound_rxq == rxq) {
381384
xa_erase(&binding->bound_rxqs, xa_idx);
385+
if (xa_empty(&binding->bound_rxqs))
386+
binding->dev = NULL;
382387
break;
383388
}
384389
}
390+
mutex_unlock(&binding->priv->lock);
385391
}
386392

387393
static const struct memory_provider_ops dmabuf_devmem_ops = {

net/core/devmem.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#define _NET_DEVMEM_H
1212

1313
#include <net/netmem.h>
14+
#include <net/netdev_netlink.h>
1415

1516
struct netlink_ext_ack;
1617

@@ -20,6 +21,7 @@ struct net_devmem_dmabuf_binding {
2021
struct sg_table *sgt;
2122
struct net_device *dev;
2223
struct gen_pool *chunk_pool;
24+
struct netdev_nl_sock *priv;
2325

2426
/* The user holds a ref (via the netlink API) for as long as they want
2527
* the binding to remain alive. Each page pool using this binding holds
@@ -63,6 +65,7 @@ struct dmabuf_genpool_chunk_owner {
6365
void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding);
6466
struct net_devmem_dmabuf_binding *
6567
net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
68+
struct netdev_nl_sock *priv,
6669
struct netlink_ext_ack *extack);
6770
void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding);
6871
int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,

net/core/netdev-genl.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -858,13 +858,11 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
858858
goto err_genlmsg_free;
859859
}
860860

861-
mutex_lock(&priv->lock);
862-
863861
err = 0;
864862
netdev = netdev_get_by_index_lock(genl_info_net(info), ifindex);
865863
if (!netdev) {
866864
err = -ENODEV;
867-
goto err_unlock_sock;
865+
goto err_genlmsg_free;
868866
}
869867
if (!netif_device_present(netdev))
870868
err = -ENODEV;
@@ -876,10 +874,11 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
876874
goto err_unlock;
877875
}
878876

879-
binding = net_devmem_bind_dmabuf(netdev, dmabuf_fd, info->extack);
877+
mutex_lock(&priv->lock);
878+
binding = net_devmem_bind_dmabuf(netdev, dmabuf_fd, priv, info->extack);
880879
if (IS_ERR(binding)) {
881880
err = PTR_ERR(binding);
882-
goto err_unlock;
881+
goto err_unlock_sock;
883882
}
884883

885884
nla_for_each_attr_type(attr, NETDEV_A_DMABUF_QUEUES,
@@ -920,18 +919,17 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
920919
if (err)
921920
goto err_unbind;
922921

923-
netdev_unlock(netdev);
924-
925922
mutex_unlock(&priv->lock);
923+
netdev_unlock(netdev);
926924

927925
return 0;
928926

929927
err_unbind:
930928
net_devmem_unbind_dmabuf(binding);
931-
err_unlock:
932-
netdev_unlock(netdev);
933929
err_unlock_sock:
934930
mutex_unlock(&priv->lock);
931+
err_unlock:
932+
netdev_unlock(netdev);
935933
err_genlmsg_free:
936934
nlmsg_free(rsp);
937935
return err;
@@ -947,14 +945,25 @@ void netdev_nl_sock_priv_destroy(struct netdev_nl_sock *priv)
947945
{
948946
struct net_devmem_dmabuf_binding *binding;
949947
struct net_devmem_dmabuf_binding *temp;
948+
netdevice_tracker dev_tracker;
950949
struct net_device *dev;
951950

952951
mutex_lock(&priv->lock);
953952
list_for_each_entry_safe(binding, temp, &priv->bindings, list) {
954953
dev = binding->dev;
954+
if (!dev) {
955+
net_devmem_unbind_dmabuf(binding);
956+
continue;
957+
}
958+
netdev_hold(dev, &dev_tracker, GFP_KERNEL);
959+
mutex_unlock(&priv->lock);
955960
netdev_lock(dev);
961+
mutex_lock(&priv->lock);
956962
net_devmem_unbind_dmabuf(binding);
963+
mutex_unlock(&priv->lock);
957964
netdev_unlock(dev);
965+
netdev_put(dev, &dev_tracker);
966+
mutex_lock(&priv->lock);
958967
}
959968
mutex_unlock(&priv->lock);
960969
}

0 commit comments

Comments
 (0)