[Qemu-devel] [PATCH for-2.12] hw/block/pflash_cfi: fix off-by-one error

[Qemu-devel] [PATCH for-2.12] hw/block/pflash_cfi: fix off-by-one error

[Philippe Mathieu-Daudé]

ASAN reported:

    hw/block/pflash_cfi02.c:245:33: runtime error: index 82 out of bounds for type 'uint8_t [82]'

Since the 'cfi_len' member is not used, remove it to keep the code safer.

Reported-by: AddressSanitizer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/block/pflash_cfi01.c | 10 ++++------
 hw/block/pflash_cfi02.c |  9 ++++-----
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 1113ab1ccf..2e8284001d 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -90,7 +90,6 @@ struct pflash_t {
     uint16_t ident1;
     uint16_t ident2;
     uint16_t ident3;
-    uint8_t cfi_len;
     uint8_t cfi_table[0x52];
     uint64_t counter;
     unsigned int writeblock_size;
@@ -153,7 +152,7 @@ static uint32_t pflash_cfi_query(pflash_t *pfl, hwaddr offset)
     boff = offset >> (ctz32(pfl->bank_width) +
                       ctz32(pfl->max_device_width) - ctz32(pfl->device_width));
 
-    if (boff > pfl->cfi_len) {
+    if (boff >= sizeof(pfl->cfi_table)) {
         return 0;
     }
     /* Now we will construct the CFI response generated by a single
@@ -385,10 +384,10 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
                 boff = boff >> 2;
             }
 
-            if (boff > pfl->cfi_len) {
-                ret = 0;
-            } else {
+            if (boff < sizeof(pfl->cfi_table)) {
                 ret = pfl->cfi_table[boff];
+            } else {
+                ret = 0;
             }
         } else {
             /* If we have a read larger than the bank_width, combine multiple
@@ -791,7 +790,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     pfl->cmd = 0;
     pfl->status = 0;
     /* Hardcoded CFI table */
-    pfl->cfi_len = 0x52;
     /* Standard "QRY" string */
     pfl->cfi_table[0x10] = 'Q';
     pfl->cfi_table[0x11] = 'R';
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index c81ddd3a99..75d1ae1026 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -83,7 +83,6 @@ struct pflash_t {
     uint16_t ident3;
     uint16_t unlock_addr0;
     uint16_t unlock_addr1;
-    uint8_t cfi_len;
     uint8_t cfi_table[0x52];
     QEMUTimer *timer;
     /* The device replicates the flash memory across its memory space.  Emulate
@@ -235,10 +234,11 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
         break;
     case 0x98:
         /* CFI query mode */
-        if (boff > pfl->cfi_len)
-            ret = 0;
-        else
+        if (boff < sizeof(pfl->cfi_table)) {
             ret = pfl->cfi_table[boff];
+        } else {
+            ret = 0;
+        }
         break;
     }
 
@@ -663,7 +663,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     pfl->cmd = 0;
     pfl->status = 0;
     /* Hardcoded CFI table (mostly from SG29 Spansion flash) */
-    pfl->cfi_len = 0x52;
     /* Standard "QRY" string */
     pfl->cfi_table[0x10] = 'Q';
     pfl->cfi_table[0x11] = 'R';
-- 
2.16.3

Re: [Qemu-devel] [PATCH for-2.12] hw/block/pflash_cfi: fix off-by-one error

[Kevin Wolf]

Am 05.04.2018 um 01:32 hat Philippe Mathieu-Daudé geschrieben:
> ASAN reported:
> 
>     hw/block/pflash_cfi02.c:245:33: runtime error: index 82 out of bounds for type 'uint8_t [82]'
> 
> Since the 'cfi_len' member is not used, remove it to keep the code safer.
> 
> Reported-by: AddressSanitizer
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Cc: qemu-stable@nongnu.org

Thanks, applied to the block branch.

Kevin