1. Patchew
  2. QEMU
  3. View series

[PATCH] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host

Thomas Huth posted 1 patch 1 day, 18 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260723120020.330089-1-thuth@redhat.com
Maintainers: Dmitry Fleytman <dmitry.fleytman@gmail.com>
There is a newer version of this series
hw/display/vmware_vga.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
Posted by Thomas Huth 1 day, 18 hours ago 
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>
---
 hw/display/vmware_vga.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index f6f9edfd1d9..04cc438cc02 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -737,6 +737,9 @@ 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 >= 32768 || y < 0 || y >= 32768) {
+                goto rewind;
+            }
             args = x * y;
             goto badcmd;
         case SVGA_CMD_RECT_ROP_FILL:
@@ -776,7 +779,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
Re: [PATCH] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
Posted by Philippe Mathieu-Daudé 1 day, 17 hours ago 
Hi Thomas,

On 23/7/26 14:00, 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>
> ---
>   hw/display/vmware_vga.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index f6f9edfd1d9..04cc438cc02 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -737,6 +737,9 @@ 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 >= 32768 || y < 0 || y >= 32768) {

Maybe add a self-decribing definition instead of magic number?

   #define ALPHA_CURSOR_SAFE_LIMIT 32768 /* Arbitrary limit */

Otherwise LGTM.

> +                goto rewind;
> +            }
>               args = x * y;
>               goto badcmd;
>           case SVGA_CMD_RECT_ROP_FILL:
> @@ -776,7 +779,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",
Re: [PATCH] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
Posted by Thomas Huth 1 day, 17 hours ago 
On 23/07/2026 14.27, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 23/7/26 14:00, 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>
>> ---
>>   hw/display/vmware_vga.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>> index f6f9edfd1d9..04cc438cc02 100644
>> --- a/hw/display/vmware_vga.c
>> +++ b/hw/display/vmware_vga.c
>> @@ -737,6 +737,9 @@ 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 >= 32768 || y < 0 || y >= 32768) {
> 
> Maybe add a self-decribing definition instead of magic number?
> 
>    #define ALPHA_CURSOR_SAFE_LIMIT 32768 /* Arbitrary limit */
> 
> Otherwise LGTM.

There is already this in the code:

#define SVGA_MAX_WIDTH                  2368
#define SVGA_MAX_HEIGHT                 1770

Shall I use those? WDYT?

  Thomas

>> +                goto rewind;
>> +            }
>>               args = x * y;
>>               goto badcmd;
>>           case SVGA_CMD_RECT_ROP_FILL:
>> @@ -776,7 +779,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",
>
Re: [PATCH] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
Posted by Philippe Mathieu-Daudé 1 day, 16 hours ago 
On 23/7/26 14:35, Thomas Huth wrote:
> On 23/07/2026 14.27, Philippe Mathieu-Daudé wrote:
>> Hi Thomas,
>>
>> On 23/7/26 14:00, 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>
>>> ---
>>>   hw/display/vmware_vga.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>>> index f6f9edfd1d9..04cc438cc02 100644
>>> --- a/hw/display/vmware_vga.c
>>> +++ b/hw/display/vmware_vga.c
>>> @@ -737,6 +737,9 @@ 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 >= 32768 || y < 0 || y >= 32768) {
>>
>> Maybe add a self-decribing definition instead of magic number?
>>
>>    #define ALPHA_CURSOR_SAFE_LIMIT 32768 /* Arbitrary limit */
>>
>> Otherwise LGTM.
> 
> There is already this in the code:
> 
> #define SVGA_MAX_WIDTH                  2368
> #define SVGA_MAX_HEIGHT                 1770
> 
> Shall I use those? WDYT?

Ah, these look like good candidates :)

> 
>   Thomas
> 
>>> +                goto rewind;
>>> +            }
>>>               args = x * y;
>>>               goto badcmd;
>>>           case SVGA_CMD_RECT_ROP_FILL:
>>> @@ -776,7 +779,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",
>>
> 
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.