Commit 790762e5 authored by Philippe Mathieu-Daudé's avatar Philippe Mathieu-Daudé
Browse files

hw/sd/sdcard: Do not switch to ReceivingData if address is invalid 



Only move the state machine to ReceivingData if there is no
pending error. This avoids later OOB access while processing
commands queued.

  "SD Specifications Part 1 Physical Layer Simplified Spec. v3.01"

  4.3.3 Data Read

  Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
  occurred and no data transfer is performed.

  4.3.4 Data Write

  Write command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
  occurred and no data transfer is performed.

WP_VIOLATION errors are not modified: the error bit is set, we
stay in receive-data state, wait for a stop command. All further
data transfer is ignored. See the check on sd->card_status at the
beginning of sd_read_data() and sd_write_data().

Fixes: CVE-2020-13253
Cc: qemu-stable@nongnu.org
Reported-by: default avatarAlexander Bulekov <alxndr@bu.edu>
Buglink: https://bugs.launchpad.net/qemu/+bug/1880822


Reviewed-by: default avatarPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: default avatarAlistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-6-f4bug@amsat.org>
parent 794d68de

hw/sd/sd.c

+24 −14
Original line number Diff line number Diff line
@@ -1171,13 +1171,15 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) 
    case 17:	/* CMD17:  READ_SINGLE_BLOCK */

        switch (sd->state) {

        case sd_transfer_state:

            sd->state = sd_sendingdata_state;

            sd->data_start = addr;

            sd->data_offset = 0;



            if (sd->data_start + sd->blk_len > sd->size) {

            if (addr + sd->blk_len > sd->size) {

                sd->card_status |= ADDRESS_ERROR;

                return sd_r1;

            }



            sd->state = sd_sendingdata_state;

            sd->data_start = addr;

            sd->data_offset = 0;

            return sd_r1;



        default:
@@ -1188,13 +1190,15 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) 
    case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */

        switch (sd->state) {

        case sd_transfer_state:

            sd->state = sd_sendingdata_state;

            sd->data_start = addr;

            sd->data_offset = 0;



            if (sd->data_start + sd->blk_len > sd->size) {

            if (addr + sd->blk_len > sd->size) {

                sd->card_status |= ADDRESS_ERROR;

                return sd_r1;

            }



            sd->state = sd_sendingdata_state;

            sd->data_start = addr;

            sd->data_offset = 0;

            return sd_r1;



        default:
@@ -1234,14 +1238,17 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) 
            /* Writing in SPI mode not implemented.  */

            if (sd->spi)

                break;



            if (addr + sd->blk_len > sd->size) {

                sd->card_status |= ADDRESS_ERROR;

                return sd_r1;

            }



            sd->state = sd_receivingdata_state;

            sd->data_start = addr;

            sd->data_offset = 0;

            sd->blk_written = 0;



            if (sd->data_start + sd->blk_len > sd->size) {

                sd->card_status |= ADDRESS_ERROR;

            }

            if (sd_wp_addr(sd, sd->data_start)) {

                sd->card_status |= WP_VIOLATION;

            }
@@ -1261,14 +1268,17 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) 
            /* Writing in SPI mode not implemented.  */

            if (sd->spi)

                break;



            if (addr + sd->blk_len > sd->size) {

                sd->card_status |= ADDRESS_ERROR;

                return sd_r1;

            }



            sd->state = sd_receivingdata_state;

            sd->data_start = addr;

            sd->data_offset = 0;

            sd->blk_written = 0;



            if (sd->data_start + sd->blk_len > sd->size) {

                sd->card_status |= ADDRESS_ERROR;

            }

            if (sd_wp_addr(sd, sd->data_start)) {

                sd->card_status |= WP_VIOLATION;

            }