All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-devel@nongnu.org, Greg Kurz <groug@kaod.org>,
	Jia Jia <physicalmtea@gmail.com>
Subject: Re: [PATCH 2/3] hw/9pfs/virtio: disable hotpluggable property of virtio-9p device
Date: Fri, 10 Jul 2026 16:31:51 +0200	[thread overview]
Message-ID: <2055091.yKVeVyVuyW@weasel> (raw)
In-Reply-To: <20260710145149.6e077fdd@imammedo>

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





  reply	other threads:[~2026-07-10 14:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1783604079.git.qemu_oss@crudebyte.com>
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
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2055091.yKVeVyVuyW@weasel \
    --to=qemu_oss@crudebyte.com \
    --cc=groug@kaod.org \
    --cc=imammedo@redhat.com \
    --cc=physicalmtea@gmail.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.