Skip to content

Commit 45d8ef6

Browse files
junnan01-wumstsirkin
authored andcommitted
virtio_net: adjust the execution order of function virtnet_close during freeze
"Use after free" issue appears in suspend once race occurs when napi poll scheduls after `netif_device_detach` and before napi disables. For details, during suspend flow of virtio-net, the tx queue state is set to "__QUEUE_STATE_DRV_XOFF" by CPU-A. And at some coincidental times, if a TCP connection is still working, CPU-B does `virtnet_poll` before napi disable. In this flow, the state "__QUEUE_STATE_DRV_XOFF" of tx queue will be cleared. This is not the normal process it expects. After that, CPU-A continues to close driver then virtqueue is removed. Sequence likes below: -------------------------------------------------------------------------- CPU-A CPU-B ----- ----- suspend is called A TCP based on virtio-net still work virtnet_freeze |- virtnet_freeze_down | |- netif_device_detach | | |- netif_tx_stop_all_queues | | |- netif_tx_stop_queue | | |- set_bit | | (__QUEUE_STATE_DRV_XOFF,...) | | softirq rasied | | |- net_rx_action | | |- napi_poll | | |- virtnet_poll | | |- virtnet_poll_cleantx | | |- netif_tx_wake_queue | | |- test_and_clear_bit | | (__QUEUE_STATE_DRV_XOFF,...) | |- virtnet_close | |- virtnet_disable_queue_pair | |- virtnet_napi_tx_disable |- remove_vq_common -------------------------------------------------------------------------- When TCP delayack timer is up, a cpu gets softirq and irq handler `tcp_delack_timer_handler` will be called, which will finally call `start_xmit` in virtio net driver. Then the access to tx virtq will cause panic. The root cause of this issue is that napi tx is not disable before `netif_tx_stop_queue`, once `virnet_poll` schedules in such coincidental time, the tx queue state will be cleared. To solve this issue, adjusts the order of function `virtnet_close` in `virtnet_freeze_down`. Co-developed-by: Ying Xu <[email protected]> Signed-off-by: Ying Xu <[email protected]> Signed-off-by: Junnan Wu <[email protected]> Message-Id: <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]>
1 parent 528d92b commit 45d8ef6

File tree

1 file changed

+4
-3
lines changed

1 file changed

+4
-3
lines changed

drivers/net/virtio_net.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5758,14 +5758,15 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
57585758
disable_rx_mode_work(vi);
57595759
flush_work(&vi->rx_mode_work);
57605760

5761-
netif_tx_lock_bh(vi->dev);
5762-
netif_device_detach(vi->dev);
5763-
netif_tx_unlock_bh(vi->dev);
57645761
if (netif_running(vi->dev)) {
57655762
rtnl_lock();
57665763
virtnet_close(vi->dev);
57675764
rtnl_unlock();
57685765
}
5766+
5767+
netif_tx_lock_bh(vi->dev);
5768+
netif_device_detach(vi->dev);
5769+
netif_tx_unlock_bh(vi->dev);
57695770
}
57705771

57715772
static int init_vqs(struct virtnet_info *vi);

0 commit comments

Comments
 (0)