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

On 23/07/2026 14.27, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 23/7/26 14:00, Thomas Huth wrote:
>> From: Thomas Huth <[email protected]>
>>
>> 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 <[email protected]>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4026
>> Reported-by: Tristan Madani <[email protected]>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4076
>> Reported-by: Sunday Jiang
>> Signed-off-by: Thomas Huth <[email protected]>
>> ---
>>   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",
>