From: Peter Maydell <[email protected]> When we check that a 2D rectangle operation isn't going to run off the end of video RAM, we do the calculations as 32 bit arithmetic. This means that carefully chosen guest register values can cause an overflow so we don't detect that the operation is going to go outside video memory.
Abstract the check out into a function, do the calculations as 64-bit arithmetic, and add assertions about the ranges of the inputs. Cc: [email protected] Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3584 Signed-off-by: Peter Maydell <[email protected]> Tested-by: BALATON Zoltan <[email protected]> Reviewed-by: BALATON Zoltan <[email protected]> Message-id: [email protected] (cherry picked from commit 422734d5fc2b5adec704b352082385511959d5a7) Signed-off-by: Michael Tokarev <[email protected]> diff --git a/hw/display/sm501.c b/hw/display/sm501.c index 13ccc4941df..d451a1fe7d7 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -681,6 +681,28 @@ static inline void hwc_invalidate(SM501State *s, int crt) get_fb_addr(s, crt) + start, end - start); } +static bool sm501_rect_outside_vram(SM501State *s, uint32_t base, + uint32_t x, uint32_t y, + uint32_t width, uint32_t height, + uint32_t pitch, uint32_t bypp) +{ + /* + * Return true if the 2D area specified by the arguments is + * partially or completely outside the VRAM (a guest error) + * + * Limits on the input sizes mean we can't overflow as long as + * we do all the arithmetic at 64 bits. + */ + uint64_t rect_size, last_addr; + + assert(x <= UINT16_MAX && y <= UINT16_MAX && height <= UINT16_MAX && + pitch <= UINT16_MAX && bypp <= 8); + rect_size = (((uint64_t)y + height) * pitch + x + width) * bypp; + last_addr = base + rect_size; + + return last_addr >= get_local_mem_size(s); +} + static void sm501_2d_operation(SM501State *s) { int cmd = (s->twoD_control >> 16) & 0x1F; @@ -730,9 +752,8 @@ static void sm501_2d_operation(SM501State *s) dst_y -= height - 1; } - if (dst_base >= get_local_mem_size(s) || - dst_base + (dst_x + width + (dst_y + height) * dst_pitch) * bypp >= - get_local_mem_size(s)) { + if (sm501_rect_outside_vram(s, dst_base, dst_x, dst_y, width, height, + dst_pitch, bypp)) { qemu_log_mask(LOG_GUEST_ERROR, "sm501: 2D op dest is outside vram.\n"); return; } @@ -759,9 +780,8 @@ static void sm501_2d_operation(SM501State *s) src_y -= height - 1; } - if (src_base >= get_local_mem_size(s) || - src_base + (src_x + width + (src_y + height) * src_pitch) * bypp >= - get_local_mem_size(s)) { + if (sm501_rect_outside_vram(s, src_base, src_x, src_y, width, height, + src_pitch, bypp)) { qemu_log_mask(LOG_GUEST_ERROR, "sm501: 2D op src is outside vram.\n"); return; -- 2.47.3
