qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@redhat.com>
To: Thomas Huth <thuth@redhat.com>,
	qemu-s390x@nongnu.org,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Halil Pasic <pasic@linux.ibm.com>,
	Eric Farman <farman@linux.ibm.com>,
	Matthew Rosato <mjrosato@linux.ibm.com>
Cc: qemu-devel@nongnu.org, David Hildenbrand <david@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>
Subject: Re: [PATCH v3] hw/s390x: Fix a possible crash with passed-through virtio devices
Date: Wed, 19 Nov 2025 11:02:51 +0100	[thread overview]
Message-ID: <521156ea-43f4-47cf-9e77-2310ff206ec1@redhat.com> (raw)
In-Reply-To: <20251118174047.73103-1-thuth@redhat.com>

On 11/18/25 18:40, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> 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.
> 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. Like in the
> other branches that detect wrong settings, we return -EINVAL from the
> function, which will later be placed in GPR2 to inform the guest about
> the error.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   v3: Print the subchannel number to ease debugging
> 
>   hw/s390x/s390-hypercall.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/hw/s390x/s390-hypercall.c b/hw/s390x/s390-hypercall.c
> index ac1b08b2cd5..508dd97ca0d 100644
> --- a/hw/s390x/s390-hypercall.c
> +++ b/hw/s390x/s390-hypercall.c
> @@ -10,6 +10,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>   #include "cpu.h"
>   #include "hw/s390x/s390-virtio-ccw.h"
>   #include "hw/s390x/s390-hypercall.h"
> @@ -42,6 +43,19 @@ static int handle_virtio_ccw_notify(uint64_t subch_id, uint64_t data)
>       if (!sch || !css_subch_visible(sch)) {
>           return -EINVAL;
>       }
> +    if (sch->id.cu_type != VIRTIO_CCW_CU_TYPE) {
> +        /*
> +         * This might happen in nested setups: If the L1 host defined the
> +         * L2 guest with a virtio device (e.g. virtio-keyboard), and the
> +         * L2 guest passes this device through to the L3 guest, the L3 guest
> +         * might send virtio notifications to the QEMU in L2 for that device.
> +         * But since the QEMU in L2 defined this device as vfio-ccw, it's not
> +         * a VirtIODevice that we can handle here!
> +         */
> +        warn_report_once("Got virtio notification for unsupported device "
> +                         "on subchannel %02x.%1x.%04x!", cssid, ssid, schid);
> +        return -EINVAL;
> +    }
>   
>       vdev = virtio_ccw_get_vdev(sch);

While at it, it would be good to test 'vdev' and return -EINVAL as in
virtio_ccw_set_vqs().

In virtio-ccw.c, this needs some care I think :

    static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
    {
        ...
        VirtioCcwDevice *dev = sch->driver_data;
        VirtIODevice *vdev = virtio_ccw_get_vdev(sch);
        ...
        if (!dev) {                <-- vdev ?
           return -EINVAL;
        }


Thanks,

C.

  

>       if (vq_idx >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, vq_idx)) {



  parent reply	other threads:[~2025-11-19 10:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-18 17:40 [PATCH v3] hw/s390x: Fix a possible crash with passed-through virtio devices Thomas Huth
2025-11-18 21:45 ` Eric Farman
2025-11-19  7:33   ` Thomas Huth
2025-11-19 14:01     ` Eric Farman
2025-11-18 22:57 ` Halil Pasic
2025-11-19  8:58 ` Christian Borntraeger
2025-11-19  9:53 ` Cornelia Huck
2025-11-19 10:02 ` Cédric Le Goater [this message]
2025-11-19 10:21   ` Thomas Huth

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=521156ea-43f4-47cf-9e77-2310ff206ec1@redhat.com \
    --to=clg@redhat.com \
    --cc=borntraeger@linux.ibm.com \
    --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 \
    --cc=thuth@redhat.com \
    /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).