diff mbox series

[PATCH-for-9.1,2/2] hw/sd/sdcard: Assert @data_offset is in range

Message ID 20240408141717.66154-3-philmd@linaro.org
State New
Headers show
Series hw/sd/sdcard: Avoid OOB in sd_read_byte() | expand

Commit Message

Philippe Mathieu-Daudé April 8, 2024, 2:17 p.m. UTC 
Prevent out-of-bound access with assertions.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Peter Maydell April 8, 2024, 2:36 p.m. UTC | #1 
On Mon, 8 Apr 2024 at 15:18, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Prevent out-of-bound access with assertions.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/sd/sd.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 16d8d52a78..c081211582 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1875,6 +1875,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
>                              sd->current_cmd, value);
>      switch (sd->current_cmd) {
>      case 24:  /* CMD24:  WRITE_SINGLE_BLOCK */
> +        assert(sd->data_offset < sizeof(sd->data));
>          sd->data[sd->data_offset ++] = value;

Abstract out functions

static void append_sd_data_byte(SDState *sd, uint8_t value)
{
    assert(sd->data_offset < sizeof(sd->data));
    sd->data[sd->data_offset++] = value;
}

static void read_sd_data_byte(SDState *sd, uint8_t value)
{
    assert(sd->data_offset < sizeof(sd->sd_data));
    return sd->data[sd->data_offset++];
}

(etc for read_sd_status_byte() etc) ?

(sadly I don't think there's a verb that is the equivalent
of "prepend/append" but for removing elements.)


>      case 22:  /* ACMD22: SEND_NUM_WR_BLOCKS */
> +        assert(sd->data_offset < sizeof(sd->sd_status));
>          ret = sd->data[sd->data_offset ++];

Checking against the size of a different array from
the one we're reading from.

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 16d8d52a78..c081211582 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1875,6 +1875,7 @@  void sd_write_byte(SDState *sd, uint8_t value)
                             sd->current_cmd, value);
     switch (sd->current_cmd) {
     case 24:  /* CMD24:  WRITE_SINGLE_BLOCK */
+        assert(sd->data_offset < sizeof(sd->data));
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
@@ -1901,6 +1902,7 @@  void sd_write_byte(SDState *sd, uint8_t value)
                 }
             }
         }
+        assert(sd->data_offset < sizeof(sd->data));
         sd->data[sd->data_offset++] = value;
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
@@ -1925,6 +1927,7 @@  void sd_write_byte(SDState *sd, uint8_t value)
         break;
 
     case 26:  /* CMD26:  PROGRAM_CID */
+        assert(sd->data_offset < sizeof(sd->data));
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sizeof(sd->cid)) {
             /* TODO: Check CRC before committing */
@@ -1944,6 +1947,7 @@  void sd_write_byte(SDState *sd, uint8_t value)
         break;
 
     case 27:  /* CMD27:  PROGRAM_CSD */
+        assert(sd->data_offset < sizeof(sd->data));
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sizeof(sd->csd)) {
             /* TODO: Check CRC before committing */
@@ -1968,6 +1972,7 @@  void sd_write_byte(SDState *sd, uint8_t value)
         break;
 
     case 42:  /* CMD42:  LOCK_UNLOCK */
+        assert(sd->data_offset < sizeof(sd->data));
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
@@ -1979,6 +1984,7 @@  void sd_write_byte(SDState *sd, uint8_t value)
         break;
 
     case 56:  /* CMD56:  GEN_CMD */
+        assert(sd->data_offset < sizeof(sd->data));
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
@@ -2046,6 +2052,7 @@  uint8_t sd_read_byte(SDState *sd)
         break;
 
     case 13:  /* ACMD13: SD_STATUS */
+        assert(sd->data_offset < sizeof(sd->sd_status));
         ret = sd->sd_status[sd->data_offset ++];
 
         if (sd->data_offset >= sizeof(sd->sd_status))
@@ -2055,6 +2062,7 @@  uint8_t sd_read_byte(SDState *sd)
     case 17:  /* CMD17:  READ_SINGLE_BLOCK */
         if (sd->data_offset == 0)
             BLK_READ_BLOCK(sd->data_start, io_len);
+        assert(sd->data_offset < sizeof(sd->data));
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= io_len)
@@ -2069,6 +2077,7 @@  uint8_t sd_read_byte(SDState *sd)
             }
             BLK_READ_BLOCK(sd->data_start, io_len);
         }
+        assert(sd->data_offset < sizeof(sd->data));
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= io_len) {
@@ -2089,10 +2098,12 @@  uint8_t sd_read_byte(SDState *sd)
         if (sd->data_offset >= SD_TUNING_BLOCK_SIZE - 1) {
             sd->state = sd_transfer_state;
         }
+        assert(sd->data_offset < sizeof(sd_tuning_block_pattern));
         ret = sd_tuning_block_pattern[sd->data_offset++];
         break;
 
     case 22:  /* ACMD22: SEND_NUM_WR_BLOCKS */
+        assert(sd->data_offset < sizeof(sd->sd_status));
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 4)
@@ -2100,6 +2111,7 @@  uint8_t sd_read_byte(SDState *sd)
         break;
 
     case 30:  /* CMD30:  SEND_WRITE_PROT */
+        assert(sd->data_offset < sizeof(sd->data));
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 4)
@@ -2107,6 +2119,7 @@  uint8_t sd_read_byte(SDState *sd)
         break;
 
     case 51:  /* ACMD51: SEND_SCR */
+        assert(sd->data_offset < sizeof(sd->scr));
         ret = sd->scr[sd->data_offset ++];
 
         if (sd->data_offset >= sizeof(sd->scr))
@@ -2116,6 +2129,7 @@  uint8_t sd_read_byte(SDState *sd)
     case 56:  /* CMD56:  GEN_CMD */
         if (sd->data_offset == 0)
             APP_READ_BLOCK(sd->data_start, sd->blk_len);
+        assert(sd->data_offset < sizeof(sd->data));
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= sd->blk_len)