diff mbox series

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

Message ID 20240731212501.44385-5-philmd@linaro.org
State New
Headers show
Series hw/sd/sdhci: Check ADMA descriptors can be accessed | expand

Commit Message

Philippe Mathieu-Daudé July 31, 2024, 9:25 p.m. UTC 
Since malicious guest can write invalid addresses to
the ADMASYSADDR register, we need to check whether the
descriptor could be correctly filled or not.

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

Comments

Michael Tokarev Dec. 20, 2024, 9:11 p.m. UTC | #1 
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-stable@nongnu.org
> Fixes: d7dfca0807 ("hw/sdhci: introduce standard SD host controller")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@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;
diff mbox series

Patch

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;