diff options
author | Peter Maydell <peter.maydell@linaro.org> | 2020-07-15 09:06:55 +0100 |
---|---|---|
committer | Peter Maydell <peter.maydell@linaro.org> | 2020-07-15 09:06:55 +0100 |
commit | 3a9163af4e3dd61795a35d47b702e302f98f81d6 (patch) | |
tree | b88eed6621df1f1f23c1f1233a9c8e1b0a3c72ff /hw | |
parent | c920fdba39480989cb5f1af3cc63acccef021b54 (diff) | |
parent | 790762e5487114341cccc5bffcec4cb3c022c3cd (diff) |
Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sdcard-CVE-2020-13253-pull-request' into staging
Fix CVE-2020-13253
By using invalidated address, guest can do out-of-bounds accesses.
These patches fix the issue by only allowing SD card image sizes
power of 2, and not switching to SEND_DATA state when the address
is invalid (out of range).
This issue was found using QEMU fuzzing mode (using --enable-fuzzing,
see docs/devel/fuzzing.txt) and reported by Alexander Bulekov.
Reproducer:
https://bugs.launchpad.net/qemu/+bug/1880822/comments/1
CI jobs results:
. https://cirrus-ci.com/build/5157142548185088
. https://gitlab.com/philmd/qemu/-/pipelines/166381731
. https://travis-ci.org/github/philmd/qemu/builds/707956535
# gpg: Signature made Tue 14 Jul 2020 14:54:44 BST
# gpg: using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE
# gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full]
# Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE
* remotes/philmd-gitlab/tags/sdcard-CVE-2020-13253-pull-request:
hw/sd/sdcard: Do not switch to ReceivingData if address is invalid
hw/sd/sdcard: Update coding style to make checkpatch.pl happy
hw/sd/sdcard: Do not allow invalid SD card sizes
hw/sd/sdcard: Simplify realize() a bit
hw/sd/sdcard: Restrict Class 6 commands to SCSD cards
tests/acceptance/boot_linux: Expand SD card image to power of 2
tests/acceptance/boot_linux: Tag tests using a SD card with 'device:sd'
docs/orangepi: Add instructions for resizing SD image to power of two
MAINTAINERS: Cc qemu-block mailing list
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Diffstat (limited to 'hw')
-rw-r--r-- | hw/sd/sd.c | 86 |
1 files changed, 67 insertions, 19 deletions
diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 5137168d66..fad9cf1ee7 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -32,6 +32,7 @@ #include "qemu/osdep.h" #include "qemu/units.h" +#include "qemu/cutils.h" #include "hw/irq.h" #include "hw/registerfields.h" #include "sysemu/block-backend.h" @@ -920,6 +921,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) sd->multi_blk_cnt = 0; } + if (sd_cmd_class[req.cmd] == 6 && FIELD_EX32(sd->ocr, OCR, CARD_CAPACITY)) { + /* Only Standard Capacity cards support class 6 commands */ + return sd_illegal; + } + switch (req.cmd) { /* Basic commands (Class 0 and Class 1) */ case 0: /* CMD0: GO_IDLE_STATE */ @@ -1165,12 +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: + + 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; - - if (sd->data_start + sd->blk_len > sd->size) - sd->card_status |= ADDRESS_ERROR; return sd_r1; default: @@ -1181,12 +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: + + 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; - - if (sd->data_start + sd->blk_len > sd->size) - sd->card_status |= ADDRESS_ERROR; return sd_r1; default: @@ -1226,17 +1238,23 @@ 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)) + if (sd_wp_addr(sd, sd->data_start)) { sd->card_status |= WP_VIOLATION; - if (sd->csd[14] & 0x30) + } + if (sd->csd[14] & 0x30) { sd->card_status |= WP_VIOLATION; + } return sd_r1; default: @@ -1250,17 +1268,23 @@ 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)) + if (sd_wp_addr(sd, sd->data_start)) { sd->card_status |= WP_VIOLATION; - if (sd->csd[14] & 0x30) + } + if (sd->csd[14] & 0x30) { sd->card_status |= WP_VIOLATION; + } return sd_r1; default: @@ -2100,12 +2124,36 @@ static void sd_realize(DeviceState *dev, Error **errp) return; } - if (sd->blk && blk_is_read_only(sd->blk)) { - error_setg(errp, "Cannot use read-only drive as SD card"); - return; - } - if (sd->blk) { + int64_t blk_size; + + if (blk_is_read_only(sd->blk)) { + error_setg(errp, "Cannot use read-only drive as SD card"); + return; + } + + blk_size = blk_getlength(sd->blk); + if (blk_size > 0 && !is_power_of_2(blk_size)) { + int64_t blk_size_aligned = pow2ceil(blk_size); + char *blk_size_str; + + blk_size_str = size_to_str(blk_size); + error_setg(errp, "Invalid SD card size: %s", blk_size_str); + g_free(blk_size_str); + + blk_size_str = size_to_str(blk_size_aligned); + error_append_hint(errp, + "SD card size has to be a power of 2, e.g. %s.\n" + "You can resize disk images with" + " 'qemu-img resize <imagefile> <new-size>'\n" + "(note that this will lose data if you make the" + " image smaller than it currently is).\n", + blk_size_str); + g_free(blk_size_str); + + return; + } + ret = blk_set_perm(sd->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, BLK_PERM_ALL, errp); if (ret < 0) { |