Message ID | 90b2648461d57d384823c90fa700cdd81d0b7254.1589981990.git.balaton@eik.bme.hu (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Misc display/sm501 clean ups and fixes | expand |
On 5/20/20 3:39 PM, BALATON Zoltan wrote: > Some places already use qemu_log_mask() to log unimplemented features > or errors but some others have printf() then abort(). Convert these to > qemu_log_mask() and avoid aborting to prevent guests to easily cause > denial of service. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/display/sm501.c | 57 ++++++++++++++++++++++------------------------ > 1 file changed, 27 insertions(+), 30 deletions(-) > > diff --git a/hw/display/sm501.c b/hw/display/sm501.c > index acc692531a..bd3ccfe311 100644 > --- a/hw/display/sm501.c > +++ b/hw/display/sm501.c > @@ -727,8 +727,8 @@ static void sm501_2d_operation(SM501State *s) > int fb_len = get_width(s, crt) * get_height(s, crt) * get_bpp(s, crt); > > if (addressing != 0x0) { > - printf("%s: only XY addressing is supported.\n", __func__); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: only XY addressing is supported.\n"); > + return; > } > > if (rop_mode == 0) { > @@ -754,8 +754,8 @@ static void sm501_2d_operation(SM501State *s) > > if ((s->twoD_source_base & 0x08000000) || > (s->twoD_destination_base & 0x08000000)) { > - printf("%s: only local memory is supported.\n", __func__); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: only local memory is supported.\n"); > + return; > } > > switch (operation) { > @@ -823,9 +823,9 @@ static void sm501_2d_operation(SM501State *s) > break; > > default: > - printf("non-implemented SM501 2D operation. %d\n", operation); > - abort(); > - break; > + qemu_log_mask(LOG_UNIMP, "sm501: not implemented 2D operation: %d\n", > + operation); > + return; > } > > if (dst_base >= get_fb_addr(s, crt) && > @@ -892,9 +892,8 @@ static uint64_t sm501_system_config_read(void *opaque, hwaddr addr, > break; > > default: > - printf("sm501 system config : not implemented register read." > - " addr=%x\n", (int)addr); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: not implemented system config" > + "register read. addr=%" HWADDR_PRIx "\n", addr); > } > > return ret; > @@ -948,15 +947,15 @@ static void sm501_system_config_write(void *opaque, hwaddr addr, > break; > case SM501_ENDIAN_CONTROL: > if (value & 0x00000001) { > - printf("sm501 system config : big endian mode not implemented.\n"); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: system config big endian mode not" > + " implemented.\n"); > } > break; > > default: > - printf("sm501 system config : not implemented register write." > - " addr=%x, val=%x\n", (int)addr, (uint32_t)value); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: not implemented system config" > + "register write. addr=%" HWADDR_PRIx > + ", val=%" PRIx64 "\n", addr, value); > } > } > > @@ -1207,9 +1206,8 @@ static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr, > break; > > default: > - printf("sm501 disp ctrl : not implemented register read." > - " addr=%x\n", (int)addr); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: not implemented disp ctrl register " > + "read. addr=%" HWADDR_PRIx "\n", addr); > } > > return ret; > @@ -1345,9 +1343,9 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr addr, > break; > > default: > - printf("sm501 disp ctrl : not implemented register write." > - " addr=%x, val=%x\n", (int)addr, (unsigned)value); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: not implemented disp ctrl register " > + "write. addr=%" HWADDR_PRIx > + ", val=%" PRIx64 "\n", addr, value); > } > } > > @@ -1433,9 +1431,8 @@ static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr, > ret = 0; /* Should return interrupt status */ > break; > default: > - printf("sm501 disp ctrl : not implemented register read." > - " addr=%x\n", (int)addr); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: not implemented disp ctrl register " > + "read. addr=%" HWADDR_PRIx "\n", addr); > } > > return ret; > @@ -1520,9 +1517,9 @@ static void sm501_2d_engine_write(void *opaque, hwaddr addr, > /* ignored, writing 0 should clear interrupt status */ > break; > default: > - printf("sm501 2d engine : not implemented register write." > - " addr=%x, val=%x\n", (int)addr, (unsigned)value); > - abort(); > + qemu_log_mask(LOG_UNIMP, "sm501: not implemented 2d engine register " > + "write. addr=%" HWADDR_PRIx > + ", val=%" PRIx64 "\n", addr, value); > } > } > > @@ -1670,9 +1667,9 @@ static void sm501_update_display(void *opaque) > draw_line = draw_line32_funcs[dst_depth_index]; > break; > default: > - printf("sm501 update display : invalid control register value.\n"); > - abort(); > - break; > + qemu_log_mask(LOG_GUEST_ERROR, "sm501: update display" > + "invalid control register value.\n"); > + return; > } > > /* set up to draw hardware cursor */ > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff --git a/hw/display/sm501.c b/hw/display/sm501.c index acc692531a..bd3ccfe311 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -727,8 +727,8 @@ static void sm501_2d_operation(SM501State *s) int fb_len = get_width(s, crt) * get_height(s, crt) * get_bpp(s, crt); if (addressing != 0x0) { - printf("%s: only XY addressing is supported.\n", __func__); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: only XY addressing is supported.\n"); + return; } if (rop_mode == 0) { @@ -754,8 +754,8 @@ static void sm501_2d_operation(SM501State *s) if ((s->twoD_source_base & 0x08000000) || (s->twoD_destination_base & 0x08000000)) { - printf("%s: only local memory is supported.\n", __func__); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: only local memory is supported.\n"); + return; } switch (operation) { @@ -823,9 +823,9 @@ static void sm501_2d_operation(SM501State *s) break; default: - printf("non-implemented SM501 2D operation. %d\n", operation); - abort(); - break; + qemu_log_mask(LOG_UNIMP, "sm501: not implemented 2D operation: %d\n", + operation); + return; } if (dst_base >= get_fb_addr(s, crt) && @@ -892,9 +892,8 @@ static uint64_t sm501_system_config_read(void *opaque, hwaddr addr, break; default: - printf("sm501 system config : not implemented register read." - " addr=%x\n", (int)addr); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: not implemented system config" + "register read. addr=%" HWADDR_PRIx "\n", addr); } return ret; @@ -948,15 +947,15 @@ static void sm501_system_config_write(void *opaque, hwaddr addr, break; case SM501_ENDIAN_CONTROL: if (value & 0x00000001) { - printf("sm501 system config : big endian mode not implemented.\n"); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: system config big endian mode not" + " implemented.\n"); } break; default: - printf("sm501 system config : not implemented register write." - " addr=%x, val=%x\n", (int)addr, (uint32_t)value); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: not implemented system config" + "register write. addr=%" HWADDR_PRIx + ", val=%" PRIx64 "\n", addr, value); } } @@ -1207,9 +1206,8 @@ static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr, break; default: - printf("sm501 disp ctrl : not implemented register read." - " addr=%x\n", (int)addr); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: not implemented disp ctrl register " + "read. addr=%" HWADDR_PRIx "\n", addr); } return ret; @@ -1345,9 +1343,9 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr addr, break; default: - printf("sm501 disp ctrl : not implemented register write." - " addr=%x, val=%x\n", (int)addr, (unsigned)value); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: not implemented disp ctrl register " + "write. addr=%" HWADDR_PRIx + ", val=%" PRIx64 "\n", addr, value); } } @@ -1433,9 +1431,8 @@ static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr, ret = 0; /* Should return interrupt status */ break; default: - printf("sm501 disp ctrl : not implemented register read." - " addr=%x\n", (int)addr); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: not implemented disp ctrl register " + "read. addr=%" HWADDR_PRIx "\n", addr); } return ret; @@ -1520,9 +1517,9 @@ static void sm501_2d_engine_write(void *opaque, hwaddr addr, /* ignored, writing 0 should clear interrupt status */ break; default: - printf("sm501 2d engine : not implemented register write." - " addr=%x, val=%x\n", (int)addr, (unsigned)value); - abort(); + qemu_log_mask(LOG_UNIMP, "sm501: not implemented 2d engine register " + "write. addr=%" HWADDR_PRIx + ", val=%" PRIx64 "\n", addr, value); } } @@ -1670,9 +1667,9 @@ static void sm501_update_display(void *opaque) draw_line = draw_line32_funcs[dst_depth_index]; break; default: - printf("sm501 update display : invalid control register value.\n"); - abort(); - break; + qemu_log_mask(LOG_GUEST_ERROR, "sm501: update display" + "invalid control register value.\n"); + return; } /* set up to draw hardware cursor */
Some places already use qemu_log_mask() to log unimplemented features or errors but some others have printf() then abort(). Convert these to qemu_log_mask() and avoid aborting to prevent guests to easily cause denial of service. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/display/sm501.c | 57 ++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 30 deletions(-)