From: Tristan Madani <[email protected]> The VRAM bounds check in ati_2d_do_blt() computes (dst.y + dst.height) * dst_stride using signed 32-bit arithmetic. On Rage128, setup_2d_blt_ctx() multiplies dst_stride by bpp (up to 32), producing strides up to 524,256 bytes/row. With large dst.y + dst.height values, the signed int multiplication overflows to a negative value, causing the bounds check to evaluate false when it should reject the operation.
The subsequent blit loop uses unsigned arithmetic for the same index computation, producing large positive offsets that write past the VRAM allocation into host process memory. The same overflow affects the source bounds check in the SRCCOPY path, where (vis_src.y + vis_dst.height) * src_stride can also overflow when the guest sets large pitch values. Fix both by pre-computing the end offsets as uint64_t, which prevents the signed overflow while keeping the existing bounds check logic. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4646 Cc: [email protected] Signed-off-by: Tristan Madani <[email protected]> --- v2: - Actually use src_end for the source bounds check in the SRCCOPY path; v1 declared it but never used it (Marc-André Lureau) - Sent standalone; v1 was part of an 8-patch display series hw/display/ati_2d.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c index 7a4a115..cfa4947 100644 --- a/hw/display/ati_2d.c +++ b/hw/display/ati_2d.c @@ -157,6 +157,7 @@ static bool ati_2d_do_blt(const ATI2DCtx *ctx, uint8_t use_pixman) QemuRect vis_src, vis_dst; unsigned int x, y, i, j, bypp = ctx->bpp / 8; const uint8_t *vram_end = ctx->vga->vram_ptr + ctx->vga->vram_size; + uint64_t dst_end; if (!ctx->bpp) { qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n"); @@ -166,9 +167,11 @@ static bool ati_2d_do_blt(const ATI2DCtx *ctx, uint8_t use_pixman) qemu_log_mask(LOG_GUEST_ERROR, "Zero dest pitch\n"); return false; } + dst_end = (uint64_t)ctx->dst.x * bypp + + (uint64_t)(ctx->dst.y + ctx->dst.height) * ctx->dst_stride; if (ctx->dst.x > 0x3fff || ctx->dst.y > 0x3fff || - ctx->dst_bits >= vram_end - bypp || ctx->dst_bits + ctx->dst.x * bypp + - (ctx->dst.y + ctx->dst.height) * ctx->dst_stride >= vram_end - bypp) { + ctx->dst_bits >= vram_end - bypp || + ctx->dst_bits + dst_end >= vram_end - bypp) { qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n"); return false; } @@ -203,11 +206,12 @@ static bool ati_2d_do_blt(const ATI2DCtx *ctx, uint8_t use_pixman) qemu_log_mask(LOG_GUEST_ERROR, "Zero source pitch\n"); return false; } + uint64_t src_end = (uint64_t)vis_src.x * bypp + + (uint64_t)(vis_src.y + vis_dst.height) * ctx->src_stride; if (!ctx->host_data_active && (vis_src.x > 0x3fff || vis_src.y > 0x3fff || ctx->src_bits >= vram_end - bypp || - ctx->src_bits + vis_src.x * bypp + (vis_src.y + vis_dst.height) * - ctx->src_stride >= vram_end - bypp)) { + ctx->src_bits + src_end >= vram_end - bypp)) { qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n"); return false; } -- 2.47.3
