Re: [PATCH-for-9.1? v2 4/4] hw/sd/sdhci: Check ADMA descriptors can be accessed

[Michael Tokarev]

01.08.2024 00:25, Philippe Mathieu-Daudé wrote:

Since malicious guest can write invalid addresses to
the ADMASYSADDR register, we need to check whether the
descriptor could be correctly filled or not.

Ping?  This has been about the 9.1 release, now 9.2 is out already
and we're working on 10.0...

Thanks,

/mjt

Cc: qemu-sta...@nongnu.org
Fixes: d7dfca0807 ("hw/sdhci: introduce standard SD host controller")
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
  hw/sd/sdhci.c | 23 +++++++++++++++++------
  1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 2d8fa3151a..6794ee2267 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -701,13 +701,18 @@ static void trace_adma_description(const char *type, 
const ADMADescr *dscr)
  static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
  {
      hwaddr entry_addr = (hwaddr)s->admasysaddr;
+    MemTxResult res;
+
      switch (SDHC_DMA_TYPE(s->hostctl1)) {
      case SDHC_CTRL_ADMA2_32:
          {
              uint64_t adma2 = 0;

- dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2),

-                            MEMTXATTRS_UNSPECIFIED);
+            res = dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2),
+                                  MEMTXATTRS_UNSPECIFIED);
+            if (res != MEMTX_OK) {
+                break;
+            }
              adma2 = le64_to_cpu(adma2);
              /*
               * The spec does not specify endianness of descriptor table.
@@ -724,8 +729,11 @@ static void get_adma_description(SDHCIState *s, ADMADescr 
*dscr)
          {
              uint32_t adma1 = 0;

- dma_memory_read(s->dma_as, entry_addr, &adma1, sizeof(adma1),

-                            MEMTXATTRS_UNSPECIFIED);
+            res = dma_memory_read(s->dma_as, entry_addr, &adma1, sizeof(adma1),
+                                  MEMTXATTRS_UNSPECIFIED);
+            if (res != MEMTX_OK) {
+                break;
+            }
              adma1 = le32_to_cpu(adma1);
              dscr->addr = (hwaddr)(adma1 & ~0xfff);
              dscr->attr = (uint8_t)extract32(adma1, 0, 7);
@@ -748,8 +756,11 @@ static void get_adma_description(SDHCIState *s, ADMADescr 
*dscr)
              } QEMU_PACKED adma2;
              QEMU_BUILD_BUG_ON(sizeof(adma2) != 12);

- dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2),

-                            MEMTXATTRS_UNSPECIFIED);
+            res = dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2),
+                                  MEMTXATTRS_UNSPECIFIED);
+            if (res != MEMTX_OK) {
+                break;
+            }
              dscr->length = le16_to_cpu(adma2.length);
              dscr->addr = le64_to_cpu(adma2.addr);
              dscr->attr = adma2.attr & (uint8_t) ~0xc0;

--
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 
ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 
8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt