All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] vsock/virtio: clear vqs[] to avoid a UAF on a failed PM restore
@ 2026-10-28 18:22 ` Jerome Mohm
  0 siblings, 0 replies; 5+ messages in thread
From: Jerome Mohm via B4 Relay @ 2026-10-28 18:22 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Bobby Eshleman,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez, Xuan Zhuo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: virtualization, kvm, netdev, linux-kernel, stable, Jerome Mohm

From: Jerome Mohm <jrmmhm.kernel@eldare.de>

virtio_vsock_vqs_del() frees the virtqueues via del_vqs() but never
clears vsock->vqs[]. If a later virtio_vsock_restore() fails to
reallocate them (its error path is a bare "goto out" that only unlocks
and returns), the driver is left bound with a dangling vqs[]. The next
virtio_vsock_vqs_del() - on a second freeze or on remove/unbind - then
passes a freed vring_virtqueue to virtqueue_detach_unused_buf(), a slab
use-after-free. KASAN on a guest with a virtio-vsock device on
virtio-pci, after a restore made to fail by fault injection:

  BUG: KASAN: slab-use-after-free in virtqueue_detach_unused_buf
  Read of size 4 at addr ... by task repro
   virtqueue_detach_unused_buf
   virtio_vsock_vqs_del (net/vmw_vsock/virtio_transport.c:785)
   virtio_vsock_remove
   virtio_dev_remove
   ... unbind_store
  Freed by task ...:
   kfree
   vp_del_vqs
   virtio_vsock_freeze          <- the preceding freeze freed the vq
  Allocated by task 1:
   vring_create_virtqueue
   virtio_vsock_vqs_init
   virtio_vsock_probe           <- original allocation at probe

That second virtio_vsock_vqs_del() also repeats del_vqs(), which not
every transport tolerates. virtio-mmio's vm_del_vqs() always ends in
free_irq(), but after the failed vm_find_vqs() the device no longer
holds its IRQ, so on virtio-mmio the second call also warns:

  Trying to free already-free IRQ 51
  WARNING: kernel/irq/manage.c:1894 at free_irq+0x23e/0x840
   vm_del_vqs
   virtio_vsock_vqs_del
   virtio_vsock_remove          (or virtio_vsock_freeze)

virtio-pci's vp_del_vqs() is idempotent, so the PCI case shows only
the use-after-free.

Clear vsock->vqs[] after del_vqs() and when virtio_find_vqs() fails in
virtio_vsock_vqs_init(), and treat an empty vqs[] as "no virtqueues
are live": virtio_vsock_vqs_del() then still resets the device
(virtio_dev_remove() expects a reset device) and purges the send
queue, but skips the detach loops and del_vqs(). find_vqs() cleans up
after itself on failure, so skipping the second del_vqs() leaks
nothing. Clearing the stale pointers follows virtio_blk
commit 0739c2c6a015 ("virtio_blk: NULL out vqs to avoid double free on
failed resume").

The use-after-free was found with the help of an LLM coding assistant
(Anthropic's Claude); the free_irq() warning on virtio-mmio was reported
by the netdev Sashiko AI review of an earlier version of this patch.

Testing: on a private VM with a KASAN fuzz kernel, over virtio-pci and
over virtio-mmio (QEMU microvm). The restore allocation failure was
forced with CONFIG_FAILSLAB steered by the fault-injection stacktrace
filter to the virtio-vsock restore path only; a guest-root program
drives a pm_test=devices freeze/restore cycle (so .restore returns
-ENOMEM) and then unbinds the device, or runs a second freeze/restore
cycle. Without a fix (v7.3-rc5 based) the PCI run reports the
slab-use-after-free above. With only the vqs[] clearing (v1 of this
patch, on net 363c8c79220d) the MMIO runs report the free_irq warning on
remove and on the second freeze. With this patch on the same base,
restore still fails with -ENOMEM on both transports and neither report
appears; the device works again after a second, successful
freeze/restore cycle and after unbind/bind, and a normal freeze/restore
and unbind/bind still leave a working device. A find_vqs() failure after
some virtqueues were set up was not exercised (each restore attempt
failed at its first allocation, before any virtqueue was set up); that
clear is covered by code reading only. The three single-VM tests of
tools/testing/selftests/vsock/vmtest.sh (vm_server_host_client,
vm_client_host_server, vm_loopback; 44 vsock_test cases each) pass on
this tree, as on net; on both kernels the host-side runs skip the SO_INQ
cases (SO_INQ not supported by the host kernel). The ns_* tests of
vmtest.sh were not run (they need root on the host). checkpatch.pl
--strict is clean and the fuzz-config build is warning-free. The
reproducer needs guest root, CONFIG_FAILSLAB with
CONFIG_FAULT_INJECTION_DEBUG_FS and
CONFIG_FAULT_INJECTION_STACKTRACE_FILTER, and CONFIG_HIBERNATION with
CONFIG_PM_DEBUG for pm_test; it is available on request.

Fixes: bd50c5dc182b ("vsock/virtio: add support for device suspend/resume")
Cc: stable@vger.kernel.org
Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>
---
The use-after-free was found with the help of Anthropic's Claude; the
virtio-mmio warning comes from the netdev Sashiko review.

Changes in v2:
- Skip the detach loops and del_vqs() in virtio_vsock_vqs_del() when no
  virtqueues are live, so the teardown after a failed restore does not
  free virtio-mmio's IRQ a second time (Sashiko review).
- Use an empty vqs[] as the 'no live virtqueues' marker instead of v1's
  per-loop NULL checks, and purge the send queue before the early
  return.
- Commit message: cite only virtio_blk 0739c2c6a015; drop the wrong
  virtio_rtc and virtio_console references (Sashiko review). Drop the
  commit ID from the code comment.
- Tested on virtio-mmio (QEMU microvm) as well as virtio-pci.
- Drop the Assisted-by trailer (removed upstream); tool use is
  disclosed above instead.
- Link to v1: https://patch.msgid.link/20261001-vsock-restore-uaf-send-v1-1-5da60f707506@eldare.de
---
 net/vmw_vsock/virtio_transport.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 4f9aa9c4c3aa..bba594b7fc14 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -714,8 +714,14 @@ 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) {
+		/* On failure virtio_find_vqs() can leave freed virtqueues or
+		 * error pointers in vsock->vqs[]. Clear it: an empty vqs[]
+		 * tells virtio_vsock_vqs_del() that no virtqueues are live.
+		 */
+		memset(vsock->vqs, 0, sizeof(vsock->vqs));
 		return ret;
+	}
 
 	virtio_vsock_update_guest_cid(vsock);
 
@@ -781,6 +787,15 @@ static void virtio_vsock_vqs_del(struct virtio_vsock *vsock)
 	 */
 	virtio_reset_device(vdev);
 
+	virtio_vsock_skb_queue_purge(&vsock->send_pkt_queue);
+
+	/* No virtqueues are live, e.g. after a failed restore: they were
+	 * already deleted, and a second del_vqs() is not safe on every
+	 * transport (virtio-mmio would free its 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 +806,13 @@ 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);
+
+	/* The virtqueues are freed: clear the stale pointers, so that a
+	 * virtio_vsock_vqs_del() after a failed restore skips them.
+	 */
+	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 <jrmmhm.kernel@eldare.de>



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net v2] vsock/virtio: clear vqs[] to avoid a UAF on a failed PM restore
@ 2026-10-28 18:22 ` Jerome Mohm
  0 siblings, 0 replies; 5+ messages in thread
From: Jerome Mohm @ 2026-10-28 18:22 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Bobby Eshleman,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez, Xuan Zhuo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: virtualization, kvm, netdev, linux-kernel, stable, Jerome Mohm

virtio_vsock_vqs_del() frees the virtqueues via del_vqs() but never
clears vsock->vqs[]. If a later virtio_vsock_restore() fails to
reallocate them (its error path is a bare "goto out" that only unlocks
and returns), the driver is left bound with a dangling vqs[]. The next
virtio_vsock_vqs_del() - on a second freeze or on remove/unbind - then
passes a freed vring_virtqueue to virtqueue_detach_unused_buf(), a slab
use-after-free. KASAN on a guest with a virtio-vsock device on
virtio-pci, after a restore made to fail by fault injection:

  BUG: KASAN: slab-use-after-free in virtqueue_detach_unused_buf
  Read of size 4 at addr ... by task repro
   virtqueue_detach_unused_buf
   virtio_vsock_vqs_del (net/vmw_vsock/virtio_transport.c:785)
   virtio_vsock_remove
   virtio_dev_remove
   ... unbind_store
  Freed by task ...:
   kfree
   vp_del_vqs
   virtio_vsock_freeze          <- the preceding freeze freed the vq
  Allocated by task 1:
   vring_create_virtqueue
   virtio_vsock_vqs_init
   virtio_vsock_probe           <- original allocation at probe

That second virtio_vsock_vqs_del() also repeats del_vqs(), which not
every transport tolerates. virtio-mmio's vm_del_vqs() always ends in
free_irq(), but after the failed vm_find_vqs() the device no longer
holds its IRQ, so on virtio-mmio the second call also warns:

  Trying to free already-free IRQ 51
  WARNING: kernel/irq/manage.c:1894 at free_irq+0x23e/0x840
   vm_del_vqs
   virtio_vsock_vqs_del
   virtio_vsock_remove          (or virtio_vsock_freeze)

virtio-pci's vp_del_vqs() is idempotent, so the PCI case shows only
the use-after-free.

Clear vsock->vqs[] after del_vqs() and when virtio_find_vqs() fails in
virtio_vsock_vqs_init(), and treat an empty vqs[] as "no virtqueues
are live": virtio_vsock_vqs_del() then still resets the device
(virtio_dev_remove() expects a reset device) and purges the send
queue, but skips the detach loops and del_vqs(). find_vqs() cleans up
after itself on failure, so skipping the second del_vqs() leaks
nothing. Clearing the stale pointers follows virtio_blk
commit 0739c2c6a015 ("virtio_blk: NULL out vqs to avoid double free on
failed resume").

The use-after-free was found with the help of an LLM coding assistant
(Anthropic's Claude); the free_irq() warning on virtio-mmio was reported
by the netdev Sashiko AI review of an earlier version of this patch.

Testing: on a private VM with a KASAN fuzz kernel, over virtio-pci and
over virtio-mmio (QEMU microvm). The restore allocation failure was
forced with CONFIG_FAILSLAB steered by the fault-injection stacktrace
filter to the virtio-vsock restore path only; a guest-root program
drives a pm_test=devices freeze/restore cycle (so .restore returns
-ENOMEM) and then unbinds the device, or runs a second freeze/restore
cycle. Without a fix (v7.3-rc5 based) the PCI run reports the
slab-use-after-free above. With only the vqs[] clearing (v1 of this
patch, on net 363c8c79220d) the MMIO runs report the free_irq warning on
remove and on the second freeze. With this patch on the same base,
restore still fails with -ENOMEM on both transports and neither report
appears; the device works again after a second, successful
freeze/restore cycle and after unbind/bind, and a normal freeze/restore
and unbind/bind still leave a working device. A find_vqs() failure after
some virtqueues were set up was not exercised (each restore attempt
failed at its first allocation, before any virtqueue was set up); that
clear is covered by code reading only. The three single-VM tests of
tools/testing/selftests/vsock/vmtest.sh (vm_server_host_client,
vm_client_host_server, vm_loopback; 44 vsock_test cases each) pass on
this tree, as on net; on both kernels the host-side runs skip the SO_INQ
cases (SO_INQ not supported by the host kernel). The ns_* tests of
vmtest.sh were not run (they need root on the host). checkpatch.pl
--strict is clean and the fuzz-config build is warning-free. The
reproducer needs guest root, CONFIG_FAILSLAB with
CONFIG_FAULT_INJECTION_DEBUG_FS and
CONFIG_FAULT_INJECTION_STACKTRACE_FILTER, and CONFIG_HIBERNATION with
CONFIG_PM_DEBUG for pm_test; it is available on request.

Fixes: bd50c5dc182b ("vsock/virtio: add support for device suspend/resume")
Cc: stable@vger.kernel.org
Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>
---
The use-after-free was found with the help of Anthropic's Claude; the
virtio-mmio warning comes from the netdev Sashiko review.

Changes in v2:
- Skip the detach loops and del_vqs() in virtio_vsock_vqs_del() when no
  virtqueues are live, so the teardown after a failed restore does not
  free virtio-mmio's IRQ a second time (Sashiko review).
- Use an empty vqs[] as the 'no live virtqueues' marker instead of v1's
  per-loop NULL checks, and purge the send queue before the early
  return.
- Commit message: cite only virtio_blk 0739c2c6a015; drop the wrong
  virtio_rtc and virtio_console references (Sashiko review). Drop the
  commit ID from the code comment.
- Tested on virtio-mmio (QEMU microvm) as well as virtio-pci.
- Drop the Assisted-by trailer (removed upstream); tool use is
  disclosed above instead.
- Link to v1: https://patch.msgid.link/20261001-vsock-restore-uaf-send-v1-1-5da60f707506@eldare.de
---
 net/vmw_vsock/virtio_transport.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 4f9aa9c4c3aa..bba594b7fc14 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -714,8 +714,14 @@ 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) {
+		/* On failure virtio_find_vqs() can leave freed virtqueues or
+		 * error pointers in vsock->vqs[]. Clear it: an empty vqs[]
+		 * tells virtio_vsock_vqs_del() that no virtqueues are live.
+		 */
+		memset(vsock->vqs, 0, sizeof(vsock->vqs));
 		return ret;
+	}
 
 	virtio_vsock_update_guest_cid(vsock);
 
@@ -781,6 +787,15 @@ static void virtio_vsock_vqs_del(struct virtio_vsock *vsock)
 	 */
 	virtio_reset_device(vdev);
 
+	virtio_vsock_skb_queue_purge(&vsock->send_pkt_queue);
+
+	/* No virtqueues are live, e.g. after a failed restore: they were
+	 * already deleted, and a second del_vqs() is not safe on every
+	 * transport (virtio-mmio would free its 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 +806,13 @@ 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);
+
+	/* The virtqueues are freed: clear the stale pointers, so that a
+	 * virtio_vsock_vqs_del() after a failed restore skips them.
+	 */
+	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 <jrmmhm.kernel@eldare.de>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net v2] vsock/virtio: clear vqs[] to avoid a UAF on a failed PM restore
  2026-10-28 18:22 ` Jerome Mohm
  (?)
@ 2026-10-28 18:27 ` sashiko-bot
  -1 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-28 18:27 UTC (permalink / raw)
  To: Jerome Mohm; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez

> From: Jerome Mohm <jrmmhm.kernel@eldare.de>
> 
> virtio_vsock_vqs_del() frees the virtqueues via del_vqs() but never
> clears vsock->vqs[]. If a later virtio_vsock_restore() fails to
> reallocate them (its error path is a bare "goto out" that only unlocks
> and returns), the driver is left bound with a dangling vqs[]. The next
> virtio_vsock_vqs_del() - on a second freeze or on remove/unbind - then
> passes a freed vring_virtqueue to virtqueue_detach_unused_buf(), a slab
> use-after-free. KASAN on a guest with a virtio-vsock device on
> virtio-pci, after a restore made to fail by fault injection:
> 
>   BUG: KASAN: slab-use-after-free in virtqueue_detach_unused_buf
>   Read of size 4 at addr ... by task repro
>    virtqueue_detach_unused_buf
>    virtio_vsock_vqs_del (net/vmw_vsock/virtio_transport.c:785)
> [ ... ]
> CONFIG_PM_DEBUG for pm_test; it is available on request.
> 
> Fixes: bd50c5dc182b ("vsock/virtio: add support for device suspend/resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-vsock-restore-uaf-send-v2-1-675aad6da714@eldare.de?part=1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v2] vsock/virtio: clear vqs[] to avoid a UAF on a failed PM restore
  2026-10-28 18:22 ` Jerome Mohm
  (?)
  (?)
@ 2026-10-30 13:51 ` Stefano Garzarella
  2026-10-30 14:21   ` Michael S. Tsirkin
  -1 siblings, 1 reply; 5+ messages in thread
From: Stefano Garzarella @ 2026-10-30 13:51 UTC (permalink / raw)
  To: jrmmhm.kernel
  Cc: Stefan Hajnoczi, Bobby Eshleman, Michael S. Tsirkin, Jason Wang,
	Eugenio Pérez, Xuan Zhuo, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, virtualization, kvm,
	netdev, linux-kernel, stable

On Wed, Oct 28, 2026 at 07:22:43PM +0100, Jerome Mohm via B4 Relay wrote:
>From: Jerome Mohm <jrmmhm.kernel@eldare.de>
>
>virtio_vsock_vqs_del() frees the virtqueues via del_vqs() but never
>clears vsock->vqs[]. If a later virtio_vsock_restore() fails to
>reallocate them (its error path is a bare "goto out" that only unlocks
>and returns), the driver is left bound with a dangling vqs[]. The next
>virtio_vsock_vqs_del() - on a second freeze or on remove/unbind - then
>passes a freed vring_virtqueue to virtqueue_detach_unused_buf(), a slab
>use-after-free. KASAN on a guest with a virtio-vsock device on
>virtio-pci, after a restore made to fail by fault injection:
>
>  BUG: KASAN: slab-use-after-free in virtqueue_detach_unused_buf
>  Read of size 4 at addr ... by task repro
>   virtqueue_detach_unused_buf
>   virtio_vsock_vqs_del (net/vmw_vsock/virtio_transport.c:785)
>   virtio_vsock_remove
>   virtio_dev_remove
>   ... unbind_store
>  Freed by task ...:
>   kfree
>   vp_del_vqs
>   virtio_vsock_freeze          <- the preceding freeze freed the vq
>  Allocated by task 1:
>   vring_create_virtqueue
>   virtio_vsock_vqs_init
>   virtio_vsock_probe           <- original allocation at probe
>
>That second virtio_vsock_vqs_del() also repeats del_vqs(), which not
>every transport tolerates. virtio-mmio's vm_del_vqs() always ends in
>free_irq(), but after the failed vm_find_vqs() the device no longer
>holds its IRQ, so on virtio-mmio the second call also warns:
>
>  Trying to free already-free IRQ 51
>  WARNING: kernel/irq/manage.c:1894 at free_irq+0x23e/0x840
>   vm_del_vqs
>   virtio_vsock_vqs_del
>   virtio_vsock_remove          (or virtio_vsock_freeze)
>
>virtio-pci's vp_del_vqs() is idempotent, so the PCI case shows only
>the use-after-free.
>
>Clear vsock->vqs[] after del_vqs() and when virtio_find_vqs() fails in
>virtio_vsock_vqs_init(), and treat an empty vqs[] as "no virtqueues
>are live": virtio_vsock_vqs_del() then still resets the device
>(virtio_dev_remove() expects a reset device) and purges the send
>queue, but skips the detach loops and del_vqs(). find_vqs() cleans up
>after itself on failure, so skipping the second del_vqs() leaks
>nothing. Clearing the stale pointers follows virtio_blk
>commit 0739c2c6a015 ("virtio_blk: NULL out vqs to avoid double free on
>failed resume").
>
>The use-after-free was found with the help of an LLM coding assistant
>(Anthropic's Claude); the free_irq() warning on virtio-mmio was reported
>by the netdev Sashiko AI review of an earlier version of this patch.
>
>Testing: on a private VM with a KASAN fuzz kernel, over virtio-pci and
>over virtio-mmio (QEMU microvm). The restore allocation failure was
>forced with CONFIG_FAILSLAB steered by the fault-injection stacktrace
>filter to the virtio-vsock restore path only; a guest-root program
>drives a pm_test=devices freeze/restore cycle (so .restore returns
>-ENOMEM) and then unbinds the device, or runs a second freeze/restore
>cycle. Without a fix (v7.3-rc5 based) the PCI run reports the
>slab-use-after-free above. With only the vqs[] clearing (v1 of this
>patch, on net 363c8c79220d) the MMIO runs report the free_irq warning on
>remove and on the second freeze. With this patch on the same base,
>restore still fails with -ENOMEM on both transports and neither report
>appears; the device works again after a second, successful
>freeze/restore cycle and after unbind/bind, and a normal freeze/restore
>and unbind/bind still leave a working device. A find_vqs() failure after
>some virtqueues were set up was not exercised (each restore attempt
>failed at its first allocation, before any virtqueue was set up); that
>clear is covered by code reading only. The three single-VM tests of
>tools/testing/selftests/vsock/vmtest.sh (vm_server_host_client,
>vm_client_host_server, vm_loopback; 44 vsock_test cases each) pass on
>this tree, as on net; on both kernels the host-side runs skip the SO_INQ
>cases (SO_INQ not supported by the host kernel). The ns_* tests of
>vmtest.sh were not run (they need root on the host). checkpatch.pl
>--strict is clean and the fuzz-config build is warning-free. The

Do we really need to write in a commit message that checkpatch.pl 
--strict is clean?

Descriptive commits are great, but we're going a bit too far here. 
Please include only the relevant information; for example, I think the 
part after "Testing:" could be summarized in a few lines or removed 
altogether.

>reproducer needs guest root, CONFIG_FAILSLAB with
>CONFIG_FAULT_INJECTION_DEBUG_FS and
>CONFIG_FAULT_INJECTION_STACKTRACE_FILTER, and CONFIG_HIBERNATION with
>CONFIG_PM_DEBUG for pm_test; it is available on request.
>
>Fixes: bd50c5dc182b ("vsock/virtio: add support for device suspend/resume")
>Cc: stable@vger.kernel.org
>Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>
>---
>The use-after-free was found with the help of Anthropic's Claude; the
>virtio-mmio warning comes from the netdev Sashiko review.

This is already in the commit description, no?

>
>Changes in v2:
>- Skip the detach loops and del_vqs() in virtio_vsock_vqs_del() when no
>  virtqueues are live, so the teardown after a failed restore does not
>  free virtio-mmio's IRQ a second time (Sashiko review).
>- Use an empty vqs[] as the 'no live virtqueues' marker instead of v1's
>  per-loop NULL checks, and purge the send queue before the early
>  return.
>- Commit message: cite only virtio_blk 0739c2c6a015; drop the wrong
>  virtio_rtc and virtio_console references (Sashiko review). Drop the
>  commit ID from the code comment.
>- Tested on virtio-mmio (QEMU microvm) as well as virtio-pci.
>- Drop the Assisted-by trailer (removed upstream); tool use is
>  disclosed above instead.
>- Link to v1: https://patch.msgid.link/20261001-vsock-restore-uaf-send-v1-1-5da60f707506@eldare.de
>---
> net/vmw_vsock/virtio_transport.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index 4f9aa9c4c3aa..bba594b7fc14 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -714,8 +714,14 @@ 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) {
>+		/* On failure virtio_find_vqs() can leave freed virtqueues or
>+		 * error pointers in vsock->vqs[]. Clear it: an empty vqs[]
>+		 * tells virtio_vsock_vqs_del() that no virtqueues are live.
>+		 */
>+		memset(vsock->vqs, 0, sizeof(vsock->vqs));

Should we move this in the virtio_find_vqs() ?

Maybe not for this patch, but for long term it could be a good idea IMO.

> 		return ret;
>+	}
>
> 	virtio_vsock_update_guest_cid(vsock);
>
>@@ -781,6 +787,15 @@ static void virtio_vsock_vqs_del(struct virtio_vsock *vsock)
> 	 */
> 	virtio_reset_device(vdev);
>
>+	virtio_vsock_skb_queue_purge(&vsock->send_pkt_queue);
>+
>+	/* No virtqueues are live, e.g. after a failed restore: they were
>+	 * already deleted, and a second del_vqs() is not safe on every
>+	 * transport (virtio-mmio would free its 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 +806,13 @@ 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);
>+
>+	/* The virtqueues are freed: clear the stale pointers, so that a
>+	 * virtio_vsock_vqs_del() after a failed restore skips them.
>+	 */
>+	memset(vsock->vqs, 0, sizeof(vsock->vqs));

Same above, should we introduce a virtio_del_vqs() to do both and call 
it in all drivers?

Or skip del_vqs() if vdev->vqs is empty.

I guess this issue could be spread around virtio drivers, so would be 
nice to add common API in the virtio core to be used in all of them.

That said, for a fix, this patch LGTM, my comments were more for a long 
term work.

With a fixed commit message I can give my R-b.

Thanks,
Stefano

> }
>
> static int virtio_vsock_probe(struct virtio_device *vdev)
>
>---
>base-commit: 363c8c79220d0db4d65dcd570529ddda15067cb5
>change-id: 20261001-vsock-restore-uaf-send-1fc7396fa1bd
>
>Best regards,
>--
>Jerome Mohm <jrmmhm.kernel@eldare.de>
>
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v2] vsock/virtio: clear vqs[] to avoid a UAF on a failed PM restore
  2026-10-30 13:51 ` Stefano Garzarella
@ 2026-10-30 14:21   ` Michael S. Tsirkin
  0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2026-10-30 14:21 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: jrmmhm.kernel, Stefan Hajnoczi, Bobby Eshleman, Jason Wang,
	Eugenio Pérez, Xuan Zhuo, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, virtualization, kvm,
	netdev, linux-kernel, stable

On Fri, Oct 30, 2026 at 02:51:26PM +0100, Stefano Garzarella wrote:
> On Wed, Oct 28, 2026 at 07:22:43PM +0100, Jerome Mohm via B4 Relay wrote:
> > From: Jerome Mohm <jrmmhm.kernel@eldare.de>
> > 
> > virtio_vsock_vqs_del() frees the virtqueues via del_vqs() but never
> > clears vsock->vqs[]. If a later virtio_vsock_restore() fails to
> > reallocate them (its error path is a bare "goto out" that only unlocks
> > and returns), the driver is left bound with a dangling vqs[]. The next
> > virtio_vsock_vqs_del() - on a second freeze or on remove/unbind - then
> > passes a freed vring_virtqueue to virtqueue_detach_unused_buf(), a slab
> > use-after-free. KASAN on a guest with a virtio-vsock device on
> > virtio-pci, after a restore made to fail by fault injection:
> > 
> >  BUG: KASAN: slab-use-after-free in virtqueue_detach_unused_buf
> >  Read of size 4 at addr ... by task repro
> >   virtqueue_detach_unused_buf
> >   virtio_vsock_vqs_del (net/vmw_vsock/virtio_transport.c:785)
> >   virtio_vsock_remove
> >   virtio_dev_remove
> >   ... unbind_store
> >  Freed by task ...:
> >   kfree
> >   vp_del_vqs
> >   virtio_vsock_freeze          <- the preceding freeze freed the vq
> >  Allocated by task 1:
> >   vring_create_virtqueue
> >   virtio_vsock_vqs_init
> >   virtio_vsock_probe           <- original allocation at probe
> > 
> > That second virtio_vsock_vqs_del() also repeats del_vqs(), which not
> > every transport tolerates. virtio-mmio's vm_del_vqs() always ends in
> > free_irq(), but after the failed vm_find_vqs() the device no longer
> > holds its IRQ, so on virtio-mmio the second call also warns:
> > 
> >  Trying to free already-free IRQ 51
> >  WARNING: kernel/irq/manage.c:1894 at free_irq+0x23e/0x840
> >   vm_del_vqs
> >   virtio_vsock_vqs_del
> >   virtio_vsock_remove          (or virtio_vsock_freeze)
> > 
> > virtio-pci's vp_del_vqs() is idempotent, so the PCI case shows only
> > the use-after-free.
> > 
> > Clear vsock->vqs[] after del_vqs() and when virtio_find_vqs() fails in
> > virtio_vsock_vqs_init(), and treat an empty vqs[] as "no virtqueues
> > are live": virtio_vsock_vqs_del() then still resets the device
> > (virtio_dev_remove() expects a reset device) and purges the send
> > queue, but skips the detach loops and del_vqs(). find_vqs() cleans up
> > after itself on failure, so skipping the second del_vqs() leaks
> > nothing. Clearing the stale pointers follows virtio_blk
> > commit 0739c2c6a015 ("virtio_blk: NULL out vqs to avoid double free on
> > failed resume").
> > 
> > The use-after-free was found with the help of an LLM coding assistant
> > (Anthropic's Claude); the free_irq() warning on virtio-mmio was reported
> > by the netdev Sashiko AI review of an earlier version of this patch.
> > 
> > Testing: on a private VM with a KASAN fuzz kernel, over virtio-pci and
> > over virtio-mmio (QEMU microvm). The restore allocation failure was
> > forced with CONFIG_FAILSLAB steered by the fault-injection stacktrace
> > filter to the virtio-vsock restore path only; a guest-root program
> > drives a pm_test=devices freeze/restore cycle (so .restore returns
> > -ENOMEM) and then unbinds the device, or runs a second freeze/restore
> > cycle. Without a fix (v7.3-rc5 based) the PCI run reports the
> > slab-use-after-free above. With only the vqs[] clearing (v1 of this
> > patch, on net 363c8c79220d) the MMIO runs report the free_irq warning on
> > remove and on the second freeze. With this patch on the same base,
> > restore still fails with -ENOMEM on both transports and neither report
> > appears; the device works again after a second, successful
> > freeze/restore cycle and after unbind/bind, and a normal freeze/restore
> > and unbind/bind still leave a working device. A find_vqs() failure after
> > some virtqueues were set up was not exercised (each restore attempt
> > failed at its first allocation, before any virtqueue was set up); that
> > clear is covered by code reading only. The three single-VM tests of
> > tools/testing/selftests/vsock/vmtest.sh (vm_server_host_client,
> > vm_client_host_server, vm_loopback; 44 vsock_test cases each) pass on
> > this tree, as on net; on both kernels the host-side runs skip the SO_INQ
> > cases (SO_INQ not supported by the host kernel). The ns_* tests of
> > vmtest.sh were not run (they need root on the host). checkpatch.pl
> > --strict is clean and the fuzz-config build is warning-free. The
> 
> Do we really need to write in a commit message that checkpatch.pl --strict
> is clean?
> 
> Descriptive commits are great, but we're going a bit too far here.

It's just what AI writes if you tell it to make it checkpatch.pl
clean in the prompt.

> Please
> include only the relevant information; for example, I think the part after
> "Testing:" could be summarized in a few lines or removed altogether.


Yea please write it yourself and make it concise. My eyes glazed over
in the 1st paragraph.

> > reproducer needs guest root, CONFIG_FAILSLAB with
> > CONFIG_FAULT_INJECTION_DEBUG_FS and
> > CONFIG_FAULT_INJECTION_STACKTRACE_FILTER, and CONFIG_HIBERNATION with
> > CONFIG_PM_DEBUG for pm_test; it is available on request.
> > 
> > Fixes: bd50c5dc182b ("vsock/virtio: add support for device suspend/resume")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>
> > ---
> > The use-after-free was found with the help of Anthropic's Claude; the
> > virtio-mmio warning comes from the netdev Sashiko review.
> 
> This is already in the commit description, no?
> 
> > 
> > Changes in v2:
> > - Skip the detach loops and del_vqs() in virtio_vsock_vqs_del() when no
> >  virtqueues are live, so the teardown after a failed restore does not
> >  free virtio-mmio's IRQ a second time (Sashiko review).
> > - Use an empty vqs[] as the 'no live virtqueues' marker instead of v1's
> >  per-loop NULL checks, and purge the send queue before the early
> >  return.
> > - Commit message: cite only virtio_blk 0739c2c6a015; drop the wrong
> >  virtio_rtc and virtio_console references (Sashiko review). Drop the
> >  commit ID from the code comment.
> > - Tested on virtio-mmio (QEMU microvm) as well as virtio-pci.
> > - Drop the Assisted-by trailer (removed upstream); tool use is
> >  disclosed above instead.
> > - Link to v1: https://patch.msgid.link/20261001-vsock-restore-uaf-send-v1-1-5da60f707506@eldare.de
> > ---
> > net/vmw_vsock/virtio_transport.c | 24 +++++++++++++++++++++---
> > 1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> > index 4f9aa9c4c3aa..bba594b7fc14 100644
> > --- a/net/vmw_vsock/virtio_transport.c
> > +++ b/net/vmw_vsock/virtio_transport.c
> > @@ -714,8 +714,14 @@ 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) {
> > +		/* On failure virtio_find_vqs() can leave freed virtqueues or
> > +		 * error pointers in vsock->vqs[]. Clear it: an empty vqs[]
> > +		 * tells virtio_vsock_vqs_del() that no virtqueues are live.
> > +		 */
> > +		memset(vsock->vqs, 0, sizeof(vsock->vqs));
> 
> Should we move this in the virtio_find_vqs() ?
> 
> Maybe not for this patch, but for long term it could be a good idea IMO.
> 
> > 		return ret;
> > +	}
> > 
> > 	virtio_vsock_update_guest_cid(vsock);
> > 
> > @@ -781,6 +787,15 @@ static void virtio_vsock_vqs_del(struct virtio_vsock *vsock)
> > 	 */
> > 	virtio_reset_device(vdev);
> > 
> > +	virtio_vsock_skb_queue_purge(&vsock->send_pkt_queue);
> > +
> > +	/* No virtqueues are live, e.g. after a failed restore: they were
> > +	 * already deleted, and a second del_vqs() is not safe on every
> > +	 * transport (virtio-mmio would free its 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 +806,13 @@ 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);
> > +
> > +	/* The virtqueues are freed: clear the stale pointers, so that a
> > +	 * virtio_vsock_vqs_del() after a failed restore skips them.
> > +	 */
> > +	memset(vsock->vqs, 0, sizeof(vsock->vqs));
> 
> Same above, should we introduce a virtio_del_vqs() to do both and call it in
> all drivers?
> 
> Or skip del_vqs() if vdev->vqs is empty.
> 
> I guess this issue could be spread around virtio drivers, so would be nice
> to add common API in the virtio core to be used in all of them.
> 
> That said, for a fix, this patch LGTM, my comments were more for a long term
> work.
> 
> With a fixed commit message I can give my R-b.
> 
> Thanks,
> Stefano
> 
> > }
> > 
> > static int virtio_vsock_probe(struct virtio_device *vdev)
> > 
> > ---
> > base-commit: 363c8c79220d0db4d65dcd570529ddda15067cb5
> > change-id: 20261001-vsock-restore-uaf-send-1fc7396fa1bd
> > 
> > Best regards,
> > --
> > Jerome Mohm <jrmmhm.kernel@eldare.de>
> > 
> > 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-30 14:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 18:22 [PATCH net v2] vsock/virtio: clear vqs[] to avoid a UAF on a failed PM restore Jerome Mohm via B4 Relay
2026-10-28 18:22 ` Jerome Mohm
2026-10-28 18:27 ` sashiko-bot
2026-10-30 13:51 ` Stefano Garzarella
2026-10-30 14:21   ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.