qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: Halil Pasic <pasic@linux.ibm.com>
Cc: qemu-s390x@nongnu.org,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Eric Farman" <farman@linux.ibm.com>,
	"Matthew Rosato" <mjrosato@linux.ibm.com>,
	qemu-devel@nongnu.org, "David Hildenbrand" <david@redhat.com>,
	"Cédric Le Goater" <clg@kaod.org>,
	"Cornelia Huck" <cohuck@redhat.com>
Subject: Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices
Date: Tue, 18 Nov 2025 13:28:19 +0100	[thread overview]
Message-ID: <50f79156-dd93-40c4-831e-66e558531be8@redhat.com> (raw)
In-Reply-To: <20251118130218.30d3da33.pasic@linux.ibm.com>

On 18/11/2025 13.02, Halil Pasic wrote:
> On Tue, 18 Nov 2025 10:39:45 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> Consider the following nested setup: An L1 host uses some virtio device
>> (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
>> device through to the L3 guest. Since the L3 guest sees a virtio device,
>> it might send virtio notifications to the QEMU in L2 for that device.
> 
> Hm, but conceptually the notification is sent to the virtio device,
> regardless of hypervisors, right? But because for virtio-ccw the
> notification is an DIAG 500, we have the usual cascade of intercept
> handling. And because we have never considered this scenario up till now
> the intercept handler in L2 QEMU gets called, because it is usually the
> responsibility of L2 QEMU to emulate instructions for an L3 guest.

Right.

> I think vfio-ccw pass through was supposed to be only about DASD.

Yes. And we only noticed this bug by accident - while trying to pass through 
a DASD device, the wrong device was used for VFIO and suddenly QEMU crashed.

>> But since the QEMU in L2 defined this device as vfio-ccw, the function
>> handle_virtio_ccw_notify() cannot handle this and crashes: It calls
>> virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
>> but since "sch" belongs to a vfio-ccw device, that driver_data rather
>> points to a CcwDevice instead. So as soon as QEMU tries to use some
>> VirtioCcwDevice specific data from that device, we've lost.
>>
>> We must not take virtio notifications for such devices. Thus fix the
>> issue by adding a check to the handle_virtio_ccw_notify() handler to
>> refuse all devices that are not our own virtio devices.
> 
> I'm on board with this patch! Virtio notifications are only supported
> for virtio devices and if a guest for what ever reason attempts
> to do a virtio notification on a non-virtio device, that should be
> handled accordingly. Which would be some sort of a program exception
> I guess. Maybe you could add what kind of exception do we end up
> with to the commit message. I would guess specification exception.
> 
> But I would argue that the L3 guest didn't do anything wrong.

That's the point - the L3 guest just sees a virtio device, so we should not 
punish it with program exceptions just because it tried to send a 
notification for the device.

> Pass-through of virtio-ccw devices is simply not implemented yet
> properly. And even  if we were to swallow that notification silently,
> it would be effectively loss of initiative I guess.

I think the current patch does the right thing: It returns an error value to 
the guest (just like we're doing it in other spots in this function 
already), so the guest sees that error value and then can finally give up on 
using the device.

> So I think it would really make sense to prevent passing through
> virtio-ccw devices with vfio-ccw.

That could be a nice addition on top (in another patch), but we have to fix 
handle_virtio_ccw_notify() anyway to avoid that the L3 guest can crash QEMU, 
so it's certainly not a replacement for this patch, I think.

  Thomas



  reply	other threads:[~2025-11-18 12:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-18  9:39 [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices Thomas Huth
2025-11-18 11:52 ` Cornelia Huck
2025-11-18 12:09   ` Thomas Huth
2025-11-18 12:15     ` Cornelia Huck
2025-11-18 12:02 ` Halil Pasic
2025-11-18 12:28   ` Thomas Huth [this message]
2025-11-18 14:24     ` Halil Pasic
2025-11-18 14:53       ` Cornelia Huck
2025-11-18 14:25     ` Cornelia Huck
2025-11-18 14:48       ` Thomas Huth
2025-11-18 15:19   ` Eric Farman
2025-11-18 22:56     ` Halil Pasic

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=50f79156-dd93-40c4-831e-66e558531be8@redhat.com \
    --to=thuth@redhat.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=clg@kaod.org \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=farman@linux.ibm.com \
    --cc=mjrosato@linux.ibm.com \
    --cc=pasic@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).