Skip to content

Commit 063f2f8

Browse files
plxtyNipaLocal
authored andcommitted
virtio-net: fix a rtnl_lock() deadlock during probing
This bug happens if the VMM sends a VIRTIO_NET_S_ANNOUNCE request while the virtio-net driver is still probing with rtnl_lock() hold, this will cause a recursive mutex in netdev_notify_peers(). Fix it by temporarily save the announce status while probing, and then in virtnet_open(), if it sees a delayed announce work is there, it starts to schedule the virtnet_config_changed_work(). Another possible solution is to directly check whether rtnl_is_locked() and call __netdev_notify_peers(), but in that way means we need to relies on netdev_queue to schedule the arp packets after ndo_open(), which we thought is not very intuitive. We've observed a softlockup with Ubuntu 24.04, and can be reproduced with QEMU sending the announce_self rapidly while booting. [ 494.167473] INFO: task swapper/0:1 blocked for more than 368 seconds. [ 494.167667] Not tainted 6.8.0-57-generic torvalds#59-Ubuntu [ 494.167810] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 494.168015] task:swapper/0 state:D stack:0 pid:1 tgid:1 ppid:0 flags:0x00004000 [ 494.168260] Call Trace: [ 494.168329] <TASK> [ 494.168389] __schedule+0x27c/0x6b0 [ 494.168495] schedule+0x33/0x110 [ 494.168585] schedule_preempt_disabled+0x15/0x30 [ 494.168709] __mutex_lock.constprop.0+0x42f/0x740 [ 494.168835] __mutex_lock_slowpath+0x13/0x20 [ 494.168949] mutex_lock+0x3c/0x50 [ 494.169039] rtnl_lock+0x15/0x20 [ 494.169128] netdev_notify_peers+0x12/0x30 [ 494.169240] virtnet_config_changed_work+0x152/0x1a0 [ 494.169377] virtnet_probe+0xa48/0xe00 [ 494.169484] ? vp_get+0x4d/0x100 [ 494.169574] virtio_dev_probe+0x1e9/0x310 [ 494.169682] really_probe+0x1c7/0x410 [ 494.169783] __driver_probe_device+0x8c/0x180 [ 494.169901] driver_probe_device+0x24/0xd0 [ 494.170011] __driver_attach+0x10b/0x210 [ 494.170117] ? __pfx___driver_attach+0x10/0x10 [ 494.170237] bus_for_each_dev+0x8d/0xf0 [ 494.170341] driver_attach+0x1e/0x30 [ 494.170440] bus_add_driver+0x14e/0x290 [ 494.170548] driver_register+0x5e/0x130 [ 494.170651] ? __pfx_virtio_net_driver_init+0x10/0x10 [ 494.170788] register_virtio_driver+0x20/0x40 [ 494.170905] virtio_net_driver_init+0x97/0xb0 [ 494.171022] do_one_initcall+0x5e/0x340 [ 494.171128] do_initcalls+0x107/0x230 [ 494.171228] ? __pfx_kernel_init+0x10/0x10 [ 494.171340] kernel_init_freeable+0x134/0x210 [ 494.171462] kernel_init+0x1b/0x200 [ 494.171560] ret_from_fork+0x47/0x70 [ 494.171659] ? __pfx_kernel_init+0x10/0x10 [ 494.171769] ret_from_fork_asm+0x1b/0x30 [ 494.171875] </TASK> Fixes: df28de7 ("virtio-net: synchronize operstate with admin state on up/down") Signed-off-by: Zigit Zo <zuozhijie@bytedance.com> Signed-off-by: NipaLocal <nipa@local>
1 parent cfe2d9f commit 063f2f8

File tree

1 file changed

+26
-17
lines changed

1 file changed

+26
-17
lines changed

drivers/net/virtio_net.c

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3174,6 +3174,10 @@ static int virtnet_open(struct net_device *dev)
31743174
if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
31753175
if (vi->status & VIRTIO_NET_S_LINK_UP)
31763176
netif_carrier_on(vi->dev);
3177+
if (vi->status & VIRTIO_NET_S_ANNOUNCE) {
3178+
vi->status &= ~VIRTIO_NET_S_ANNOUNCE;
3179+
schedule_work(&vi->config_work);
3180+
}
31773181
virtio_config_driver_enable(vi->vdev);
31783182
} else {
31793183
vi->status = VIRTIO_NET_S_LINK_UP;
@@ -6233,33 +6237,34 @@ static void virtnet_config_changed_work(struct work_struct *work)
62336237
{
62346238
struct virtnet_info *vi =
62356239
container_of(work, struct virtnet_info, config_work);
6236-
u16 v;
6240+
u16 v, changed;
62376241

62386242
if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
62396243
struct virtio_net_config, status, &v) < 0)
62406244
return;
62416245

6242-
if (v & VIRTIO_NET_S_ANNOUNCE) {
6246+
changed = vi->status ^ v;
6247+
6248+
/* Assume the device will clear announce status when ack received. */
6249+
if ((changed & VIRTIO_NET_S_ANNOUNCE) && (v & VIRTIO_NET_S_ANNOUNCE)) {
62436250
netdev_notify_peers(vi->dev);
62446251
virtnet_ack_link_announce(vi);
6252+
v &= ~VIRTIO_NET_S_ANNOUNCE;
62456253
}
62466254

6247-
/* Ignore unknown (future) status bits */
6248-
v &= VIRTIO_NET_S_LINK_UP;
6249-
6250-
if (vi->status == v)
6251-
return;
6252-
6253-
vi->status = v;
6254-
6255-
if (vi->status & VIRTIO_NET_S_LINK_UP) {
6256-
virtnet_update_settings(vi);
6257-
netif_carrier_on(vi->dev);
6258-
netif_tx_wake_all_queues(vi->dev);
6259-
} else {
6260-
netif_carrier_off(vi->dev);
6261-
netif_tx_stop_all_queues(vi->dev);
6255+
if (changed & VIRTIO_NET_S_LINK_UP) {
6256+
if (v & VIRTIO_NET_S_LINK_UP) {
6257+
virtnet_update_settings(vi);
6258+
netif_carrier_on(vi->dev);
6259+
netif_tx_wake_all_queues(vi->dev);
6260+
} else {
6261+
netif_carrier_off(vi->dev);
6262+
netif_tx_stop_all_queues(vi->dev);
6263+
}
62626264
}
6265+
6266+
/* Ignore unknown (future) status bits */
6267+
vi->status = v & (VIRTIO_NET_S_LINK_UP | VIRTIO_NET_S_ANNOUNCE);
62636268
}
62646269

62656270
static void virtnet_config_changed(struct virtio_device *vdev)
@@ -7048,6 +7053,10 @@ static int virtnet_probe(struct virtio_device *vdev)
70487053
otherwise get link status from config. */
70497054
netif_carrier_off(dev);
70507055
if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
7056+
/* If there's (rarely) an announcement, the actual work will be
7057+
* scheduled on ndo_open() to avoid recursive rtnl_lock() here.
7058+
*/
7059+
vi->status = VIRTIO_NET_S_ANNOUNCE;
70517060
virtnet_config_changed_work(&vi->config_work);
70527061
} else {
70537062
vi->status = VIRTIO_NET_S_LINK_UP;

0 commit comments

Comments
 (0)