From: Marc-André Lureau <marcandre.lureau@redhat.com>

The qxl_phys2virt() call for guest_monitors_config only validates
sizeof(QXLMonitorsConfig), which covers the fixed header (count and
max_allowed) since commit 8efec0ef8bbc ("hw/display/qxl: Pass requested
buffer size to qxl_phys2virt()"), but not the flexible array member
heads[]. When count == 1, heads[0] is accessed without its memory being
validated, allowing a guest to cause an out-of-bounds read.

Include sizeof(QXLHead) in the size passed to qxl_phys2virt() so that
the first head entry is validated within the guest memory slot, preventing
guest-visible memory reading.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4027
Reported-by: Tristan @TristanInSec
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
 hw/display/qxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 74258afa582..3df796175c2 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -270,7 +270,7 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
     }
 
     cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config, MEMSLOT_GROUP_GUEST,
-                        sizeof(QXLMonitorsConfig));
+                        sizeof(QXLMonitorsConfig) + sizeof(QXLHead));
     if (cfg != NULL && cfg->count == 1) {
         qxl->guest_primary.resized = 1;
         qxl->guest_head0_width  = cfg->heads[0].width;
-- 
2.55.0

On 7/15/26 10:27, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The qxl_phys2virt() call for guest_monitors_config only validates
> sizeof(QXLMonitorsConfig), which covers the fixed header (count and
> max_allowed) since commit 8efec0ef8bbc ("hw/display/qxl: Pass requested
> buffer size to qxl_phys2virt()"), but not the flexible array member
> heads[]. When count == 1, heads[0] is accessed without its memory being
> validated, allowing a guest to cause an out-of-bounds read.
> 
> Include sizeof(QXLHead) in the size passed to qxl_phys2virt() so that
> the first head entry is validated within the guest memory slot, preventing
> guest-visible memory reading.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4027
> Reported-by: Tristan @TristanInSec
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>

I'm picking this one up for the stable qemu series.
Please let me know if I shouldn't.

It would be nice if changes which should be picked up,
had Cc: qemu-stable@ in the first place, to avoid this
extra round-trip.  I'm trying to keep it as simple as
possible, only asking to reply if I should not pick it
up, but it is more work for everyone still.

Thanks,

/mjt

>   hw/display/qxl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 74258afa582..3df796175c2 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -270,7 +270,7 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
>       }
>   
>       cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config, MEMSLOT_GROUP_GUEST,
> -                        sizeof(QXLMonitorsConfig));
> +                        sizeof(QXLMonitorsConfig) + sizeof(QXLHead));
>       if (cfg != NULL && cfg->count == 1) {
>           qxl->guest_primary.resized = 1;
>           qxl->guest_head0_width  = cfg->heads[0].width;

On 22/7/26 11:38, Michael Tokarev wrote:
> On 7/15/26 10:27, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> The qxl_phys2virt() call for guest_monitors_config only validates
>> sizeof(QXLMonitorsConfig), which covers the fixed header (count and
>> max_allowed) since commit 8efec0ef8bbc ("hw/display/qxl: Pass requested
>> buffer size to qxl_phys2virt()"), but not the flexible array member
>> heads[]. When count == 1, heads[0] is accessed without its memory being
>> validated, allowing a guest to cause an out-of-bounds read.
>>
>> Include sizeof(QXLHead) in the size passed to qxl_phys2virt() so that
>> the first head entry is validated within the guest memory slot, 
>> preventing
>> guest-visible memory reading.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4027
>> Reported-by: Tristan @TristanInSec
>> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> 
> I'm picking this one up for the stable qemu series.
> Please let me know if I shouldn't.
> 
> It would be nice if changes which should be picked up,
> had Cc: qemu-stable@ in the first place, to avoid this
> extra round-trip.  I'm trying to keep it as simple as
> possible, only asking to reply if I should not pick it
> up, but it is more work for everyone still.

Indeed this patch should had a 'Cc: qemu-stable@' tag.

Hi

On Wed, Jul 22, 2026 at 2:01 PM Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> On 22/7/26 11:38, Michael Tokarev wrote:
> > On 7/15/26 10:27, marcandre.lureau@redhat.com wrote:
> >> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>
> >> The qxl_phys2virt() call for guest_monitors_config only validates
> >> sizeof(QXLMonitorsConfig), which covers the fixed header (count and
> >> max_allowed) since commit 8efec0ef8bbc ("hw/display/qxl: Pass requested
> >> buffer size to qxl_phys2virt()"), but not the flexible array member
> >> heads[]. When count == 1, heads[0] is accessed without its memory being
> >> validated, allowing a guest to cause an out-of-bounds read.
> >>
> >> Include sizeof(QXLHead) in the size passed to qxl_phys2virt() so that
> >> the first head entry is validated within the guest memory slot,
> >> preventing
> >> guest-visible memory reading.
> >>
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4027
> >> Reported-by: Tristan @TristanInSec
> >> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> >
> > I'm picking this one up for the stable qemu series.
> > Please let me know if I shouldn't.
> >
> > It would be nice if changes which should be picked up,
> > had Cc: qemu-stable@ in the first place, to avoid this
> > extra round-trip.  I'm trying to keep it as simple as
> > possible, only asking to reply if I should not pick it
> > up, but it is more work for everyone still.
>
> Indeed this patch should had a 'Cc: qemu-stable@' tag.

It could. That said, properly testing fixes on older releases takes
additional time and effort, and I prefer not to add Cc: qemu-stable@
unless I've verified the fix against those versions myself. I
appreciate you taking care of the stable evaluation, but I prefer to
keep my focus on master. Imho, we should not rush backports before
they have received a little bit more testing, it could introduce
regressions - the opposite of what I'd expect from -stable...

On 7/22/26 13:09, Marc-André Lureau wrote:
> On Wed, Jul 22, 2026 at 2:01 PM Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> wrote:
>>
>> On 22/7/26 11:38, Michael Tokarev wrote:
...
>>> It would be nice if changes which should be picked up,
>>> had Cc: qemu-stable@ in the first place, to avoid this
>>> extra round-trip.  I'm trying to keep it as simple as
>>> possible, only asking to reply if I should not pick it
>>> up, but it is more work for everyone still.
>>
>> Indeed this patch should had a 'Cc: qemu-stable@' tag.
> 
> It could. That said, properly testing fixes on older releases takes
> additional time and effort, and I prefer not to add Cc: qemu-stable@
> unless I've verified the fix against those versions myself. I
> appreciate you taking care of the stable evaluation, but I prefer to
> keep my focus on master. Imho, we should not rush backports before
> they have received a little bit more testing, it could introduce
> regressions - the opposite of what I'd expect from -stable...

I understand your concerns well (hopefully anyway).  However, I don't
really expect you to test things on older branches or to change your
focus, - it's not about that at all.

It is about just *marking* things which you, as the author of changes,
*think* might be good to fix/have in stable branches, so it gets some
attention later.

We can arrange "delayed stable queue" of sorts, we can wait for the
changes to prove themselves in the master branch, we can even try
to test the suggested changes in the stable branches - sometimes I
can do that too.  But without having such attention in the first
place, none of this can happen, ever.

Right now I have to ask for every change which looks like a candidate,
whenever it is a candidate or not.  This makes more work for you too,
by reading my emails and by cursing me making so much noise :)

Besides, with stable releases being not every second day but with some
delay, most stuff in there do have some minimal testing in the master
branch first, at the very least.  It is just before the stable series
freeze things might look as rushing for stable.  Also, a few years of
the stable series done this way already, kind of proves we're doing a
good job here.  There was one regression in migration code which was
a tricky case by its own (it was broken either way), and one or two
more regressions fixed by a quick follow-up update, and that's all.
With so many changes in there, I think it's a rather good result.
Yes there's always risk to introduce a regression.  Not doing any
fixing, on the other hand, ensures we stay with our bugs forever -
that would be the most stable stability :)

So, to make it explicit: I'm not asking your for more work -
especially not the extra testing of the changes in stable branches.
I only want to reduce your own work by not requiring to read my
questions when I feel something is a good candidate.  Tagging the
change as "for-stable" does not mean it must be applied, it is
more about marking it as a to-do item, or some sort - for you
or for someone else.  Just not to lose it.

Thanks,

/mjt

On 15/7/26 09:27, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The qxl_phys2virt() call for guest_monitors_config only validates
> sizeof(QXLMonitorsConfig), which covers the fixed header (count and
> max_allowed) since commit 8efec0ef8bbc ("hw/display/qxl: Pass requested
> buffer size to qxl_phys2virt()"), but not the flexible array member
> heads[]. When count == 1, heads[0] is accessed without its memory being
> validated, allowing a guest to cause an out-of-bounds read.
> 
> Include sizeof(QXLHead) in the size passed to qxl_phys2virt() so that
> the first head entry is validated within the guest memory slot, preventing
> guest-visible memory reading.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4027
> Reported-by: Tristan @TristanInSec
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/display/qxl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Patch queued, thanks.

Hi

On Wed, Jul 15, 2026 at 11:28 AM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The qxl_phys2virt() call for guest_monitors_config only validates
> sizeof(QXLMonitorsConfig), which covers the fixed header (count and
> max_allowed) since commit 8efec0ef8bbc ("hw/display/qxl: Pass requested
> buffer size to qxl_phys2virt()"), but not the flexible array member
> heads[]. When count == 1, heads[0] is accessed without its memory being
> validated, allowing a guest to cause an out-of-bounds read.
>
> Include sizeof(QXLHead) in the size passed to qxl_phys2virt() so that
> the first head entry is validated within the guest memory slot, preventing
> guest-visible memory reading.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4027
> Reported-by: Tristan @TristanInSec
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>

we are receiving multiple reports of this issue, can someone review?

(why the code only cares about count == 1 here? to limit the area to
render. in case of multi-monitor, it's the client job to handle the
monitor-config areas - tbh I suspect QEMU -display doesn't handle that
well, but I didn't check)

> ---
>  hw/display/qxl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 74258afa582..3df796175c2 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -270,7 +270,7 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
>      }
>
>      cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config, MEMSLOT_GROUP_GUEST,
> -                        sizeof(QXLMonitorsConfig));
> +                        sizeof(QXLMonitorsConfig) + sizeof(QXLHead));
>      if (cfg != NULL && cfg->count == 1) {
>          qxl->guest_primary.resized = 1;
>          qxl->guest_head0_width  = cfg->heads[0].width;
> --
> 2.55.0
>
>

On 20/7/26 13:06, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 15, 2026 at 11:28 AM <marcandre.lureau@redhat.com> wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> The qxl_phys2virt() call for guest_monitors_config only validates
>> sizeof(QXLMonitorsConfig), which covers the fixed header (count and
>> max_allowed) since commit 8efec0ef8bbc ("hw/display/qxl: Pass requested
>> buffer size to qxl_phys2virt()"), but not the flexible array member
>> heads[]. When count == 1, heads[0] is accessed without its memory being
>> validated, allowing a guest to cause an out-of-bounds read.
>>
>> Include sizeof(QXLHead) in the size passed to qxl_phys2virt() so that
>> the first head entry is validated within the guest memory slot, preventing
>> guest-visible memory reading.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4027
>> Reported-by: Tristan @TristanInSec
>> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> 
> we are receiving multiple reports of this issue, can someone review?
> 
> (why the code only cares about count == 1 here? to limit the area to
> render. in case of multi-monitor, it's the client job to handle the
> monitor-config areas - tbh I suspect QEMU -display doesn't handle that
> well, but I didn't check)
> 
>> ---
>>   hw/display/qxl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>