qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 0/7] SD/MMC patches for 2021-03-21
@ 2021-03-22 17:16 Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 1/7] hw/sd: sd: Fix build error when DEBUG_SD is on Philippe Mathieu-Daudé
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 17:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Simon Wörner, Bin Meng, Muhammad Alifa Ramdhan,
	Philippe Mathieu-Daudé,
	Cornelius Aschermann, Sergej Schumilo

The following changes since commit b184750926812cb78ac0caf4c4b2b13683b5bde3:

  Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2021-03-22 11:24:55 +0000)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/sdmmc-20210322

for you to fetch changes up to cffb446e8fd19a14e1634c7a3a8b07be3f01d5c9:

  hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different block size is programmed (2021-03-22 16:56:22 +0100)

----------------------------------------------------------------
SD/MMC patches queue

- Fix build error when DEBUG_SD is on
- Perform SD ERASE operation
- SDHCI ADMA heap buffer overflow
  (CVE-2020-17380, CVE-2020-25085, CVE-2021-3409)
----------------------------------------------------------------

Bin Meng (7):
  hw/sd: sd: Fix build error when DEBUG_SD is on
  hw/sd: sd: Actually perform the erase operation
  hw/sd: sdhci: Don't transfer any data when command time out
  hw/sd: sdhci: Don't write to SDHC_SYSAD register when transfer is in
    progress
  hw/sd: sdhci: Correctly set the controller status for ADMA
  hw/sd: sdhci: Limit block size only when SDHC_BLKSIZE register is
    writable
  hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a
    different block size is programmed

 hw/sd/sd.c    | 23 +++++++++++++---------
 hw/sd/sdhci.c | 53 ++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 50 insertions(+), 26 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PULL 1/7] hw/sd: sd: Fix build error when DEBUG_SD is on
  2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
@ 2021-03-22 17:16 ` Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 2/7] hw/sd: sd: Actually perform the erase operation Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 17:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Simon Wörner, Bin Meng, Muhammad Alifa Ramdhan,
	Philippe Mathieu-Daudé,
	Cornelius Aschermann, Sergej Schumilo

From: Bin Meng <bin.meng@windriver.com>

"qemu-common.h" should be included to provide the forward declaration
of qemu_hexdump() when DEBUG_SD is on.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210228050609.24779-1-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8b397effbcc..7b09ce9c2ef 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -47,6 +47,7 @@
 #include "qemu/timer.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qemu-common.h"
 #include "sdmmc-internal.h"
 #include "trace.h"
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 2/7] hw/sd: sd: Actually perform the erase operation
  2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 1/7] hw/sd: sd: Fix build error when DEBUG_SD is on Philippe Mathieu-Daudé
@ 2021-03-22 17:16 ` Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 3/7] hw/sd: sdhci: Don't transfer any data when command time out Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 17:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Simon Wörner, Bin Meng, Muhammad Alifa Ramdhan,
	Philippe Mathieu-Daudé,
	Cornelius Aschermann, Sergej Schumilo

From: Bin Meng <bin.meng@windriver.com>

At present the sd_erase() does not erase the requested range of card
data to 0xFFs. Let's make the erase operation actually happen.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Message-Id: <1613811493-58815-1-git-send-email-bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7b09ce9c2ef..282d39a7042 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -763,10 +763,12 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 
 static void sd_erase(SDState *sd)
 {
-    int i;
     uint64_t erase_start = sd->erase_start;
     uint64_t erase_end = sd->erase_end;
     bool sdsc = true;
+    uint64_t wpnum;
+    uint64_t erase_addr;
+    int erase_len = 1 << HWBLOCK_SHIFT;
 
     trace_sdcard_erase(sd->erase_start, sd->erase_end);
     if (sd->erase_start == INVALID_ADDRESS
@@ -795,17 +797,19 @@ static void sd_erase(SDState *sd)
     sd->erase_end = INVALID_ADDRESS;
     sd->csd[14] |= 0x40;
 
-    /* Only SDSC cards support write protect groups */
-    if (sdsc) {
-        erase_start = sd_addr_to_wpnum(erase_start);
-        erase_end = sd_addr_to_wpnum(erase_end);
-
-        for (i = erase_start; i <= erase_end; i++) {
-            assert(i < sd->wpgrps_size);
-            if (test_bit(i, sd->wp_groups)) {
+    memset(sd->data, 0xff, erase_len);
+    for (erase_addr = erase_start; erase_addr <= erase_end;
+         erase_addr += erase_len) {
+        if (sdsc) {
+            /* Only SDSC cards support write protect groups */
+            wpnum = sd_addr_to_wpnum(erase_addr);
+            assert(wpnum < sd->wpgrps_size);
+            if (test_bit(wpnum, sd->wp_groups)) {
                 sd->card_status |= WP_ERASE_SKIP;
+                continue;
             }
         }
+        BLK_WRITE_BLOCK(erase_addr, erase_len);
     }
 }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 3/7] hw/sd: sdhci: Don't transfer any data when command time out
  2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 1/7] hw/sd: sd: Fix build error when DEBUG_SD is on Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 2/7] hw/sd: sd: Actually perform the erase operation Philippe Mathieu-Daudé
@ 2021-03-22 17:16 ` Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 4/7] hw/sd: sdhci: Don't write to SDHC_SYSAD register when transfer is in progress Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 17:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Simon Wörner, Bin Meng, Muhammad Alifa Ramdhan,
	Philippe Mathieu-Daudé,
	qemu-stable, Alexander Bulekov, Cornelius Aschermann,
	Alistair Francis, Bin Meng, Sergej Schumilo

From: Bin Meng <bmeng.cn@gmail.com>

At the end of sdhci_send_command(), it starts a data transfer if the
command register indicates data is associated. But the data transfer
should only be initiated when the command execution has succeeded.

With this fix, the following reproducer:

outl 0xcf8 0x80001810
outl 0xcfc 0xe1068000
outl 0xcf8 0x80001804
outw 0xcfc 0x7
write 0xe106802c 0x1 0x0f
write 0xe1068004 0xc 0x2801d10101fffffbff28a384
write 0xe106800c 0x1f 0x9dacbbcad9e8f7061524334251606f7e8d9cabbac9d8e7f60514233241505f
write 0xe1068003 0x28 0x80d000251480d000252280d000253080d000253e80d000254c80d000255a80d000256880d0002576
write 0xe1068003 0x1 0xfe

cannot be reproduced with the following QEMU command line:

$ qemu-system-x86_64 -nographic -M pc-q35-5.0 \
      -device sdhci-pci,sd-spec-version=3 \
      -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \
      -device sd-card,drive=mydrive \
      -monitor none -serial none -qtest stdio

Cc: qemu-stable@nongnu.org
Fixes: CVE-2020-17380
Fixes: CVE-2020-25085
Fixes: CVE-2021-3409
Fixes: d7dfca0807a0 ("hw/sdhci: introduce standard SD host controller")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Cornelius Aschermann (Ruhr-Universität Bochum)
Reported-by: Sergej Schumilo (Ruhr-Universität Bochum)
Reported-by: Simon Wörner (Ruhr-Universität Bochum)
Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210303122639.20004-2-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 9acf4467a32..f72d76c1784 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -326,6 +326,7 @@ static void sdhci_send_command(SDHCIState *s)
     SDRequest request;
     uint8_t response[16];
     int rlen;
+    bool timeout = false;
 
     s->errintsts = 0;
     s->acmd12errsts = 0;
@@ -349,6 +350,7 @@ static void sdhci_send_command(SDHCIState *s)
             trace_sdhci_response16(s->rspreg[3], s->rspreg[2],
                                    s->rspreg[1], s->rspreg[0]);
         } else {
+            timeout = true;
             trace_sdhci_error("timeout waiting for command response");
             if (s->errintstsen & SDHC_EISEN_CMDTIMEOUT) {
                 s->errintsts |= SDHC_EIS_CMDTIMEOUT;
@@ -369,7 +371,7 @@ static void sdhci_send_command(SDHCIState *s)
 
     sdhci_update_irq(s);
 
-    if (s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
+    if (!timeout && s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
         s->data_count = 0;
         sdhci_data_transfer(s);
     }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 4/7] hw/sd: sdhci: Don't write to SDHC_SYSAD register when transfer is in progress
  2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-03-22 17:16 ` [PULL 3/7] hw/sd: sdhci: Don't transfer any data when command time out Philippe Mathieu-Daudé
@ 2021-03-22 17:16 ` Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 5/7] hw/sd: sdhci: Correctly set the controller status for ADMA Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 17:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Simon Wörner, Bin Meng, Muhammad Alifa Ramdhan,
	Philippe Mathieu-Daudé,
	qemu-stable, Alexander Bulekov, Cornelius Aschermann, Bin Meng,
	Sergej Schumilo

From: Bin Meng <bmeng.cn@gmail.com>

Per "SD Host Controller Standard Specification Version 7.00"
chapter 2.2.1 SDMA System Address Register:

This register can be accessed only if no transaction is executing
(i.e., after a transaction has stopped).

With this fix, the following reproducer:

outl 0xcf8 0x80001010
outl 0xcfc 0xfbefff00
outl 0xcf8 0x80001001
outl 0xcfc 0x06000000
write 0xfbefff2c 0x1 0x05
write 0xfbefff0f 0x1 0x37
write 0xfbefff0a 0x1 0x01
write 0xfbefff0f 0x1 0x29
write 0xfbefff0f 0x1 0x02
write 0xfbefff0f 0x1 0x03
write 0xfbefff04 0x1 0x01
write 0xfbefff05 0x1 0x01
write 0xfbefff07 0x1 0x02
write 0xfbefff0c 0x1 0x33
write 0xfbefff0e 0x1 0x20
write 0xfbefff0f 0x1 0x00
write 0xfbefff2a 0x1 0x01
write 0xfbefff0c 0x1 0x00
write 0xfbefff03 0x1 0x00
write 0xfbefff05 0x1 0x00
write 0xfbefff2a 0x1 0x02
write 0xfbefff0c 0x1 0x32
write 0xfbefff01 0x1 0x01
write 0xfbefff02 0x1 0x01
write 0xfbefff03 0x1 0x01

cannot be reproduced with the following QEMU command line:

$ qemu-system-x86_64 -nographic -machine accel=qtest -m 512M \
       -nodefaults -device sdhci-pci,sd-spec-version=3 \
       -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \
       -device sd-card,drive=mydrive -qtest stdio

Cc: qemu-stable@nongnu.org
Fixes: CVE-2020-17380
Fixes: CVE-2020-25085
Fixes: CVE-2021-3409
Fixes: d7dfca0807a0 ("hw/sdhci: introduce standard SD host controller")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Cornelius Aschermann (Ruhr-Universität Bochum)
Reported-by: Sergej Schumilo (Ruhr-Universität Bochum)
Reported-by: Simon Wörner (Ruhr-Universität Bochum)
Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210303122639.20004-3-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index f72d76c1784..3feb6c3a1fe 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1121,15 +1121,17 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
 
     switch (offset & ~0x3) {
     case SDHC_SYSAD:
-        s->sdmasysad = (s->sdmasysad & mask) | value;
-        MASKED_WRITE(s->sdmasysad, mask, value);
-        /* Writing to last byte of sdmasysad might trigger transfer */
-        if (!(mask & 0xFF000000) && TRANSFERRING_DATA(s->prnsts) && s->blkcnt &&
-                s->blksize && SDHC_DMA_TYPE(s->hostctl1) == SDHC_CTRL_SDMA) {
-            if (s->trnmod & SDHC_TRNS_MULTI) {
-                sdhci_sdma_transfer_multi_blocks(s);
-            } else {
-                sdhci_sdma_transfer_single_block(s);
+        if (!TRANSFERRING_DATA(s->prnsts)) {
+            s->sdmasysad = (s->sdmasysad & mask) | value;
+            MASKED_WRITE(s->sdmasysad, mask, value);
+            /* Writing to last byte of sdmasysad might trigger transfer */
+            if (!(mask & 0xFF000000) && s->blkcnt && s->blksize &&
+                SDHC_DMA_TYPE(s->hostctl1) == SDHC_CTRL_SDMA) {
+                if (s->trnmod & SDHC_TRNS_MULTI) {
+                    sdhci_sdma_transfer_multi_blocks(s);
+                } else {
+                    sdhci_sdma_transfer_single_block(s);
+                }
             }
         }
         break;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 5/7] hw/sd: sdhci: Correctly set the controller status for ADMA
  2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2021-03-22 17:16 ` [PULL 4/7] hw/sd: sdhci: Don't write to SDHC_SYSAD register when transfer is in progress Philippe Mathieu-Daudé
@ 2021-03-22 17:16 ` Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 6/7] hw/sd: sdhci: Limit block size only when SDHC_BLKSIZE register is writable Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 17:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Simon Wörner, Bin Meng, Muhammad Alifa Ramdhan,
	Philippe Mathieu-Daudé,
	qemu-stable, Alexander Bulekov, Cornelius Aschermann, Bin Meng,
	Sergej Schumilo

From: Bin Meng <bmeng.cn@gmail.com>

When an ADMA transfer is started, the codes forget to set the
controller status to indicate a transfer is in progress.

With this fix, the following 2 reproducers:

https://paste.debian.net/plain/1185136
https://paste.debian.net/plain/1185141

cannot be reproduced with the following QEMU command line:

$ qemu-system-x86_64 -nographic -machine accel=qtest -m 512M \
      -nodefaults -device sdhci-pci,sd-spec-version=3 \
      -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \
      -device sd-card,drive=mydrive -qtest stdio

Cc: qemu-stable@nongnu.org
Fixes: CVE-2020-17380
Fixes: CVE-2020-25085
Fixes: CVE-2021-3409
Fixes: d7dfca0807a0 ("hw/sdhci: introduce standard SD host controller")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Cornelius Aschermann (Ruhr-Universität Bochum)
Reported-by: Sergej Schumilo (Ruhr-Universität Bochum)
Reported-by: Simon Wörner (Ruhr-Universität Bochum)
Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210303122639.20004-4-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 3feb6c3a1fe..7a2003b28b3 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -768,7 +768,9 @@ static void sdhci_do_adma(SDHCIState *s)
 
         switch (dscr.attr & SDHC_ADMA_ATTR_ACT_MASK) {
         case SDHC_ADMA_ATTR_ACT_TRAN:  /* data transfer */
+            s->prnsts |= SDHC_DATA_INHIBIT | SDHC_DAT_LINE_ACTIVE;
             if (s->trnmod & SDHC_TRNS_READ) {
+                s->prnsts |= SDHC_DOING_READ;
                 while (length) {
                     if (s->data_count == 0) {
                         sdbus_read_data(&s->sdbus, s->fifo_buffer, block_size);
@@ -796,6 +798,7 @@ static void sdhci_do_adma(SDHCIState *s)
                     }
                 }
             } else {
+                s->prnsts |= SDHC_DOING_WRITE;
                 while (length) {
                     begin = s->data_count;
                     if ((length + begin) < block_size) {
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 6/7] hw/sd: sdhci: Limit block size only when SDHC_BLKSIZE register is writable
  2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2021-03-22 17:16 ` [PULL 5/7] hw/sd: sdhci: Correctly set the controller status for ADMA Philippe Mathieu-Daudé
@ 2021-03-22 17:16 ` Philippe Mathieu-Daudé
  2021-03-22 17:16 ` [PULL 7/7] hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different block size is programmed Philippe Mathieu-Daudé
  2021-03-23 10:48 ` [PULL 0/7] SD/MMC patches for 2021-03-21 Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 17:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Simon Wörner, Bin Meng, Muhammad Alifa Ramdhan,
	Philippe Mathieu-Daudé,
	Alexander Bulekov, Cornelius Aschermann, Bin Meng,
	Sergej Schumilo

From: Bin Meng <bmeng.cn@gmail.com>

The codes to limit the maximum block size is only necessary when
SDHC_BLKSIZE register is writable.

Tested-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210303122639.20004-5-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 7a2003b28b3..d0c8e293c0b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1142,15 +1142,15 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
         if (!TRANSFERRING_DATA(s->prnsts)) {
             MASKED_WRITE(s->blksize, mask, extract32(value, 0, 12));
             MASKED_WRITE(s->blkcnt, mask >> 16, value >> 16);
-        }
 
-        /* Limit block size to the maximum buffer size */
-        if (extract32(s->blksize, 0, 12) > s->buf_maxsz) {
-            qemu_log_mask(LOG_GUEST_ERROR, "%s: Size 0x%x is larger than "
-                          "the maximum buffer 0x%x\n", __func__, s->blksize,
-                          s->buf_maxsz);
+            /* Limit block size to the maximum buffer size */
+            if (extract32(s->blksize, 0, 12) > s->buf_maxsz) {
+                qemu_log_mask(LOG_GUEST_ERROR, "%s: Size 0x%x is larger than "
+                              "the maximum buffer 0x%x\n", __func__, s->blksize,
+                              s->buf_maxsz);
 
-            s->blksize = deposit32(s->blksize, 0, 12, s->buf_maxsz);
+                s->blksize = deposit32(s->blksize, 0, 12, s->buf_maxsz);
+            }
         }
 
         break;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 7/7] hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different block size is programmed
  2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2021-03-22 17:16 ` [PULL 6/7] hw/sd: sdhci: Limit block size only when SDHC_BLKSIZE register is writable Philippe Mathieu-Daudé
@ 2021-03-22 17:16 ` Philippe Mathieu-Daudé
  2021-03-23 10:48 ` [PULL 0/7] SD/MMC patches for 2021-03-21 Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 17:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Simon Wörner, Bin Meng, Muhammad Alifa Ramdhan,
	Philippe Mathieu-Daudé,
	qemu-stable, Alexander Bulekov, Cornelius Aschermann, Bin Meng,
	Sergej Schumilo

From: Bin Meng <bmeng.cn@gmail.com>

If the block size is programmed to a different value from the
previous one, reset the data pointer of s->fifo_buffer[] so that
s->fifo_buffer[] can be filled in using the new block size in
the next transfer.

With this fix, the following reproducer:

outl 0xcf8 0x80001010
outl 0xcfc 0xe0000000
outl 0xcf8 0x80001001
outl 0xcfc 0x06000000
write 0xe000002c 0x1 0x05
write 0xe0000005 0x1 0x02
write 0xe0000007 0x1 0x01
write 0xe0000028 0x1 0x10
write 0x0 0x1 0x23
write 0x2 0x1 0x08
write 0xe000000c 0x1 0x01
write 0xe000000e 0x1 0x20
write 0xe000000f 0x1 0x00
write 0xe000000c 0x1 0x32
write 0xe0000004 0x2 0x0200
write 0xe0000028 0x1 0x00
write 0xe0000003 0x1 0x40

cannot be reproduced with the following QEMU command line:

$ qemu-system-x86_64 -nographic -machine accel=qtest -m 512M \
      -nodefaults -device sdhci-pci,sd-spec-version=3 \
      -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \
      -device sd-card,drive=mydrive -qtest stdio

Cc: qemu-stable@nongnu.org
Fixes: CVE-2020-17380
Fixes: CVE-2020-25085
Fixes: CVE-2021-3409
Fixes: d7dfca0807a0 ("hw/sdhci: introduce standard SD host controller")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Cornelius Aschermann (Ruhr-Universität Bochum)
Reported-by: Sergej Schumilo (Ruhr-Universität Bochum)
Reported-by: Simon Wörner (Ruhr-Universität Bochum)
Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210303122639.20004-6-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index d0c8e293c0b..5b8678110b0 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1140,6 +1140,8 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
         break;
     case SDHC_BLKSIZE:
         if (!TRANSFERRING_DATA(s->prnsts)) {
+            uint16_t blksize = s->blksize;
+
             MASKED_WRITE(s->blksize, mask, extract32(value, 0, 12));
             MASKED_WRITE(s->blkcnt, mask >> 16, value >> 16);
 
@@ -1151,6 +1153,16 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
 
                 s->blksize = deposit32(s->blksize, 0, 12, s->buf_maxsz);
             }
+
+            /*
+             * If the block size is programmed to a different value from
+             * the previous one, reset the data pointer of s->fifo_buffer[]
+             * so that s->fifo_buffer[] can be filled in using the new block
+             * size in the next transfer.
+             */
+            if (blksize != s->blksize) {
+                s->data_count = 0;
+            }
         }
 
         break;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PULL 0/7] SD/MMC patches for 2021-03-21
  2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2021-03-22 17:16 ` [PULL 7/7] hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different block size is programmed Philippe Mathieu-Daudé
@ 2021-03-23 10:48 ` Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2021-03-23 10:48 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Simon Wörner, Qemu-block, Bin Meng, Muhammad Alifa Ramdhan,
	QEMU Developers, Cornelius Aschermann, Sergej Schumilo

On Mon, 22 Mar 2021 at 17:23, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The following changes since commit b184750926812cb78ac0caf4c4b2b13683b5bde3:
>
>   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2021-03-22 11:24:55 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/sdmmc-20210322
>
> for you to fetch changes up to cffb446e8fd19a14e1634c7a3a8b07be3f01d5c9:
>
>   hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different block size is programmed (2021-03-22 16:56:22 +0100)
>
> ----------------------------------------------------------------
> SD/MMC patches queue
>
> - Fix build error when DEBUG_SD is on
> - Perform SD ERASE operation
> - SDHCI ADMA heap buffer overflow
>   (CVE-2020-17380, CVE-2020-25085, CVE-2021-3409)


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-23 10:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 17:16 [PULL 0/7] SD/MMC patches for 2021-03-21 Philippe Mathieu-Daudé
2021-03-22 17:16 ` [PULL 1/7] hw/sd: sd: Fix build error when DEBUG_SD is on Philippe Mathieu-Daudé
2021-03-22 17:16 ` [PULL 2/7] hw/sd: sd: Actually perform the erase operation Philippe Mathieu-Daudé
2021-03-22 17:16 ` [PULL 3/7] hw/sd: sdhci: Don't transfer any data when command time out Philippe Mathieu-Daudé
2021-03-22 17:16 ` [PULL 4/7] hw/sd: sdhci: Don't write to SDHC_SYSAD register when transfer is in progress Philippe Mathieu-Daudé
2021-03-22 17:16 ` [PULL 5/7] hw/sd: sdhci: Correctly set the controller status for ADMA Philippe Mathieu-Daudé
2021-03-22 17:16 ` [PULL 6/7] hw/sd: sdhci: Limit block size only when SDHC_BLKSIZE register is writable Philippe Mathieu-Daudé
2021-03-22 17:16 ` [PULL 7/7] hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different block size is programmed Philippe Mathieu-Daudé
2021-03-23 10:48 ` [PULL 0/7] SD/MMC patches for 2021-03-21 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).