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] --- hw/display/sm501.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/hw/display/sm501.c b/hw/display/sm501.c index 0da25477bcc..d5e165daefc 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -682,6 +682,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; @@ -731,9 +753,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; } @@ -760,9 +781,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.43.0
