* [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
@ 2026-07-23 12:44 Thomas Huth
2026-07-23 13:54 ` Philippe Mathieu-Daudé
2026-07-23 15:05 ` Michael Tokarev
0 siblings, 2 replies; 8+ messages in thread
From: Thomas Huth @ 2026-07-23 12:44 UTC (permalink / raw)
To: qemu-devel, Dmitry Fleytman; +Cc: qemu-trivial, qemu-stable
From: Thomas Huth <thuth@redhat.com>
The code in the SVGA_CMD_DEFINE_ALPHA_CURSOR handler in vmsvga_fifo_run()
basically does:
x = vmsvga_fifo_read(s);
y = vmsvga_fifo_read(s);
args = x * y;
goto badcmd;
...
badcmd:
len -= args;
if (len < 0) {
goto rewind;
}
while (args--) {
vmsvga_fifo_read(s);
}
Thus by supplying huge values for x and y that overflow the result of
the multiplication, the guest can trigger a long-running loop here
that burns the host's CPU cycles.
Add some sanity checks so that this cannot happen anymore.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3782
Reported-by: Feifan Qian <bea1e@proton.me>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
Reported-by: Tristan Madani <tristan@talencesecurity.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
Reported-by: Sunday Jiang
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
v2: Use SVGA_MAX_WIDTH and SVGA_MAX_HEIGHT instead of an arbitrary value
hw/display/vmware_vga.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index f6f9edfd1d9..567806f0e47 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -737,6 +737,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
vmsvga_fifo_read(s);
x = vmsvga_fifo_read(s);
y = vmsvga_fifo_read(s);
+ if (x < 0 || x >= SVGA_MAX_WIDTH ||
+ y < 0 || y >= SVGA_MAX_HEIGHT) {
+ goto rewind;
+ }
args = x * y;
goto badcmd;
case SVGA_CMD_RECT_ROP_FILL:
@@ -776,7 +780,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
if (len < 0) {
goto rewind;
}
- while (args--) {
+ while (args-- > 0) {
vmsvga_fifo_read(s);
}
printf("%s: Unknown command 0x%02x in SVGA command FIFO\n",
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
2026-07-23 12:44 [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host Thomas Huth
@ 2026-07-23 13:54 ` Philippe Mathieu-Daudé
2026-07-23 15:16 ` Daniel P. Berrangé
2026-07-23 15:05 ` Michael Tokarev
1 sibling, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-07-23 13:54 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Dmitry Fleytman; +Cc: qemu-trivial, qemu-stable
On 23/7/26 14:44, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
>
> The code in the SVGA_CMD_DEFINE_ALPHA_CURSOR handler in vmsvga_fifo_run()
> basically does:
>
> x = vmsvga_fifo_read(s);
> y = vmsvga_fifo_read(s);
> args = x * y;
> goto badcmd;
> ...
> badcmd:
> len -= args;
> if (len < 0) {
> goto rewind;
> }
> while (args--) {
> vmsvga_fifo_read(s);
> }
>
> Thus by supplying huge values for x and y that overflow the result of
> the multiplication, the guest can trigger a long-running loop here
> that burns the host's CPU cycles.
>
> Add some sanity checks so that this cannot happen anymore.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3782
> Reported-by: Feifan Qian <bea1e@proton.me>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
> Reported-by: Tristan Madani <tristan@talencesecurity.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
> Reported-by: Sunday Jiang
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> v2: Use SVGA_MAX_WIDTH and SVGA_MAX_HEIGHT instead of an arbitrary value
>
> hw/display/vmware_vga.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index f6f9edfd1d9..567806f0e47 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -737,6 +737,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
> vmsvga_fifo_read(s);
> x = vmsvga_fifo_read(s);
> y = vmsvga_fifo_read(s);
> + if (x < 0 || x >= SVGA_MAX_WIDTH ||
> + y < 0 || y >= SVGA_MAX_HEIGHT) {
vmsvga_fifo_read() returns unsigned... otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> + goto rewind;
> + }
> args = x * y;
> goto badcmd;
> case SVGA_CMD_RECT_ROP_FILL:
> @@ -776,7 +780,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
> if (len < 0) {
> goto rewind;
> }
> - while (args--) {
> + while (args-- > 0) {
> vmsvga_fifo_read(s);
> }
> printf("%s: Unknown command 0x%02x in SVGA command FIFO\n",
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
2026-07-23 13:54 ` Philippe Mathieu-Daudé
@ 2026-07-23 15:16 ` Daniel P. Berrangé
2026-07-23 16:18 ` Thomas Huth
0 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2026-07-23 15:16 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Thomas Huth, qemu-devel, Dmitry Fleytman, qemu-trivial,
qemu-stable
On Thu, Jul 23, 2026 at 03:54:41PM +0200, Philippe Mathieu-Daudé wrote:
> On 23/7/26 14:44, Thomas Huth wrote:
> > From: Thomas Huth <thuth@redhat.com>
> >
> > The code in the SVGA_CMD_DEFINE_ALPHA_CURSOR handler in vmsvga_fifo_run()
> > basically does:
> >
> > x = vmsvga_fifo_read(s);
> > y = vmsvga_fifo_read(s);
> > args = x * y;
> > goto badcmd;
> > ...
> > badcmd:
> > len -= args;
> > if (len < 0) {
> > goto rewind;
> > }
> > while (args--) {
> > vmsvga_fifo_read(s);
> > }
> >
> > Thus by supplying huge values for x and y that overflow the result of
> > the multiplication, the guest can trigger a long-running loop here
> > that burns the host's CPU cycles.
> >
> > Add some sanity checks so that this cannot happen anymore.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3782
> > Reported-by: Feifan Qian <bea1e@proton.me>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
> > Reported-by: Tristan Madani <tristan@talencesecurity.com>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
> > Reported-by: Sunday Jiang
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> > v2: Use SVGA_MAX_WIDTH and SVGA_MAX_HEIGHT instead of an arbitrary value
> >
> > hw/display/vmware_vga.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> > index f6f9edfd1d9..567806f0e47 100644
> > --- a/hw/display/vmware_vga.c
> > +++ b/hw/display/vmware_vga.c
> > @@ -737,6 +737,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
> > vmsvga_fifo_read(s);
> > x = vmsvga_fifo_read(s);
> > y = vmsvga_fifo_read(s);
> > + if (x < 0 || x >= SVGA_MAX_WIDTH ||
> > + y < 0 || y >= SVGA_MAX_HEIGHT) {
>
> vmsvga_fifo_read() returns unsigned... otherwise:
But x & y are declared 'int', so at least on 32-bit builds
a large uint32_t value would wrap and become negative.
Although we dropped support for 32-bit platforms, IMHO it would
be better to use uint32_t for 'x' and 'y' too rather than
assuming the 'int' value won't be negative.
> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>
> > + goto rewind;
> > + }
> > args = x * y;
> > goto badcmd;
> > case SVGA_CMD_RECT_ROP_FILL:
> > @@ -776,7 +780,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
> > if (len < 0) {
> > goto rewind;
> > }
> > - while (args--) {
> > + while (args-- > 0) {
> > vmsvga_fifo_read(s);
> > }
> > printf("%s: Unknown command 0x%02x in SVGA command FIFO\n",
>
>
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
2026-07-23 15:16 ` Daniel P. Berrangé
@ 2026-07-23 16:18 ` Thomas Huth
2026-07-23 16:20 ` Daniel P. Berrangé
2026-07-24 6:26 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 8+ messages in thread
From: Thomas Huth @ 2026-07-23 16:18 UTC (permalink / raw)
To: Daniel P. Berrangé, Philippe Mathieu-Daudé
Cc: qemu-devel, Dmitry Fleytman, qemu-trivial, qemu-stable
On 23/07/2026 17.16, Daniel P. Berrangé wrote:
> On Thu, Jul 23, 2026 at 03:54:41PM +0200, Philippe Mathieu-Daudé wrote:
>> On 23/7/26 14:44, Thomas Huth wrote:
>>> From: Thomas Huth <thuth@redhat.com>
>>>
>>> The code in the SVGA_CMD_DEFINE_ALPHA_CURSOR handler in vmsvga_fifo_run()
>>> basically does:
>>>
>>> x = vmsvga_fifo_read(s);
>>> y = vmsvga_fifo_read(s);
>>> args = x * y;
>>> goto badcmd;
>>> ...
>>> badcmd:
>>> len -= args;
>>> if (len < 0) {
>>> goto rewind;
>>> }
>>> while (args--) {
>>> vmsvga_fifo_read(s);
>>> }
>>>
>>> Thus by supplying huge values for x and y that overflow the result of
>>> the multiplication, the guest can trigger a long-running loop here
>>> that burns the host's CPU cycles.
>>>
>>> Add some sanity checks so that this cannot happen anymore.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3782
>>> Reported-by: Feifan Qian <bea1e@proton.me>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
>>> Reported-by: Tristan Madani <tristan@talencesecurity.com>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
>>> Reported-by: Sunday Jiang
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> v2: Use SVGA_MAX_WIDTH and SVGA_MAX_HEIGHT instead of an arbitrary value
>>>
>>> hw/display/vmware_vga.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>>> index f6f9edfd1d9..567806f0e47 100644
>>> --- a/hw/display/vmware_vga.c
>>> +++ b/hw/display/vmware_vga.c
>>> @@ -737,6 +737,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
>>> vmsvga_fifo_read(s);
>>> x = vmsvga_fifo_read(s);
>>> y = vmsvga_fifo_read(s);
>>> + if (x < 0 || x >= SVGA_MAX_WIDTH ||
>>> + y < 0 || y >= SVGA_MAX_HEIGHT) {
>>
>> vmsvga_fifo_read() returns unsigned... otherwise:
>
> But x & y are declared 'int', so at least on 32-bit builds
> a large uint32_t value would wrap and become negative.
> Although we dropped support for 32-bit platforms, IMHO it would
> be better to use uint32_t for 'x' and 'y' too rather than
> assuming the 'int' value won't be negative.
x and y are used as signed int all over the place here ... so reworking that
goes way beyond fixing this problem. Could we please get this patch merged
first for 11.1, and if someone then still feels like reworking the code,
this could be done for 11.2 ?
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
2026-07-23 16:18 ` Thomas Huth
@ 2026-07-23 16:20 ` Daniel P. Berrangé
2026-07-24 6:26 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2026-07-23 16:20 UTC (permalink / raw)
To: Thomas Huth
Cc: Philippe Mathieu-Daudé, qemu-devel, Dmitry Fleytman,
qemu-trivial, qemu-stable
On Thu, Jul 23, 2026 at 06:18:28PM +0200, Thomas Huth wrote:
> On 23/07/2026 17.16, Daniel P. Berrangé wrote:
> > On Thu, Jul 23, 2026 at 03:54:41PM +0200, Philippe Mathieu-Daudé wrote:
> > > On 23/7/26 14:44, Thomas Huth wrote:
> > > > From: Thomas Huth <thuth@redhat.com>
> > > >
> > > > The code in the SVGA_CMD_DEFINE_ALPHA_CURSOR handler in vmsvga_fifo_run()
> > > > basically does:
> > > >
> > > > x = vmsvga_fifo_read(s);
> > > > y = vmsvga_fifo_read(s);
> > > > args = x * y;
> > > > goto badcmd;
> > > > ...
> > > > badcmd:
> > > > len -= args;
> > > > if (len < 0) {
> > > > goto rewind;
> > > > }
> > > > while (args--) {
> > > > vmsvga_fifo_read(s);
> > > > }
> > > >
> > > > Thus by supplying huge values for x and y that overflow the result of
> > > > the multiplication, the guest can trigger a long-running loop here
> > > > that burns the host's CPU cycles.
> > > >
> > > > Add some sanity checks so that this cannot happen anymore.
> > > >
> > > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3782
> > > > Reported-by: Feifan Qian <bea1e@proton.me>
> > > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
> > > > Reported-by: Tristan Madani <tristan@talencesecurity.com>
> > > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
> > > > Reported-by: Sunday Jiang
> > > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > > ---
> > > > v2: Use SVGA_MAX_WIDTH and SVGA_MAX_HEIGHT instead of an arbitrary value
> > > >
> > > > hw/display/vmware_vga.c | 6 +++++-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> > > > index f6f9edfd1d9..567806f0e47 100644
> > > > --- a/hw/display/vmware_vga.c
> > > > +++ b/hw/display/vmware_vga.c
> > > > @@ -737,6 +737,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
> > > > vmsvga_fifo_read(s);
> > > > x = vmsvga_fifo_read(s);
> > > > y = vmsvga_fifo_read(s);
> > > > + if (x < 0 || x >= SVGA_MAX_WIDTH ||
> > > > + y < 0 || y >= SVGA_MAX_HEIGHT) {
> > >
> > > vmsvga_fifo_read() returns unsigned... otherwise:
> >
> > But x & y are declared 'int', so at least on 32-bit builds
> > a large uint32_t value would wrap and become negative.
> > Although we dropped support for 32-bit platforms, IMHO it would
> > be better to use uint32_t for 'x' and 'y' too rather than
> > assuming the 'int' value won't be negative.
> x and y are used as signed int all over the place here ... so reworking that
> goes way beyond fixing this problem. Could we please get this patch merged
> first for 11.1, and if someone then still feels like reworking the code,
> this could be done for 11.2 ?
Ok, then the < 0 checks should be kept.
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
2026-07-23 16:18 ` Thomas Huth
2026-07-23 16:20 ` Daniel P. Berrangé
@ 2026-07-24 6:26 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-07-24 6:26 UTC (permalink / raw)
To: Thomas Huth, Daniel P. Berrangé
Cc: qemu-devel, Dmitry Fleytman, qemu-trivial, qemu-stable
On 23/7/26 18:18, Thomas Huth wrote:
> On 23/07/2026 17.16, Daniel P. Berrangé wrote:
>> On Thu, Jul 23, 2026 at 03:54:41PM +0200, Philippe Mathieu-Daudé wrote:
>>> On 23/7/26 14:44, Thomas Huth wrote:
>>>> From: Thomas Huth <thuth@redhat.com>
>>>>
>>>> The code in the SVGA_CMD_DEFINE_ALPHA_CURSOR handler in
>>>> vmsvga_fifo_run()
>>>> basically does:
>>>>
>>>> x = vmsvga_fifo_read(s);
>>>> y = vmsvga_fifo_read(s);
>>>> args = x * y;
>>>> goto badcmd;
>>>> ...
>>>> badcmd:
>>>> len -= args;
>>>> if (len < 0) {
>>>> goto rewind;
>>>> }
>>>> while (args--) {
>>>> vmsvga_fifo_read(s);
>>>> }
>>>>
>>>> Thus by supplying huge values for x and y that overflow the result of
>>>> the multiplication, the guest can trigger a long-running loop here
>>>> that burns the host's CPU cycles.
>>>>
>>>> Add some sanity checks so that this cannot happen anymore.
>>>>
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3782
>>>> Reported-by: Feifan Qian <bea1e@proton.me>
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
>>>> Reported-by: Tristan Madani <tristan@talencesecurity.com>
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
>>>> Reported-by: Sunday Jiang
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>> v2: Use SVGA_MAX_WIDTH and SVGA_MAX_HEIGHT instead of an
>>>> arbitrary value
>>>>
>>>> hw/display/vmware_vga.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>>>> index f6f9edfd1d9..567806f0e47 100644
>>>> --- a/hw/display/vmware_vga.c
>>>> +++ b/hw/display/vmware_vga.c
>>>> @@ -737,6 +737,10 @@ static void vmsvga_fifo_run(struct
>>>> vmsvga_state_s *s)
>>>> vmsvga_fifo_read(s);
>>>> x = vmsvga_fifo_read(s);
>>>> y = vmsvga_fifo_read(s);
>>>> + if (x < 0 || x >= SVGA_MAX_WIDTH ||
>>>> + y < 0 || y >= SVGA_MAX_HEIGHT) {
>>>
>>> vmsvga_fifo_read() returns unsigned... otherwise:
>>
>> But x & y are declared 'int', so at least on 32-bit builds
>> a large uint32_t value would wrap and become negative.
>> Although we dropped support for 32-bit platforms, IMHO it would
>> be better to use uint32_t for 'x' and 'y' too rather than
>> assuming the 'int' value won't be negative.
> x and y are used as signed int all over the place here ...
Ah I missed that, sorry for the troubles.
> so reworking
> that goes way beyond fixing this problem. Could we please get this patch
> merged first for 11.1, and if someone then still feels like reworking
> the code, this could be done for 11.2 ?
>
> Thomas
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
2026-07-23 12:44 [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host Thomas Huth
2026-07-23 13:54 ` Philippe Mathieu-Daudé
@ 2026-07-23 15:05 ` Michael Tokarev
2026-07-23 16:12 ` Thomas Huth
1 sibling, 1 reply; 8+ messages in thread
From: Michael Tokarev @ 2026-07-23 15:05 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Dmitry Fleytman; +Cc: qemu-trivial, qemu-stable
On 7/23/26 15:44, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
>
> The code in the SVGA_CMD_DEFINE_ALPHA_CURSOR handler in vmsvga_fifo_run()
> basically does:
>
> x = vmsvga_fifo_read(s);
> y = vmsvga_fifo_read(s);
> args = x * y;
> goto badcmd;
> ...
> badcmd:
> len -= args;
> if (len < 0) {
> goto rewind;
> }
> while (args--) {
> vmsvga_fifo_read(s);
> }
>
> Thus by supplying huge values for x and y that overflow the result of
> the multiplication, the guest can trigger a long-running loop here
> that burns the host's CPU cycles.
>
> Add some sanity checks so that this cannot happen anymore.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3782
> Reported-by: Feifan Qian <bea1e@proton.me>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
> Reported-by: Tristan Madani <tristan@talencesecurity.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
> Reported-by: Sunday Jiang
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> v2: Use SVGA_MAX_WIDTH and SVGA_MAX_HEIGHT instead of an arbitrary value
>
> hw/display/vmware_vga.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index f6f9edfd1d9..567806f0e47 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -737,6 +737,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
> vmsvga_fifo_read(s);
> x = vmsvga_fifo_read(s);
> y = vmsvga_fifo_read(s);
> + if (x < 0 || x >= SVGA_MAX_WIDTH ||
> + y < 0 || y >= SVGA_MAX_HEIGHT) {
Applied to the trivial-patches tree, dropping <0 conditions here.
Thanks,
/mjt
> + goto rewind;
> + }
> args = x * y;
> goto badcmd;
> case SVGA_CMD_RECT_ROP_FILL:
> @@ -776,7 +780,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
> if (len < 0) {
> goto rewind;
> }
> - while (args--) {
> + while (args-- > 0) {
> vmsvga_fifo_read(s);
> }
> printf("%s: Unknown command 0x%02x in SVGA command FIFO\n",
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
2026-07-23 15:05 ` Michael Tokarev
@ 2026-07-23 16:12 ` Thomas Huth
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Huth @ 2026-07-23 16:12 UTC (permalink / raw)
To: Michael Tokarev, qemu-devel, Dmitry Fleytman; +Cc: qemu-trivial, qemu-stable
On 23/07/2026 17.05, Michael Tokarev wrote:
> On 7/23/26 15:44, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> The code in the SVGA_CMD_DEFINE_ALPHA_CURSOR handler in vmsvga_fifo_run()
>> basically does:
>>
>> x = vmsvga_fifo_read(s);
>> y = vmsvga_fifo_read(s);
>> args = x * y;
>> goto badcmd;
>> ...
>> badcmd:
>> len -= args;
>> if (len < 0) {
>> goto rewind;
>> }
>> while (args--) {
>> vmsvga_fifo_read(s);
>> }
>>
>> Thus by supplying huge values for x and y that overflow the result of
>> the multiplication, the guest can trigger a long-running loop here
>> that burns the host's CPU cycles.
>>
>> Add some sanity checks so that this cannot happen anymore.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3782
>> Reported-by: Feifan Qian <bea1e@proton.me>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
>> Reported-by: Tristan Madani <tristan@talencesecurity.com>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
>> Reported-by: Sunday Jiang
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> v2: Use SVGA_MAX_WIDTH and SVGA_MAX_HEIGHT instead of an arbitrary value
>>
>> hw/display/vmware_vga.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>> index f6f9edfd1d9..567806f0e47 100644
>> --- a/hw/display/vmware_vga.c
>> +++ b/hw/display/vmware_vga.c
>> @@ -737,6 +737,10 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
>> vmsvga_fifo_read(s);
>> x = vmsvga_fifo_read(s);
>> y = vmsvga_fifo_read(s);
>> + if (x < 0 || x >= SVGA_MAX_WIDTH ||
>> + y < 0 || y >= SVGA_MAX_HEIGHT) {
>
> Applied to the trivial-patches tree, dropping <0 conditions here.
Please don't drop the "< 0" ! x and y are signed variables, so we should
better check this here.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-24 6:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-23 12:44 [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host Thomas Huth
2026-07-23 13:54 ` Philippe Mathieu-Daudé
2026-07-23 15:16 ` Daniel P. Berrangé
2026-07-23 16:18 ` Thomas Huth
2026-07-23 16:20 ` Daniel P. Berrangé
2026-07-24 6:26 ` Philippe Mathieu-Daudé
2026-07-23 15:05 ` Michael Tokarev
2026-07-23 16:12 ` Thomas Huth
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.