[Qemu-devel] [PATCH v2 14/15] sd: Check for READ_MULTIPLE_BLOCK size limit violation first

[Andrey Smirnov <andrew.smirnov@gmail.com>]

Check for READ_MULTIPLE_BLOCK size limit violation first as opposed to
doing at the end of the command handler. Consider the following
scenario:

Emulated host driver is trying to read last byte of the last sector
via CMD18/ADMA, so what would happen is the following:

1. "ret" is filled with desired byte and "sd->data_offset" is
   incremented to 512 by

	    ret = sd->data[sd->data_offset ++];

2. sd->data_offset >= io_len becomes true, so

            sd->data_start += io_len;

   moves "sd->data_start" past valid data boundaries.

3. as a result "sd->data_start + io_len > sd->size" check becomes true
   and sd->card_status is marked with ADDRESS_ERROR, telling emulated
   host that the last CMD18 read failed, despite nothing bad/illegal
   happening.

To avoid having this false positive, move out-of-bounds check to
happen before BLK_READ_BLOCK(), so this way it will only trigger if
illegal read is truly about to happen.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Cc: yurovsky@gmail.com
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 hw/sd/sd.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ba47bff4db..ce4ef17be3 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1797,8 +1797,17 @@ uint8_t sd_read_data(SDState *sd)
         break;
 
     case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */
-        if (sd->data_offset == 0)
+        if (sd->data_offset == 0) {
+            if (sd->data_start + io_len > sd->size) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "%s: Trying to read past card's capacity\n",
+                              __func__);
+                sd->card_status |= ADDRESS_ERROR;
+                return 0x00;
+            }
+
             BLK_READ_BLOCK(sd->data_start, io_len);
+        }
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= io_len) {
@@ -1812,11 +1821,6 @@ uint8_t sd_read_data(SDState *sd)
                     break;
                 }
             }
-
-            if (sd->data_start + io_len > sd->size) {
-                sd->card_status |= ADDRESS_ERROR;
-                break;
-            }
         }
         break;
 
-- 
2.14.3