| Message ID | 1486023789-28995-5-git-send-email-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/02/17 09:23, Gerd Hoffmann wrote: > From: Li Qiang <liqiang6-s@360.cn> > > When doing bitblt copy in backward mode, we should minus the > blt width first just like the adding in the forward mode. This > can avoid the oob access of the front of vga's vram. > > Signed-off-by: Li Qiang <liqiang6-s@360.cn> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Message-id: 1485938101-26602-1-git-send-email-kraxel@redhat.com > Message-id: 5887254f.863a240a.2c122.5500@mx.google.com > > { kraxel: with backward blits (negative pitch) addr is the topmost > address, so check it as-is against vram size ] > > Cc: qemu-stable@nongnu.org > Cc: P J P <ppandit@redhat.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Wolfgang Bumiller <w.bumiller@proxmox.com> > Fixes: d3532a0db02296e687711b8cdc7791924efccea0 (CVE-2014-8106) > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/display/cirrus_vga.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c > index 7db6409..16f27e8 100644 > --- a/hw/display/cirrus_vga.c > +++ b/hw/display/cirrus_vga.c > @@ -274,10 +274,9 @@ static bool blit_region_is_unsafe(struct CirrusVGAState *s, > { > if (pitch < 0) { > int64_t min = addr > - + ((int64_t)s->cirrus_blt_height-1) * pitch; > - int32_t max = addr > - + s->cirrus_blt_width; > - if (min < 0 || max > s->vga.vram_size) { > + + ((int64_t)s->cirrus_blt_height - 1) * pitch > + - s->cirrus_blt_width; > + if (min < -1 || addr >= s->vga.vram_size) { > return true; > } > } else { > My inner pedant wishes to observe that my review concerned the patch as modified by you, so for complete accuracy, my R-b should be at the bottom, under your S-o-b. Not sure if this merits a PULLv2, I just needed to "silence to voices". Thanks Laszlo
> > Signed-off-by: Li Qiang <liqiang6-s@360.cn> > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > Message-id: 1485938101-26602-1-git-send-email-kraxel@redhat.com > > Message-id: 5887254f.863a240a.2c122.5500@mx.google.com > > > > { kraxel: with backward blits (negative pitch) addr is the topmost > > address, so check it as-is against vram size ] > > > > Cc: qemu-stable@nongnu.org > > Cc: P J P <ppandit@redhat.com> > > Cc: Laszlo Ersek <lersek@redhat.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Wolfgang Bumiller <w.bumiller@proxmox.com> > > Fixes: d3532a0db02296e687711b8cdc7791924efccea0 (CVE-2014-8106) > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > My inner pedant wishes to observe that my review concerned the patch as > modified by you, so for complete accuracy, my R-b should be at the > bottom, under your S-o-b. > > Not sure if this merits a PULLv2, I just needed to "silence to voices". Oops. The message ids are kinda f*cked up too. Guess I shouldn't trust the patches tool too much when it comes to non-trivial patch flows. I'll respin the pull request. cheers, Gerd
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 7db6409..16f27e8 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -274,10 +274,9 @@ static bool blit_region_is_unsafe(struct CirrusVGAState *s, { if (pitch < 0) { int64_t min = addr - + ((int64_t)s->cirrus_blt_height-1) * pitch; - int32_t max = addr - + s->cirrus_blt_width; - if (min < 0 || max > s->vga.vram_size) { + + ((int64_t)s->cirrus_blt_height - 1) * pitch + - s->cirrus_blt_width; + if (min < -1 || addr >= s->vga.vram_size) { return true; } } else {