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

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.

> +                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",