[Qemu-devel] [PATCH v2] cirrus: fix oob access in mode4and5 write functions

Gerd Hoffmann posted 1 patch 49 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171011084314.21752-1-kraxel@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
hw/display/cirrus_vga.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

[Qemu-devel] [PATCH v2] cirrus: fix oob access in mode4and5 write functions

Posted by Gerd Hoffmann 49 weeks ago
Move dst calculation into the loop, so we apply the mask on each
interation and will not overflow vga memory.

Cc: Prasad J Pandit <pjp@fedoraproject.org>
Reported-by: Niu Guoxiang <niuguoxiang@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index afc290ab91..077a8cb74f 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2038,15 +2038,14 @@ static void cirrus_mem_writeb_mode4and5_8bpp(CirrusVGAState * s,
     unsigned val = mem_value;
     uint8_t *dst;
 
-    dst = s->vga.vram_ptr + (offset &= s->cirrus_addr_mask);
     for (x = 0; x < 8; x++) {
+        dst = s->vga.vram_ptr + ((offset + x) & s->cirrus_addr_mask);
 	if (val & 0x80) {
 	    *dst = s->cirrus_shadow_gr1;
 	} else if (mode == 5) {
 	    *dst = s->cirrus_shadow_gr0;
 	}
 	val <<= 1;
-	dst++;
     }
     memory_region_set_dirty(&s->vga.vram, offset, 8);
 }
@@ -2060,8 +2059,8 @@ static void cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
     unsigned val = mem_value;
     uint8_t *dst;
 
-    dst = s->vga.vram_ptr + (offset &= s->cirrus_addr_mask);
     for (x = 0; x < 8; x++) {
+        dst = s->vga.vram_ptr + ((offset + 2 * x) & s->cirrus_addr_mask & ~1);
 	if (val & 0x80) {
 	    *dst = s->cirrus_shadow_gr1;
 	    *(dst + 1) = s->vga.gr[0x11];
@@ -2070,7 +2069,6 @@ static void cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
 	    *(dst + 1) = s->vga.gr[0x10];
 	}
 	val <<= 1;
-	dst += 2;
     }
     memory_region_set_dirty(&s->vga.vram, offset, 16);
 }
-- 
2.9.3


Re: [Qemu-devel] [PATCH v2] cirrus: fix oob access in mode4and5 write functions

Posted by P J P 49 weeks ago
+-- On Wed, 11 Oct 2017, Gerd Hoffmann wrote --+
| Move dst calculation into the loop, so we apply the mask on each

s/into the loop/inside for loop

| interation and will not overflow vga memory.

s/interation/iteration


| diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
| index afc290ab91..077a8cb74f 100644
| --- a/hw/display/cirrus_vga.c
| +++ b/hw/display/cirrus_vga.c
| @@ -2038,15 +2038,14 @@ static void cirrus_mem_writeb_mode4and5_8bpp(CirrusVGAState * s,
|      unsigned val = mem_value;
|      uint8_t *dst;
|  
| -    dst = s->vga.vram_ptr + (offset &= s->cirrus_addr_mask);
|      for (x = 0; x < 8; x++) {
| +        dst = s->vga.vram_ptr + ((offset + x) & s->cirrus_addr_mask);
|  	if (val & 0x80) {
|  	    *dst = s->cirrus_shadow_gr1;
|  	} else if (mode == 5) {
|  	    *dst = s->cirrus_shadow_gr0;
|  	}
|  	val <<= 1;
| -	dst++;
|      }
|      memory_region_set_dirty(&s->vga.vram, offset, 8);
|  }
| @@ -2060,8 +2059,8 @@ static void cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
|      unsigned val = mem_value;
|      uint8_t *dst;
|  
| -    dst = s->vga.vram_ptr + (offset &= s->cirrus_addr_mask);
|      for (x = 0; x < 8; x++) {
| +        dst = s->vga.vram_ptr + ((offset + 2 * x) & s->cirrus_addr_mask & ~1);
|  	if (val & 0x80) {
|  	    *dst = s->cirrus_shadow_gr1;
|  	    *(dst + 1) = s->vga.gr[0x11];
| @@ -2070,7 +2069,6 @@ static void cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
|  	    *(dst + 1) = s->vga.gr[0x10];
|  	}
|  	val <<= 1;
| -	dst += 2;
|      }
|      memory_region_set_dirty(&s->vga.vram, offset, 16);
|  }

Works to fix the issue.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F