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-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 | 86 ++++++++++++++++++++++++++--------------------
hw/sd/trace-events | 2 +-
2 files changed, 49 insertions(+), 39 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 66b9364e9e..eb0476b9aa 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -698,53 +698,62 @@ static void trace_adma_description(const char *type,
const ADMADescr *dscr)
trace_sdhci_adma_desc(type, dscr->addr, dscr->length, dscr->attr,
dscr->incr);
}
-static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
+static MemTxResult get_adma_description(SDHCIState *s, ADMADescr *dscr)
{
uint32_t adma1 = 0;
uint64_t adma2 = 0;
hwaddr entry_addr = (hwaddr)s->admasysaddr;
+ MemTxResult res;
+
switch (SDHC_DMA_TYPE(s->hostctl1)) {
case SDHC_CTRL_ADMA2_32:
- dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2),
- MEMTXATTRS_UNSPECIFIED);
- adma2 = le64_to_cpu(adma2);
- /* The spec does not specify endianness of descriptor table.
- * We currently assume that it is LE.
- */
- dscr->addr = (hwaddr)extract64(adma2, 32, 32) & ~0x3ull;
- dscr->length = (uint16_t)extract64(adma2, 16, 16);
- dscr->attr = (uint8_t)extract64(adma2, 0, 7);
- dscr->incr = 8;
- trace_adma_description("ADMA2_32", dscr);
+ res = dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2),
+ MEMTXATTRS_UNSPECIFIED);
+ if (res == MEMTX_OK) {
+ adma2 = le64_to_cpu(adma2);
+ /* The spec does not specify endianness of descriptor table.
+ * We currently assume that it is LE.
+ */
+ dscr->addr = (hwaddr)extract64(adma2, 32, 32) & ~0x3ull;
+ dscr->length = (uint16_t)extract64(adma2, 16, 16);
+ dscr->attr = (uint8_t)extract64(adma2, 0, 7);
+ dscr->incr = 8;
+ trace_adma_description("ADMA2_32", dscr);
+ }
break;
case SDHC_CTRL_ADMA1_32:
- dma_memory_read(s->dma_as, entry_addr, &adma1, sizeof(adma1),
- MEMTXATTRS_UNSPECIFIED);
- adma1 = le32_to_cpu(adma1);
- dscr->addr = (hwaddr)(adma1 & 0xFFFFF000);
- dscr->attr = (uint8_t)extract32(adma1, 0, 7);
- dscr->incr = 4;
- if ((dscr->attr & SDHC_ADMA_ATTR_ACT_MASK) == SDHC_ADMA_ATTR_SET_LEN) {
- dscr->length = (uint16_t)extract32(adma1, 12, 16);
- } else {
- dscr->length = 4 * KiB;
+ res = dma_memory_read(s->dma_as, entry_addr, &adma1, sizeof(adma1),
+ MEMTXATTRS_UNSPECIFIED);
+ if (res == MEMTX_OK) {
+ adma1 = le32_to_cpu(adma1);
+ dscr->addr = (hwaddr)(adma1 & ~0xfff);
+ dscr->attr = (uint8_t)extract32(adma1, 0, 7);
+ dscr->incr = 4;
+ if ((dscr->attr & SDHC_ADMA_ATTR_ACT_MASK) ==
SDHC_ADMA_ATTR_SET_LEN) {
+ dscr->length = (uint16_t)extract32(adma1, 12, 16);
+ } else {
+ dscr->length = 4 * KiB;
+ }
+ trace_adma_description("ADMA1_32", dscr);
}
- trace_adma_description("ADMA1_32", dscr);
break;
case SDHC_CTRL_ADMA2_64:
- dma_memory_read(s->dma_as, entry_addr, &dscr->attr, 1,
- MEMTXATTRS_UNSPECIFIED);
- dma_memory_read(s->dma_as, entry_addr + 2, &dscr->length, 2,
- MEMTXATTRS_UNSPECIFIED);
- dscr->length = le16_to_cpu(dscr->length);
- dma_memory_read(s->dma_as, entry_addr + 4, &dscr->addr, 8,
- MEMTXATTRS_UNSPECIFIED);
- dscr->addr = le64_to_cpu(dscr->addr);
- dscr->attr &= (uint8_t) ~0xC0;
- dscr->incr = 12;
- trace_adma_description("ADMA2_64", dscr);
+ res = dma_memory_read(s->dma_as, entry_addr, &dscr->attr, 1,
+ MEMTXATTRS_UNSPECIFIED);
+ res |= dma_memory_read(s->dma_as, entry_addr + 2, &dscr->length, 2,
+ MEMTXATTRS_UNSPECIFIED);
+ res |= dma_memory_read(s->dma_as, entry_addr + 4, &dscr->addr, 8,
+ MEMTXATTRS_UNSPECIFIED);
+ if (res == MEMTX_OK) {
+ dscr->length = le16_to_cpu(dscr->length);
+ dscr->addr = le64_to_cpu(dscr->addr);
+ dscr->attr &= (uint8_t) ~0xc0;
+ dscr->incr = 12;
+ trace_adma_description("ADMA2_64", dscr);
+ }
break;
}
+ return res;
}
/* Advanced DMA data transfer */
@@ -755,7 +764,6 @@ static void sdhci_do_adma(SDHCIState *s)
const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK;
const MemTxAttrs attrs = { .memory = true };
ADMADescr dscr = {};
- MemTxResult res;
int i;
if (s->trnmod & SDHC_TRNS_BLK_CNT_EN && !s->blkcnt) {
@@ -765,12 +773,14 @@ static void sdhci_do_adma(SDHCIState *s)
}
for (i = 0; i < SDHC_ADMA_DESCS_PER_DELAY; ++i) {
+ MemTxResult res;
+
s->admaerr &= ~SDHC_ADMAERR_LENGTH_MISMATCH;
- get_adma_description(s, &dscr);
- trace_sdhci_adma_loop(dscr.addr, dscr.length, dscr.attr);
+ res = get_adma_description(s, &dscr);
+ trace_sdhci_adma_loop(dscr.addr, dscr.length, dscr.attr, res);
- if ((dscr.attr & SDHC_ADMA_ATTR_VALID) == 0) {
+ if (res != MEMTX_OK || (dscr.attr & SDHC_ADMA_ATTR_VALID) == 0) {
/* Indicate that error occurred in ST_FDS state */
s->admaerr &= ~SDHC_ADMAERR_STATE_MASK;
s->admaerr |= SDHC_ADMAERR_STATE_ST_FDS;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 3d3f5c1cb7..a802a717b9 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -29,7 +29,7 @@ sdhci_response4(uint32_t r0) "RSPREG[31..0]=0x%08x"
sdhci_response16(uint32_t r3, uint32_t r2, uint32_t r1, uint32_t r0)
"RSPREG[127..96]=0x%08x, RSPREG[95..64]=0x%08x, RSPREG[63..32]=0x%08x,
RSPREG[31..0]=0x%08x"
sdhci_end_transfer(uint8_t cmd, uint32_t arg) "Automatically issue CMD%02u
0x%08x"
sdhci_adma(const char *desc, uint32_t sysad) "%s: admasysaddr=0x%" PRIx32
-sdhci_adma_loop(uint64_t addr, uint16_t length, uint8_t attr) "addr=0x%08"
PRIx64 ", len=%d, attr=0x%x"
+sdhci_adma_loop(uint64_t addr, uint16_t length, uint8_t attr, uint32_t res)
"addr=0x%08" PRIx64 ", len=%d, attr=0x%x, res=%" PRIu32
sdhci_adma_transfer_completed(void) ""
sdhci_access(const char *access, unsigned int size, uint64_t offset, const
char *dir, uint64_t val, uint64_t val2) "%s%u: addr[0x%04" PRIx64 "] %s 0x%08"
PRIx64 " (%" PRIu64 ")"
sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read
from input buffer"
--
2.45.2
