| Message ID | 1402392027-9164-5-git-send-email-arei.gonglei@huawei.com |
|---|---|
| State | New |
| Headers | show |
Hi, > 2097 if (cursor_visible && cursor_offset < size && cursor_offset >= 0) > (23) Event divide_by_zero: In expression "cursor_offset / width", > division by expression "width" which may be zero has undefined behavior. > - if (cursor_visible && cursor_offset < size && cursor_offset >= 0) > + if (cursor_visible && cursor_offset < size && cursor_offset > 0) > dpy_text_cursor(s->con, > TEXTMODE_X(cursor_offset), > TEXTMODE_Y(cursor_offset)); That doesn't fix the reported issue. It's "width" which Coverity thinks might be zero, not cursor_offset. And cursor_offset being zero is perfectly fine, happens when the cursor is in the upper left corner. I have no idea why Coverity thinks width can be zero there. Line 2047: width = (s->cr[VGA_CRTC_H_DISP] + 1); (where cr is uint8_t). Hmm, maybe for the wraparound case (i.e. s->cr[VGA_CRTC_H_DISP] == 0xff)? cheers, Gerd
Il 12/06/2014 12:43, Gerd Hoffmann ha scritto: > That doesn't fix the reported issue. It's "width" which Coverity thinks > might be zero, not cursor_offset. And cursor_offset being zero is > perfectly fine, happens when the cursor is in the upper left corner. > > I have no idea why Coverity thinks width can be zero there. Line 2047: > > width = (s->cr[VGA_CRTC_H_DISP] + 1); > > (where cr is uint8_t). Hmm, maybe for the wraparound case (i.e. > s->cr[VGA_CRTC_H_DISP] == 0xff)? Not even that, the result is 0x100, math is done on the "int" data type. In fact I don't even see this defect on scan.coverity.com. Paolo
> -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: Thursday, June 12, 2014 6:44 PM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; lcapitulino@redhat.com; > av1474@comtv.ru; stefanha@redhat.com; Luonengjun; Huangweidong (C) > Subject: Re: [PATCH v3 4/4] vga: Fix divide-by-zero in vga_update_text > > Hi, > > > 2097 if (cursor_visible && cursor_offset < size && cursor_offset >= 0) > > (23) Event divide_by_zero: In expression "cursor_offset / width", > > division by expression "width" which may be zero has undefined behavior. > > > - if (cursor_visible && cursor_offset < size && cursor_offset >= > 0) > > + if (cursor_visible && cursor_offset < size && cursor_offset > > 0) > > dpy_text_cursor(s->con, > > TEXTMODE_X(cursor_offset), > > TEXTMODE_Y(cursor_offset)); > > That doesn't fix the reported issue. It's "width" which Coverity thinks > might be zero, not cursor_offset. And cursor_offset being zero is > perfectly fine, happens when the cursor is in the upper left corner. > Yep, I'm sorry for this fault. > I have no idea why Coverity thinks width can be zero there. Line 2047: > > width = (s->cr[VGA_CRTC_H_DISP] + 1); > > (where cr is uint8_t). Hmm, maybe for the wraparound case (i.e. > s->cr[VGA_CRTC_H_DISP] == 0xff)? > > cheers, > Gerd > Best regards, -Gonglei
diff --git a/hw/display/vga.c b/hw/display/vga.c index 8cd6afe..3c1c6eb 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2094,7 +2094,7 @@ static void vga_update_text(void *opaque, console_ch_t *chardata) s->cr[VGA_CRTC_CURSOR_START] != s->cursor_start || s->cr[VGA_CRTC_CURSOR_END] != s->cursor_end || full_update) { cursor_visible = !(s->cr[VGA_CRTC_CURSOR_START] & 0x20); - if (cursor_visible && cursor_offset < size && cursor_offset >= 0) + if (cursor_visible && cursor_offset < size && cursor_offset > 0) dpy_text_cursor(s->con, TEXTMODE_X(cursor_offset), TEXTMODE_Y(cursor_offset));