On Friday, 10 July 2026 14:51:49 CEST Igor Mammedov wrote:
> On Fri, 10 Jul 2026 12:49:06 +0200
> 
> Christian Schoenebeck <[email protected]> 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




Reply via email to