All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] hw/9pfs/virtio: drain in-flight PDUs before virtio-9p unrealize
       [not found] <cover.1783604079.git.qemu_oss@crudebyte.com>
  2026-07-09 13:50 ` [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device Christian Schoenebeck
  2026-07-09 13:50 ` [PATCH 3/3] hw/9pfs/xen: drain in-flight PDUs before xen-9p disconnect Christian Schoenebeck
@ 2026-07-09 13:50 ` Christian Schoenebeck
  2026-07-09 14:20   ` Christian Schoenebeck
  2 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck @ 2026-07-09 13:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-stable, Greg Kurz, Jia Jia

A guest can trigger a heap-use-after-free in the virtio transport
unrealize path by submitting a Treaddir request and immediately
ejecting the device via ACPI PCI hotplug. The unrealize path frees
struct LocalData while a worker thread still holds a reference
on it, causing a UAF in local_open_nofollow().

Fix this by draining all in-flight 9p PDUs by calling v9fs_reset()
before final server cleanup. This ensures all coroutines completed,
all FIDs are closed, and no worker thread still holds references
on 9p server state when it is freed.

Reported-by: Jia Jia <physicalmtea@gmail.com>
Suggested-by: Jia Jia <physicalmtea@gmail.com>
Fixes: 6cecf09373 ("virtio-9p-device: add minimal unrealize handler")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3937
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/virtio-9p-device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 50dc93091d..1ec48fc9e0 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -243,6 +243,7 @@ static void virtio_9p_device_unrealize(DeviceState *dev)
     V9fsVirtioState *v = VIRTIO_9P(dev);
     V9fsState *s = &v->state;
 
+    v9fs_reset(s);
     virtio_delete_queue(v->vq);
     virtio_cleanup(vdev);
     v9fs_device_unrealize_common(s);
-- 
2.47.3



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

* [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
       [not found] <cover.1783604079.git.qemu_oss@crudebyte.com>
@ 2026-07-09 13:50 ` Christian Schoenebeck
  2026-07-10  7:37   ` Igor Mammedov
  2026-07-09 13:50 ` [PATCH 3/3] hw/9pfs/xen: drain in-flight PDUs before xen-9p disconnect Christian Schoenebeck
  2026-07-09 13:50 ` [PATCH 1/3] hw/9pfs/virtio: drain in-flight PDUs before virtio-9p unrealize Christian Schoenebeck
  2 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck @ 2026-07-09 13:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz, Jia Jia

Harden security by disabling hotpluggable property, to prevent
issues like fixed in the previous commit.

Virtio-9p is a pass-through file-sharing device that provides a
guest mount of a host filesystem tree. Unlike block or network
devices, guest-triggered hotplug of the 9p device has no practical
use case: the guest cannot recover from the device removal, and there
is no protocol-level device-loss scenario as with block or ethernet
devices.

Prevent guest-triggered hotplug by marking the device as
non-hotpluggable. Device removal via QMP is not affected by this, as
the QMP removal path is not reading the device's hotpluggable field
at all.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/virtio-9p-device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 1ec48fc9e0..ba43fd22d2 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -273,6 +273,7 @@ static void virtio_9p_class_init(ObjectClass *klass, const void *data)
 
     device_class_set_props(dc, virtio_9p_properties);
     dc->vmsd = &vmstate_virtio_9p;
+    dc->hotpluggable = false;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->realize = virtio_9p_device_realize;
     vdc->unrealize = virtio_9p_device_unrealize;
-- 
2.47.3



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

* [PATCH 3/3] hw/9pfs/xen: drain in-flight PDUs before xen-9p disconnect
       [not found] <cover.1783604079.git.qemu_oss@crudebyte.com>
  2026-07-09 13:50 ` [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device Christian Schoenebeck
@ 2026-07-09 13:50 ` Christian Schoenebeck
  2026-07-09 13:50 ` [PATCH 1/3] hw/9pfs/virtio: drain in-flight PDUs before virtio-9p unrealize Christian Schoenebeck
  2 siblings, 0 replies; 13+ messages in thread
From: Christian Schoenebeck @ 2026-07-09 13:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-stable, Greg Kurz, Stefano Stabellini, Anthony PERARD,
	Edgar E. Iglesias

The xen-9p disconnect path has two issues:

1. It frees the Xen9pfsRing structures while in-flight PDUs may still
   reference them via pdu->tag to index rings[]. This causes a UAF
   in xen_9pfs_push_and_notify() when worker threads resume after
   completing filesystem operations.

2. It never calls v9fs_device_unrealize_common(), which means server
   state (struct LocalData, mountfd, FIDs) is never cleaned up on
   disconnect, causing a resource leak on every guest-initiated
   disconnect.

Fix both by draining in-flight PDUs via v9fs_reset() before tearing
down rings, and calling v9fs_device_unrealize_common() to clean up
server state.

Fixes: b37eeb0201 ("xen/9pfs: introduce Xen 9pfs backend")
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/xen-9p-backend.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 24c90d97ec..3b7a68779a 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -368,10 +368,14 @@ static void xen_9pfs_evtchn_event(void *opaque)
 static void xen_9pfs_disconnect(struct XenLegacyDevice *xendev)
 {
     Xen9pfsDev *xen_9pdev = container_of(xendev, Xen9pfsDev, xendev);
+    V9fsState *s = &xen_9pdev->state;
     int i;
 
     trace_xen_9pfs_disconnect(xendev->name);
 
+    v9fs_reset(s);
+    v9fs_device_unrealize_common(s);
+
     for (i = 0; i < xen_9pdev->num_rings; i++) {
         if (xen_9pdev->rings[i].evtchndev != NULL) {
             qemu_set_fd_handler(qemu_xen_evtchn_fd(xen_9pdev->rings[i].evtchndev),
-- 
2.47.3



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

* Re: [PATCH 1/3] hw/9pfs/virtio: drain in-flight PDUs before virtio-9p unrealize
  2026-07-09 13:50 ` [PATCH 1/3] hw/9pfs/virtio: drain in-flight PDUs before virtio-9p unrealize Christian Schoenebeck
@ 2026-07-09 14:20   ` Christian Schoenebeck
  2026-07-10  1:39     ` m'te'a physical
  0 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck @ 2026-07-09 14:20 UTC (permalink / raw)
  To: Jia Jia; +Cc: qemu-devel, qemu-stable, Greg Kurz

On Thursday, 9 July 2026 15:50:36 CEST Christian Schoenebeck wrote:
> A guest can trigger a heap-use-after-free in the virtio transport
> unrealize path by submitting a Treaddir request and immediately
> ejecting the device via ACPI PCI hotplug. The unrealize path frees
> struct LocalData while a worker thread still holds a reference
> on it, causing a UAF in local_open_nofollow().
> 
> Fix this by draining all in-flight 9p PDUs by calling v9fs_reset()
> before final server cleanup. This ensures all coroutines completed,
> all FIDs are closed, and no worker thread still holds references
> on 9p server state when it is freed.
> 
> Reported-by: Jia Jia <physicalmtea@gmail.com>
> Suggested-by: Jia Jia <physicalmtea@gmail.com>
> Fixes: 6cecf09373 ("virtio-9p-device: add minimal unrealize handler")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3937
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---

Jia, if desired, I can make you the official author of this patch. If that's 
what you want, then please reply-to-all to this message with the same email 
address that you used for your report already, and acknowledge that you would 
be fine that I add a:

Signed-off-by: Jia Jia <physicalmtea@gmail.com>

tag on behalf of you, for the reason described here:

https://www.qemu.org/docs/master/devel/submitting-a-patch.html

>  hw/9pfs/virtio-9p-device.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 50dc93091d..1ec48fc9e0 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -243,6 +243,7 @@ static void virtio_9p_device_unrealize(DeviceState *dev)
> V9fsVirtioState *v = VIRTIO_9P(dev);
>      V9fsState *s = &v->state;
> 
> +    v9fs_reset(s);
>      virtio_delete_queue(v->vq);
>      virtio_cleanup(vdev);
>      v9fs_device_unrealize_common(s);




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

* Re: [PATCH 1/3] hw/9pfs/virtio: drain in-flight PDUs before virtio-9p unrealize
  2026-07-09 14:20   ` Christian Schoenebeck
@ 2026-07-10  1:39     ` m'te'a physical
  0 siblings, 0 replies; 13+ messages in thread
From: m'te'a physical @ 2026-07-10  1:39 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel, qemu-stable, Greg Kurz

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

 Hi Christian,

 Sure, thanks for handling this.

 Please make me the author of patch 1/3, and feel free to add:

 Signed-off-by: Jia Jia <physicalmtea@gmail.com>

 on my behalf.

 Thanks,
 Jia

Christian Schoenebeck <qemu_oss@crudebyte.com> 于2026年7月9日周四 22:20写道:

> On Thursday, 9 July 2026 15:50:36 CEST Christian Schoenebeck wrote:
> > A guest can trigger a heap-use-after-free in the virtio transport
> > unrealize path by submitting a Treaddir request and immediately
> > ejecting the device via ACPI PCI hotplug. The unrealize path frees
> > struct LocalData while a worker thread still holds a reference
> > on it, causing a UAF in local_open_nofollow().
> >
> > Fix this by draining all in-flight 9p PDUs by calling v9fs_reset()
> > before final server cleanup. This ensures all coroutines completed,
> > all FIDs are closed, and no worker thread still holds references
> > on 9p server state when it is freed.
> >
> > Reported-by: Jia Jia <physicalmtea@gmail.com>
> > Suggested-by: Jia Jia <physicalmtea@gmail.com>
> > Fixes: 6cecf09373 ("virtio-9p-device: add minimal unrealize handler")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3937
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
>
> Jia, if desired, I can make you the official author of this patch. If
> that's
> what you want, then please reply-to-all to this message with the same
> email
> address that you used for your report already, and acknowledge that you
> would
> be fine that I add a:
>
> Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>
> tag on behalf of you, for the reason described here:
>
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html
>
> >  hw/9pfs/virtio-9p-device.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index 50dc93091d..1ec48fc9e0 100644
> > --- a/hw/9pfs/virtio-9p-device.c
> > +++ b/hw/9pfs/virtio-9p-device.c
> > @@ -243,6 +243,7 @@ static void virtio_9p_device_unrealize(DeviceState
> *dev)
> > V9fsVirtioState *v = VIRTIO_9P(dev);
> >      V9fsState *s = &v->state;
> >
> > +    v9fs_reset(s);
> >      virtio_delete_queue(v->vq);
> >      virtio_cleanup(vdev);
> >      v9fs_device_unrealize_common(s);
>
>
>

[-- Attachment #2: Type: text/html, Size: 3425 bytes --]

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

* Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
  2026-07-09 13:50 ` [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device Christian Schoenebeck
@ 2026-07-10  7:37   ` Igor Mammedov
  2026-07-10  8:06     ` Christian Schoenebeck
  0 siblings, 1 reply; 13+ messages in thread
From: Igor Mammedov @ 2026-07-10  7:37 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel, Greg Kurz, Jia Jia

On Thu, 9 Jul 2026 15:50:36 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> Harden security by disabling hotpluggable property, to prevent
> issues like fixed in the previous commit.
> 
> Virtio-9p is a pass-through file-sharing device that provides a
> guest mount of a host filesystem tree. Unlike block or network
> devices, guest-triggered hotplug of the 9p device has no practical
> use case: the guest cannot recover from the device removal, and there
> is no protocol-level device-loss scenario as with block or ethernet
> devices.

hmh, I'm no maintainer of 9pfs, but to me it looks like any other
storage device.
One should be able to unmount/stop using it and unplug
(it doesn't really matter if unplug is triggered by guest or host side).

> Prevent guest-triggered hotplug by marking the device as
> non-hotpluggable. Device removal via QMP is not affected by this, as
> the QMP removal path is not reading the device's hotpluggable field
> at all.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  hw/9pfs/virtio-9p-device.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 1ec48fc9e0..ba43fd22d2 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -273,6 +273,7 @@ static void virtio_9p_class_init(ObjectClass *klass, const void *data)
>  
>      device_class_set_props(dc, virtio_9p_properties);
>      dc->vmsd = &vmstate_virtio_9p;
> +    dc->hotpluggable = false;
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>      vdc->realize = virtio_9p_device_realize;
>      vdc->unrealize = virtio_9p_device_unrealize;



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

* Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
  2026-07-10  7:37   ` Igor Mammedov
@ 2026-07-10  8:06     ` Christian Schoenebeck
  2026-07-10 10:23       ` Igor Mammedov
  0 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck @ 2026-07-10  8:06 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, Greg Kurz, Jia Jia

On Friday, 10 July 2026 09:37:37 CEST Igor Mammedov wrote:
> On Thu, 9 Jul 2026 15:50:36 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > Harden security by disabling hotpluggable property, to prevent
> > issues like fixed in the previous commit.
> > 
> > Virtio-9p is a pass-through file-sharing device that provides a
> > guest mount of a host filesystem tree. Unlike block or network
> > devices, guest-triggered hotplug of the 9p device has no practical
> > use case: the guest cannot recover from the device removal, and there
> > is no protocol-level device-loss scenario as with block or ethernet
> > devices.
> 
> hmh, I'm no maintainer of 9pfs, but to me it looks like any other
> storage device.
> One should be able to unmount/stop using it and unplug
> (it doesn't really matter if unplug is triggered by guest or host side).

Guest could still unmount and stop using the 9pfs device. But why should guest 
be able to unrealize the 9pfs device at any time? What should be the purpose 
for this particular device?

/Christian




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

* Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
  2026-07-10  8:06     ` Christian Schoenebeck
@ 2026-07-10 10:23       ` Igor Mammedov
  2026-07-10 10:49         ` Christian Schoenebeck
  0 siblings, 1 reply; 13+ messages in thread
From: Igor Mammedov @ 2026-07-10 10:23 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel, Greg Kurz, Jia Jia

On Fri, 10 Jul 2026 10:06:29 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Friday, 10 July 2026 09:37:37 CEST Igor Mammedov wrote:
> > On Thu, 9 Jul 2026 15:50:36 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:  
> > > Harden security by disabling hotpluggable property, to prevent
> > > issues like fixed in the previous commit.
> > > 
> > > Virtio-9p is a pass-through file-sharing device that provides a
> > > guest mount of a host filesystem tree. Unlike block or network
> > > devices, guest-triggered hotplug of the 9p device has no practical
> > > use case: the guest cannot recover from the device removal, and there
> > > is no protocol-level device-loss scenario as with block or ethernet
> > > devices.  
> > 
> > hmh, I'm no maintainer of 9pfs, but to me it looks like any other
> > storage device.
> > One should be able to unmount/stop using it and unplug
> > (it doesn't really matter if unplug is triggered by guest or host side).  
> 
> Guest could still unmount and stop using the 9pfs device. But why should guest 
> be able to unrealize the 9pfs device at any time? What should be the purpose 
> for this particular device?

unrealize is part of unplug process, at the end of unplug no device should be left attached.
You might not need/use it but others might be using it actively.

--- 

In QEMU impl. it's cooperative process (we don't do surprise removal of PCI devices),
where host triggers plug/unplug process and guest side completes it, by:
  1. freeing resources on its side 1st (hopefully)
  2. issuing eject/poweroff (either via ACPI or native PCIE-HP/SHPC interface)

If guest fails to do #1 it is able to abort process, or it might continue and
do #2 with a risk of loosing data but it's guest's business/policy.

If you need to disable hotplug for a specific PCI device instance,
use 'hotplug' CLI option on pcie bridge. If you use other than PCI variant
of virtio device, then unplug as you describe it isn't reachable, and if it
should be disabled in code, it's up to actual frontend device that exposes
underlying virtio one.

> /Christian
> 
> 



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

* Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
  2026-07-10 10:23       ` Igor Mammedov
@ 2026-07-10 10:49         ` Christian Schoenebeck
  2026-07-10 12:51           ` Igor Mammedov
  0 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck @ 2026-07-10 10:49 UTC (permalink / raw)
  To: qemu-devel, Greg Kurz; +Cc: Jia Jia, Igor Mammedov

On Friday, 10 July 2026 12:23:45 CEST Igor Mammedov wrote:
> On Fri, 10 Jul 2026 10:06:29 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Friday, 10 July 2026 09:37:37 CEST Igor Mammedov wrote:
> > > On Thu, 9 Jul 2026 15:50:36 +0200
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > Harden security by disabling hotpluggable property, to prevent
> > > > issues like fixed in the previous commit.
> > > > 
> > > > Virtio-9p is a pass-through file-sharing device that provides a
> > > > guest mount of a host filesystem tree. Unlike block or network
> > > > devices, guest-triggered hotplug of the 9p device has no practical
> > > > use case: the guest cannot recover from the device removal, and there
> > > > is no protocol-level device-loss scenario as with block or ethernet
> > > > devices.
> > > 
> > > hmh, I'm no maintainer of 9pfs, but to me it looks like any other
> > > storage device.
> > > One should be able to unmount/stop using it and unplug
> > > (it doesn't really matter if unplug is triggered by guest or host side).
> > 
> > Guest could still unmount and stop using the 9pfs device. But why should
> > guest be able to unrealize the 9pfs device at any time? What should be
> > the purpose for this particular device?
> 
> unrealize is part of unplug process, at the end of unplug no device should
> be left attached. You might not need/use it but others might be using it
> actively.

I got that, but my question was: why would they want to do that with an 9pfs 
device? Can you make up any real-life scenario why a guest might want to 
remove a 9p device, given there is no way for them to bring the device back?

Looking at the fixed issue (patch 1), my impression was that original 9p 
server developers were unaware that guest can actually trigger a device 
unrealize via ACPI eject. Most probably because hotplugging is enabled by 
default for all devices in QEMU

Independent of this patch here, I'm therefore currently investigating whether 
enabling hotplugging by default in QEMU actually make sense. To me, this 
should be an opt-in, not the other way around. Because device developers 
should make sure that ejecting a device a) makes sense for the device type in 
the first place and most importantly b) that ejecting the device works 
(without data loss and without negative security impact, as it was the case 
here).

> ---
> 
> In QEMU impl. it's cooperative process (we don't do surprise removal of PCI
> devices), where host triggers plug/unplug process and guest side completes
> it, by: 1. freeing resources on its side 1st (hopefully)
>   2. issuing eject/poweroff (either via ACPI or native PCIE-HP/SHPC
> interface)
> 
> If guest fails to do #1 it is able to abort process, or it might continue
> and do #2 with a risk of loosing data but it's guest's business/policy.
> 
> If you need to disable hotplug for a specific PCI device instance,
> use 'hotplug' CLI option on pcie bridge. If you use other than PCI variant
> of virtio device, then unplug as you describe it isn't reachable, and if it
> should be disabled in code, it's up to actual frontend device that exposes
> underlying virtio one.

What does that have to do with this issue here? The normal 9pfs live-cycle is 
to create the 9p device at QEMU startup, and unrealizing the 9p device at QEMU 
shutdown.

If guest does an ACPI eject, then the only way to bring the 9p device back is 
to restart QEMU with the required -virtfs / -fsdev -device CLI options.

The Linux 9p client (guest) does never trigger an ACPI reject on unmount. It 
just marks the virtio channel as unused. If client would eject it, it would 
not be possible for guest to re-mount it later on.

/Christian






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

* Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
  2026-07-10 10:49         ` Christian Schoenebeck
@ 2026-07-10 12:51           ` Igor Mammedov
  2026-07-10 14:31             ` Christian Schoenebeck
  0 siblings, 1 reply; 13+ messages in thread
From: Igor Mammedov @ 2026-07-10 12:51 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel, Greg Kurz, Jia Jia

On Fri, 10 Jul 2026 12:49:06 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Friday, 10 July 2026 12:23:45 CEST Igor Mammedov wrote:
> > On Fri, 10 Jul 2026 10:06:29 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:  
> > > On Friday, 10 July 2026 09:37:37 CEST Igor Mammedov wrote:  
> > > > On Thu, 9 Jul 2026 15:50:36 +0200
> > > > 
> > > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:  
> > > > > Harden security by disabling hotpluggable property, to prevent
> > > > > issues like fixed in the previous commit.
> > > > > 
> > > > > Virtio-9p is a pass-through file-sharing device that provides a
> > > > > guest mount of a host filesystem tree. Unlike block or network
> > > > > devices, guest-triggered hotplug of the 9p device has no practical
> > > > > use case: the guest cannot recover from the device removal, and there
> > > > > is no protocol-level device-loss scenario as with block or ethernet
> > > > > devices.  
> > > > 
> > > > hmh, I'm no maintainer of 9pfs, but to me it looks like any other
> > > > storage device.
> > > > One should be able to unmount/stop using it and unplug
> > > > (it doesn't really matter if unplug is triggered by guest or host side).  
> > > 
> > > Guest could still unmount and stop using the 9pfs device. But why should
> > > guest be able to unrealize the 9pfs device at any time? What should be
> > > the purpose for this particular device?  
> > 
> > unrealize is part of unplug process, at the end of unplug no device should
> > be left attached. You might not need/use it but others might be using it
> > actively.  
> 
> I got that, but my question was: why would they want to do that with an 9pfs 
> device? Can you make up any real-life scenario why a guest might want to 
> remove a 9p device, given there is no way for them to bring the device back?

note: I'm looking from pov of hotpluggable PCI device and generic hotplug infra, only.

it's not guest users directly, it's how hotplug flow works for various guest OSes:

1. host plugs device in (-device or device_add)
2. guest OS get's notified one way or another and does what ever guest side
   init needed (incl. mounting share in 9pfs case)

opposite flow:
1. host does device_del (basically notify guest to remove device)
2. guest OS frees resources and tells qemu to delete device 
3. qemu process remove event (which incl. unrealize as part of destroying device)

Of cause guest if free to eject device without signal from host,
(I could imagine: get some file from share once and then guest releases
no longer need resource).
 
> Looking at the fixed issue (patch 1), my impression was that original 9p 
> server developers were unaware that guest can actually trigger a device 
> unrealize via ACPI eject.

I'd say it's a bug, and you are trying to fix it in patch #1

> Most probably because hotplugging is enabled by 
> default for all devices in QEMU

it is on by default for PCI devices but also heavily depends on used
configuration (where/what is plugged).

> Independent of this patch here, I'm therefore currently investigating whether 
> enabling hotplugging by default in QEMU actually make sense. To me, this 
> should be an opt-in, not the other way around. Because device developers 
> should make sure that ejecting a device a) makes sense for the device type in 
> the first place and most importantly b) that ejecting the device works 
> (without data loss and without negative security impact, as it was the case 
> here).

well, hotplug is on by default for PCI devices. It is unlikely we would
flip default now.

It might make sense to disable hotplug in mmio frontend if such exists,
but then it's pointless as there is no way to trigger unplug for it
(both host or guest side)

> 
> > ---
> > 
> > In QEMU impl. it's cooperative process (we don't do surprise removal of PCI
> > devices), where host triggers plug/unplug process and guest side completes
> > it, by: 1. freeing resources on its side 1st (hopefully)
> >   2. issuing eject/poweroff (either via ACPI or native PCIE-HP/SHPC
> > interface)
> > 
> > If guest fails to do #1 it is able to abort process, or it might continue
> > and do #2 with a risk of loosing data but it's guest's business/policy.
> > 
> > If you need to disable hotplug for a specific PCI device instance,
> > use 'hotplug' CLI option on pcie bridge. If you use other than PCI variant
> > of virtio device, then unplug as you describe it isn't reachable, and if it
> > should be disabled in code, it's up to actual frontend device that exposes
> > underlying virtio one.  
> 
> What does that have to do with this issue here? The normal 9pfs live-cycle is 
> to create the 9p device at QEMU startup, and unrealizing the 9p device at QEMU 
> shutdown.
> 
> If guest does an ACPI eject, then the only way to bring the 9p device back is 
> to restart QEMU with the required -virtfs / -fsdev -device CLI options.

one can use device_add to put it back into running machine.

given mention of ACPI eject, 9pfs flavor actually used is PCI variant =>
all PCI device features apply incl. hot(un)plug.
If hotplug is not desirable for particular device instance, one can disable
it on CLI.

that said,
if 9pfs maintainer deem that it shouldn't be hotpluggable and it's
safe to remove capability, I'm fine with that too, just fix commit message
section wrt QMP removal and perhaps move the flag into front-end impl of
the device.

BTW: always describe a way to reproduce problem (cover letter and/or patch)
so that reviewer could see whole flow. Otherwise one has to guess what/how it gets
broken based on context.
 
> The Linux 9p client (guest) does never trigger an ACPI reject on unmount. It 
> just marks the virtio channel as unused. If client would eject it, it would 
> not be possible for guest to re-mount it later on.

no argument here

> /Christian
> 
> 
> 
> 



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

* Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
  2026-07-10 12:51           ` Igor Mammedov
@ 2026-07-10 14:31             ` Christian Schoenebeck
  2026-07-10 14:40               ` Daniel P. Berrangé
  0 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck @ 2026-07-10 14:31 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, Greg Kurz, Jia Jia

On Friday, 10 July 2026 14:51:49 CEST Igor Mammedov wrote:
> On Fri, 10 Jul 2026 12:49:06 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Friday, 10 July 2026 12:23:45 CEST Igor Mammedov wrote:
[...]
> note: I'm looking from pov of hotpluggable PCI device and generic hotplug
> infra, only.

That's okay, but so far I don't see the relevance for this particular 9p 
device.
 
> it's not guest users directly, it's how hotplug flow works for various guest
> OSes:
> 
> 1. host plugs device in (-device or device_add)
> 2. guest OS get's notified one way or another and does what ever guest side
>    init needed (incl. mounting share in 9pfs case)

That's not affected by this patch, right?

> opposite flow:
> 1. host does device_del (basically notify guest to remove device)
> 2. guest OS frees resources and tells qemu to delete device
> 3. qemu process remove event (which incl. unrealize as part of destroying
> device)

And that's not affected by this patch either, right?

> Of cause guest if free to eject device without signal from host,

OK, here is the point where we deviate: you are apparently seeing this from a 
purely theoretical PoV.

I am facing reality: for several years I'm the only person taking care about 
this piece of code at all (on a side channel, for free, next to my actual 
work). And for several months I get AI generated security reports thrown at 
me, where I have to a) filter legit ones, and b) fix those legit security 
issues.

For that reason, I am tightening security wherever I can, to prevent further 
flood.

So the question here is: are you concerned about a real-life issue being 
introduced by disabling hotplugging for 9pfs specifically?

> (I could imagine: get some file from share once and then guest releases
> no longer need resource).

It's a pass-through file system. Ejecting the device does not really free a 
noteworthy amount of resources.

> > Looking at the fixed issue (patch 1), my impression was that original 9p
> > server developers were unaware that guest can actually trigger a device
> > unrealize via ACPI eject.
> 
> I'd say it's a bug, and you are trying to fix it in patch #1
> 
> > Most probably because hotplugging is enabled by
> > default for all devices in QEMU
> 
> it is on by default for PCI devices but also heavily depends on used
> configuration (where/what is plugged).

Exactly! And I was trying to explain, that for this particular use case 
(9pfs), I don't see any real-live use-case for allowing a guest to eject the 
9p device at runtime.

So why allowing it? Just for fun?

> > Independent of this patch here, I'm therefore currently investigating
> > whether enabling hotplugging by default in QEMU actually make sense. To
> > me, this should be an opt-in, not the other way around. Because device
> > developers should make sure that ejecting a device a) makes sense for the
> > device type in the first place and most importantly b) that ejecting the
> > device works (without data loss and without negative security impact, as
> > it was the case here).
> 
> well, hotplug is on by default for PCI devices. It is unlikely we would
> flip default now.

I am investigating to flip the default value for good reasons: I made a quick 
scan on the huge list of devices in QEMU and found quite a number that are 
affected by a similar UAF issue on guest ACPI eject. What do those devices 
have in common? Hotplug is enabled by default, not explicitly by the 
respective device.

> that said, if 9pfs maintainer deem that it shouldn't be hotpluggable and 
> it's safe to remove capability,

That's me, unfortunately. Singular.

> I'm fine with that too, just fix commit message
> section wrt QMP removal and perhaps move the flag into front-end impl of
> the device.

Yeah, I just noticed that as well, that this should rather go to the PCI 
bridge device. This patch applied it to the inner device, which is wrong.

> BTW: always describe a way to reproduce problem (cover letter and/or patch)
> so that reviewer could see whole flow. Otherwise one has to guess what/how
> it gets broken based on context.

The original report (private) is linked by patch 1:
https://gitlab.com/qemu-project/qemu/-/work_items/3937

Anyway, I'll think some more days about this, whether I should fix or drop the 
patch.

If you have more feedback, always appreciated of course.

Thanks!

/Christian





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

* Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
  2026-07-10 14:31             ` Christian Schoenebeck
@ 2026-07-10 14:40               ` Daniel P. Berrangé
  2026-07-13  7:49                 ` Igor Mammedov
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrangé @ 2026-07-10 14:40 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: Igor Mammedov, qemu-devel, Greg Kurz, Jia Jia

On Fri, Jul 10, 2026 at 04:31:51PM +0200, Christian Schoenebeck wrote:
> On Friday, 10 July 2026 14:51:49 CEST Igor Mammedov wrote:
> > On Fri, 10 Jul 2026 12:49:06 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > On Friday, 10 July 2026 12:23:45 CEST Igor Mammedov wrote:
> [...]
> > note: I'm looking from pov of hotpluggable PCI device and generic hotplug
> > infra, only.
> 
> That's okay, but so far I don't see the relevance for this particular 9p 
> device.
>  
> > it's not guest users directly, it's how hotplug flow works for various guest
> > OSes:
> > 
> > 1. host plugs device in (-device or device_add)
> > 2. guest OS get's notified one way or another and does what ever guest side
> >    init needed (incl. mounting share in 9pfs case)
> 
> That's not affected by this patch, right?
> 
> > opposite flow:
> > 1. host does device_del (basically notify guest to remove device)
> > 2. guest OS frees resources and tells qemu to delete device
> > 3. qemu process remove event (which incl. unrealize as part of destroying
> > device)
> 
> And that's not affected by this patch either, right?
> 
> > Of cause guest if free to eject device without signal from host,
> 
> OK, here is the point where we deviate: you are apparently seeing this from a 
> purely theoretical PoV.
> 
> I am facing reality: for several years I'm the only person taking care about 
> this piece of code at all (on a side channel, for free, next to my actual 
> work). And for several months I get AI generated security reports thrown at 
> me, where I have to a) filter legit ones, and b) fix those legit security 
> issues.
> 
> For that reason, I am tightening security wherever I can, to prevent further 
> flood.
> 
> So the question here is: are you concerned about a real-life issue being 
> introduced by disabling hotplugging for 9pfs specifically?
> 
> > (I could imagine: get some file from share once and then guest releases
> > no longer need resource).
> 
> It's a pass-through file system. Ejecting the device does not really free a 
> noteworthy amount of resources.
> 
> > > Looking at the fixed issue (patch 1), my impression was that original 9p
> > > server developers were unaware that guest can actually trigger a device
> > > unrealize via ACPI eject.
> > 
> > I'd say it's a bug, and you are trying to fix it in patch #1
> > 
> > > Most probably because hotplugging is enabled by
> > > default for all devices in QEMU
> > 
> > it is on by default for PCI devices but also heavily depends on used
> > configuration (where/what is plugged).
> 
> Exactly! And I was trying to explain, that for this particular use case 
> (9pfs), I don't see any real-live use-case for allowing a guest to eject the 
> 9p device at runtime.
> 
> So why allowing it? Just for fun?

I don't think it would be usual for a guest to unilaterally "eject"
a 9p device. A host initiated PCI hot-unplug request from QEMU
would ultimately trigger the guest eject code path though. IMHO
host initiated hotplug+unplug is a valid use for any device. For
9p the use is to provide a time limited data share to some workload
in the guest. There are many other ways that can be done too,
USB MTP, or virtiofs, or in guest with SFTP or WebDAV, etc. 


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



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

* Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
  2026-07-10 14:40               ` Daniel P. Berrangé
@ 2026-07-13  7:49                 ` Igor Mammedov
  0 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2026-07-13  7:49 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Christian Schoenebeck, qemu-devel, Greg Kurz, Jia Jia

On Fri, 10 Jul 2026 15:40:04 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Fri, Jul 10, 2026 at 04:31:51PM +0200, Christian Schoenebeck wrote:
> > On Friday, 10 July 2026 14:51:49 CEST Igor Mammedov wrote:  
> > > On Fri, 10 Jul 2026 12:49:06 +0200
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:  
> > > > On Friday, 10 July 2026 12:23:45 CEST Igor Mammedov wrote:  
> > [...]  
> > > note: I'm looking from pov of hotpluggable PCI device and generic hotplug
> > > infra, only.  
> > 
> > That's okay, but so far I don't see the relevance for this particular 9p 
> > device.
> >    
> > > it's not guest users directly, it's how hotplug flow works for various guest
> > > OSes:
> > > 
> > > 1. host plugs device in (-device or device_add)
> > > 2. guest OS get's notified one way or another and does what ever guest side
> > >    init needed (incl. mounting share in 9pfs case)  
> > 
> > That's not affected by this patch, right?
> >   
> > > opposite flow:
> > > 1. host does device_del (basically notify guest to remove device)
> > > 2. guest OS frees resources and tells qemu to delete device
> > > 3. qemu process remove event (which incl. unrealize as part of destroying
> > > device)  
> > 
> > And that's not affected by this patch either, right?

the patch would break unplug flow by effectively removing 'eject' knob 
from guest side, which is part of unplug flow.

> >   
> > > Of cause guest if free to eject device without signal from host,  
> > 
> > OK, here is the point where we deviate: you are apparently seeing this from a 
> > purely theoretical PoV.
> > 
> > I am facing reality: for several years I'm the only person taking care about 
> > this piece of code at all (on a side channel, for free, next to my actual 
> > work). And for several months I get AI generated security reports thrown at 
> > me, where I have to a) filter legit ones, and b) fix those legit security 
> > issues.
> > 
> > For that reason, I am tightening security wherever I can, to prevent further 
> > flood.
> > 
> > So the question here is: are you concerned about a real-life issue being 
> > introduced by disabling hotplugging for 9pfs specifically?

my concern is that there might be users that use unplug despite present bug(s),
and the patch would regress their usecase.

If unplug 'works', then we shouldn't disable it.
If it's never worked to begin with, it should be safe to disable it.

> > > (I could imagine: get some file from share once and then guest releases
> > > no longer need resource).  
> > 
> > It's a pass-through file system. Ejecting the device does not really free a 
> > noteworthy amount of resources.
> >
> > > > Looking at the fixed issue (patch 1), my impression was that original 9p
> > > > server developers were unaware that guest can actually trigger a device
> > > > unrealize via ACPI eject.  
> > > 
> > > I'd say it's a bug, and you are trying to fix it in patch #1
> > >   
> > > > Most probably because hotplugging is enabled by
> > > > default for all devices in QEMU  
> > > 
> > > it is on by default for PCI devices but also heavily depends on used
> > > configuration (where/what is plugged).  
> > 
> > Exactly! And I was trying to explain, that for this particular use case 
> > (9pfs), I don't see any real-live use-case for allowing a guest to eject the 
> > 9p device at runtime.
> > 
> > So why allowing it? Just for fun?  
> 
> I don't think it would be usual for a guest to unilaterally "eject"
> a 9p device. A host initiated PCI hot-unplug request from QEMU
> would ultimately trigger the guest eject code path though. IMHO
> host initiated hotplug+unplug is a valid use for any device. For
> 9p the use is to provide a time limited data share to some workload
> in the guest. There are many other ways that can be done too,
> USB MTP, or virtiofs, or in guest with SFTP or WebDAV, etc. 
> 
> 
> With regards,
> Daniel



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

end of thread, other threads:[~2026-07-13  7:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1783604079.git.qemu_oss@crudebyte.com>
2026-07-09 13:50 ` [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device Christian Schoenebeck
2026-07-10  7:37   ` Igor Mammedov
2026-07-10  8:06     ` Christian Schoenebeck
2026-07-10 10:23       ` Igor Mammedov
2026-07-10 10:49         ` Christian Schoenebeck
2026-07-10 12:51           ` Igor Mammedov
2026-07-10 14:31             ` Christian Schoenebeck
2026-07-10 14:40               ` Daniel P. Berrangé
2026-07-13  7:49                 ` Igor Mammedov
2026-07-09 13:50 ` [PATCH 3/3] hw/9pfs/xen: drain in-flight PDUs before xen-9p disconnect Christian Schoenebeck
2026-07-09 13:50 ` [PATCH 1/3] hw/9pfs/virtio: drain in-flight PDUs before virtio-9p unrealize Christian Schoenebeck
2026-07-09 14:20   ` Christian Schoenebeck
2026-07-10  1:39     ` m'te'a physical

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.