From: Jerome Mohm <[email protected]>
virtio_vsock_vqs_del() frees the virtqueues but leaves vsock->vqs[]
pointing at them. If virtio_vsock_restore() then fails, the driver
stays bound with a dangling vqs[], and the next freeze or unbind
passes a freed virtqueue to virtqueue_detach_unused_buf():
BUG: KASAN: slab-use-after-free in virtqueue_detach_unused_buf
virtio_vsock_vqs_del (net/vmw_vsock/virtio_transport.c:785)
virtio_vsock_remove
Freed by task ...:
vp_del_vqs
virtio_vsock_freeze
Clear vqs[] after del_vqs() and when virtio_find_vqs() fails. Return
early from virtio_vsock_vqs_del() when vqs[] is empty: a second
del_vqs() would free virtio-mmio's IRQ again ("Trying to free
already-free IRQ").
Found with the help of Anthropic's Claude and reproduced on virtio-pci
by failing the restore with failslab; the virtio-mmio IRQ issue was
found by the netdev Sashiko review. With this patch neither report
appears on virtio-pci or virtio-mmio guests, and the vsock vmtest.sh
single-VM tests pass. A find_vqs() failure after some virtqueues were
set up was not exercised.
Fixes: bd50c5dc182b ("vsock/virtio: add support for device suspend/resume")
Cc: [email protected]
Signed-off-by: Jerome Mohm <[email protected]>
---
Changes in v3:
- Shorten the commit message (Stefano, Michael).
- Drop the two comments on clearing vqs[] (Bobby) and shorten the
remaining one.
- Drop the tool note below "---"; it is in the commit message (Stefano).
- Comment changes only; not rebuilt or retested.
- Link to v2:
https://patch.msgid.link/[email protected]
---
net/vmw_vsock/virtio_transport.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 4f9aa9c4c3aa..ffe7cd93463d 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -714,8 +714,10 @@ static int virtio_vsock_vqs_init(struct virtio_vsock
*vsock)
atomic_set(&vsock->queued_replies, 0);
ret = virtio_find_vqs(vdev, VSOCK_VQ_MAX, vsock->vqs, vqs_info, NULL);
- if (ret < 0)
+ if (ret < 0) {
+ memset(vsock->vqs, 0, sizeof(vsock->vqs));
return ret;
+ }
virtio_vsock_update_guest_cid(vsock);
@@ -781,6 +783,14 @@ static void virtio_vsock_vqs_del(struct virtio_vsock
*vsock)
*/
virtio_reset_device(vdev);
+ virtio_vsock_skb_queue_purge(&vsock->send_pkt_queue);
+
+ /* After a failed restore no virtqueues are live, and a second
+ * del_vqs() would free virtio-mmio's IRQ again.
+ */
+ if (!vsock->vqs[VSOCK_VQ_RX])
+ return;
+
mutex_lock(&vsock->rx_lock);
while ((skb = virtqueue_detach_unused_buf(vsock->vqs[VSOCK_VQ_RX])))
kfree_skb(skb);
@@ -791,10 +801,10 @@ static void virtio_vsock_vqs_del(struct virtio_vsock
*vsock)
kfree_skb(skb);
mutex_unlock(&vsock->tx_lock);
- virtio_vsock_skb_queue_purge(&vsock->send_pkt_queue);
-
/* Delete virtqueues and flush outstanding callbacks if any */
vdev->config->del_vqs(vdev);
+
+ memset(vsock->vqs, 0, sizeof(vsock->vqs));
}
static int virtio_vsock_probe(struct virtio_device *vdev)
---
base-commit: 363c8c79220d0db4d65dcd570529ddda15067cb5
change-id: 20261001-vsock-restore-uaf-send-1fc7396fa1bd
Best regards,
--
Jerome Mohm <[email protected]>
