Re: [PATCH v2] hw/display/vmware_vga: Don't allow guest to trigger long running loop in host
Thomas Huth <[email protected]> Thu, 23 Jul 2026 18:12:35 +0200
| Newsgroups | org.nongnu.qemu-trivial,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
On 23/07/2026 17.05, Michael Tokarev wrote: > On 7/23/26 15:44, 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]> >> --- >> 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