The cursor bounds checks compare (srcoff + N) against vram_size, but
both sides are uint32_t so the addition can wrap past UINT32_MAX when
srcoff underflows from the cur_hv_offs subtraction, causing the check
to be bypassed.

Rewrite the checks as (srcoff > vram_size - N) to avoid the
overflow-prone addition, matching the style already used in
ati_mm_read() and ati_mm_write().

Fixes: 2f1fbe6ee9b5 ("ati-vga: Make sure hardware cursor data is within vram")
Cc: qemu-stable@nongnu.org
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
Found while running local QEMU fuzz testing.

 hw/display/ati.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 88a5bbbf07..0489995d00 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -149,7 +149,7 @@ static void ati_cursor_define(ATIVGAState *s)
     /* FIXME handle cur_hv_offs correctly */
     srcoff = s->regs.cur_offset - (s->regs.cur_hv_offs >> 16) -
              (s->regs.cur_hv_offs & 0xffff) * 16;
-    if (srcoff + 64 * 16 > s->vga.vram_size) {
+    if (srcoff > s->vga.vram_size - 64 * 16) {
         return;
     }
     for (int i = 0; i < 64; i++, srcoff += 16) {
@@ -206,7 +206,7 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
     }
     /* FIXME handle cur_hv_offs correctly */
     srcoff = s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
-    if (srcoff + 16 > s->vga.vram_size) {
+    if (srcoff > s->vga.vram_size - 16) {
         return;
     }
     dp = &dp[vga->hw_cursor_x];
-- 
2.43.0

On Tue, 14 Apr 2026, Junjie Cao wrote:
> The cursor bounds checks compare (srcoff + N) against vram_size, but
> both sides are uint32_t so the addition can wrap past UINT32_MAX when
> srcoff underflows from the cur_hv_offs subtraction, causing the check
> to be bypassed.
>
> Rewrite the checks as (srcoff > vram_size - N) to avoid the
> overflow-prone addition, matching the style already used in
> ati_mm_read() and ati_mm_write().
>
> Fixes: 2f1fbe6ee9b5 ("ati-vga: Make sure hardware cursor data is within vram")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
> ---
> Found while running local QEMU fuzz testing.
>
> hw/display/ati.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index 88a5bbbf07..0489995d00 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -149,7 +149,7 @@ static void ati_cursor_define(ATIVGAState *s)
>     /* FIXME handle cur_hv_offs correctly */
>     srcoff = s->regs.cur_offset - (s->regs.cur_hv_offs >> 16) -
>              (s->regs.cur_hv_offs & 0xffff) * 16;
> -    if (srcoff + 64 * 16 > s->vga.vram_size) {
> +    if (srcoff > s->vga.vram_size - 64 * 16) {
>         return;
>     }
>     for (int i = 0; i < 64; i++, srcoff += 16) {
> @@ -206,7 +206,7 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
>     }
>     /* FIXME handle cur_hv_offs correctly */
>     srcoff = s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
> -    if (srcoff + 16 > s->vga.vram_size) {
> +    if (srcoff > s->vga.vram_size - 16) {
>         return;
>     }
>     dp = &dp[vga->hw_cursor_x];

Considering that actual cursor offset is in bits 0-26 this should not 
happen but bit 31 is the lock bit that erroneously counted into offset 
here. So additionally we should also mask that out using 
(s->regs.cur_offset & 0x07fffff0) in calculation of scroff. There's one 
more similar usage in ati_cursor_invalidate. Otherwise,

Reviewed-by; BALATON Zoltan <balaton@eik.bme.hu>

Regards,
BALATON Zoltan

Thanks for the quick review and the suggestion.

I'll send a v2 series quickly, also collecting the Reviewed-by along
the way (there was a semicolon in the tag, which might have kept it
from being automatically collected). Sorry for the churn, Philippe.