Merge tag 'pull-request-2022-03-21' of https://gitlab.com/thuth/qemu into staging
* Fix stack-overflow due to recursive DMA in intel-hda (CVE-2021-3611)
* Fix heap overflow due to recursive DMA in sdhci code
# gpg: Signature made Mon 21 Mar 2022 16:14:36 GMT
# gpg: using RSA key 27B88847EEE0250118F3EAB92ED9D774FE702DB5
# gpg: issuer "thuth@redhat.com"
# gpg: Good signature from "Thomas Huth <th.huth@gmx.de>" [full]
# gpg: aka "Thomas Huth <thuth@redhat.com>" [full]
# gpg: aka "Thomas Huth <huth@tuxfamily.org>" [full]
# gpg: aka "Thomas Huth <th.huth@posteo.de>" [unknown]
# Primary key fingerprint: 27B8 8847 EEE0 2501 18F3 EAB9 2ED9 D774 FE70 2DB5
* tag 'pull-request-2022-03-21' of https://gitlab.com/thuth/qemu:
tests/qtest/fuzz-sdcard-test: Add reproducer for OSS-Fuzz (Issue 29225)
hw/sd/sdhci: Prohibit DMA accesses to devices
hw/sd/sdhci: Honor failed DMA transactions
tests/qtest/intel-hda-test: Add reproducer for issue #542
hw/audio/intel-hda: Restrict DMA engine to memories (not MMIO devices)
hw/audio/intel-hda: Do not ignore DMA overrun errors
softmmu/physmem: Introduce MemTxAttrs::memory field and MEMTX_ACCESS_ERROR
softmmu/physmem: Simplify flatview_write and address_space_access_valid
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 686fb94..bc77e3d 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -345,11 +345,12 @@
static void intel_hda_response(HDACodecDevice *dev, bool solicited, uint32_t response)
{
- const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
+ const MemTxAttrs attrs = { .memory = true };
HDACodecBus *bus = HDA_BUS(dev->qdev.parent_bus);
IntelHDAState *d = container_of(bus, IntelHDAState, codecs);
hwaddr addr;
uint32_t wp, ex;
+ MemTxResult res = MEMTX_OK;
if (d->ics & ICH6_IRS_BUSY) {
dprint(d, 2, "%s: [irr] response 0x%x, cad 0x%x\n",
@@ -368,8 +369,12 @@
ex = (solicited ? 0 : (1 << 4)) | dev->cad;
wp = (d->rirb_wp + 1) & 0xff;
addr = intel_hda_addr(d->rirb_lbase, d->rirb_ubase);
- stl_le_pci_dma(&d->pci, addr + 8 * wp, response, attrs);
- stl_le_pci_dma(&d->pci, addr + 8 * wp + 4, ex, attrs);
+ res |= stl_le_pci_dma(&d->pci, addr + 8 * wp, response, attrs);
+ res |= stl_le_pci_dma(&d->pci, addr + 8 * wp + 4, ex, attrs);
+ if (res != MEMTX_OK && (d->rirb_ctl & ICH6_RBCTL_OVERRUN_EN)) {
+ d->rirb_sts |= ICH6_RBSTS_OVERRUN;
+ intel_hda_update_irq(d);
+ }
d->rirb_wp = wp;
dprint(d, 2, "%s: [wp 0x%x] response 0x%x, extra 0x%x\n",
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e0bbc90..0e5e9889 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -741,7 +741,9 @@
{
unsigned int begin, length;
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) {
@@ -790,10 +792,13 @@
s->data_count = block_size;
length -= block_size - begin;
}
- dma_memory_write(s->dma_as, dscr.addr,
- &s->fifo_buffer[begin],
- s->data_count - begin,
- MEMTXATTRS_UNSPECIFIED);
+ res = dma_memory_write(s->dma_as, dscr.addr,
+ &s->fifo_buffer[begin],
+ s->data_count - begin,
+ attrs);
+ if (res != MEMTX_OK) {
+ break;
+ }
dscr.addr += s->data_count - begin;
if (s->data_count == block_size) {
s->data_count = 0;
@@ -816,10 +821,13 @@
s->data_count = block_size;
length -= block_size - begin;
}
- dma_memory_read(s->dma_as, dscr.addr,
- &s->fifo_buffer[begin],
- s->data_count - begin,
- MEMTXATTRS_UNSPECIFIED);
+ res = dma_memory_read(s->dma_as, dscr.addr,
+ &s->fifo_buffer[begin],
+ s->data_count - begin,
+ attrs);
+ if (res != MEMTX_OK) {
+ break;
+ }
dscr.addr += s->data_count - begin;
if (s->data_count == block_size) {
sdbus_write_data(&s->sdbus, s->fifo_buffer, block_size);
@@ -833,7 +841,16 @@
}
}
}
- s->admasysaddr += dscr.incr;
+ if (res != MEMTX_OK) {
+ if (s->errintstsen & SDHC_EISEN_ADMAERR) {
+ trace_sdhci_error("Set ADMA error flag");
+ s->errintsts |= SDHC_EIS_ADMAERR;
+ s->norintsts |= SDHC_NIS_ERR;
+ }
+ sdhci_update_irq(s);
+ } else {
+ s->admasysaddr += dscr.incr;
+ }
break;
case SDHC_ADMA_ATTR_ACT_LINK: /* link to next descriptor table */
s->admasysaddr = dscr.addr;
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 95f2d20..9fb98bc 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -35,6 +35,14 @@
unsigned int secure:1;
/* Memory access is usermode (unprivileged) */
unsigned int user:1;
+ /*
+ * Bus interconnect and peripherals can access anything (memories,
+ * devices) by default. By setting the 'memory' bit, bus transaction
+ * are restricted to "normal" memories (per the AMBA documentation)
+ * versus devices. Access to devices will be logged and rejected
+ * (see MEMTX_ACCESS_ERROR).
+ */
+ unsigned int memory:1;
/* Requester ID (for MSI for example) */
unsigned int requester_id:16;
/* Invert endianness for this page */
@@ -66,6 +74,7 @@
#define MEMTX_OK 0
#define MEMTX_ERROR (1U << 0) /* device returned an error */
#define MEMTX_DECODE_ERROR (1U << 1) /* nothing at that address */
+#define MEMTX_ACCESS_ERROR (1U << 2) /* access denied */
typedef uint32_t MemTxResult;
#endif
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 43ae70f..4e1b27a 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -42,6 +42,7 @@
#include "qemu/config-file.h"
#include "qemu/error-report.h"
#include "qemu/qemu-print.h"
+#include "qemu/log.h"
#include "qemu/memalign.h"
#include "exec/memory.h"
#include "exec/ioport.h"
@@ -2760,6 +2761,33 @@
return release_lock;
}
+/**
+ * flatview_access_allowed
+ * @mr: #MemoryRegion to be accessed
+ * @attrs: memory transaction attributes
+ * @addr: address within that memory region
+ * @len: the number of bytes to access
+ *
+ * Check if a memory transaction is allowed.
+ *
+ * Returns: true if transaction is allowed, false if denied.
+ */
+static bool flatview_access_allowed(MemoryRegion *mr, MemTxAttrs attrs,
+ hwaddr addr, hwaddr len)
+{
+ if (likely(!attrs.memory)) {
+ return true;
+ }
+ if (memory_region_is_ram(mr)) {
+ return true;
+ }
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid access to non-RAM device at "
+ "addr 0x%" HWADDR_PRIX ", size %" HWADDR_PRIu ", "
+ "region '%s'\n", addr, len, memory_region_name(mr));
+ return false;
+}
+
/* Called within RCU critical section. */
static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
MemTxAttrs attrs,
@@ -2774,7 +2802,10 @@
const uint8_t *buf = ptr;
for (;;) {
- if (!memory_access_is_direct(mr, true)) {
+ if (!flatview_access_allowed(mr, attrs, addr1, l)) {
+ result |= MEMTX_ACCESS_ERROR;
+ /* Keep going. */
+ } else if (!memory_access_is_direct(mr, true)) {
release_lock |= prepare_mmio_access(mr);
l = memory_access_size(mr, l, addr1);
/* XXX: could force current_cpu to NULL to avoid
@@ -2816,14 +2847,14 @@
hwaddr l;
hwaddr addr1;
MemoryRegion *mr;
- MemTxResult result = MEMTX_OK;
l = len;
mr = flatview_translate(fv, addr, &addr1, &l, true, attrs);
- result = flatview_write_continue(fv, addr, attrs, buf, len,
- addr1, l, mr);
-
- return result;
+ if (!flatview_access_allowed(mr, attrs, addr, len)) {
+ return MEMTX_ACCESS_ERROR;
+ }
+ return flatview_write_continue(fv, addr, attrs, buf, len,
+ addr1, l, mr);
}
/* Called within RCU critical section. */
@@ -2840,7 +2871,10 @@
fuzz_dma_read_cb(addr, len, mr);
for (;;) {
- if (!memory_access_is_direct(mr, false)) {
+ if (!flatview_access_allowed(mr, attrs, addr1, l)) {
+ result |= MEMTX_ACCESS_ERROR;
+ /* Keep going. */
+ } else if (!memory_access_is_direct(mr, false)) {
/* I/O case */
release_lock |= prepare_mmio_access(mr);
l = memory_access_size(mr, l, addr1);
@@ -2883,6 +2917,9 @@
l = len;
mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
+ if (!flatview_access_allowed(mr, attrs, addr, len)) {
+ return MEMTX_ACCESS_ERROR;
+ }
return flatview_read_continue(fv, addr, attrs, buf, len,
addr1, l, mr);
}
@@ -3139,12 +3176,10 @@
MemTxAttrs attrs)
{
FlatView *fv;
- bool result;
RCU_READ_LOCK_GUARD();
fv = address_space_to_flatview(as);
- result = flatview_access_valid(fv, addr, len, is_write, attrs);
- return result;
+ return flatview_access_valid(fv, addr, len, is_write, attrs);
}
static hwaddr
diff --git a/tests/qtest/fuzz-sdcard-test.c b/tests/qtest/fuzz-sdcard-test.c
index ae14305..0f94965 100644
--- a/tests/qtest/fuzz-sdcard-test.c
+++ b/tests/qtest/fuzz-sdcard-test.c
@@ -87,6 +87,81 @@
qtest_quit(s);
}
+/*
+ * https://gitlab.com/qemu-project/qemu/-/issues/451
+ * Used to trigger a heap buffer overflow.
+ */
+static void oss_fuzz_36391(void)
+{
+ QTestState *s;
+
+ s = qtest_init(" -display none -m 512M -nodefaults -nographic"
+ " -device sdhci-pci,sd-spec-version=3"
+ " -device sd-card,drive=drv"
+ " -drive if=none,index=0,file=null-co://,format=raw,id=drv");
+ qtest_outl(s, 0xcf8, 0x80001010);
+ qtest_outl(s, 0xcfc, 0xe0000000);
+ qtest_outl(s, 0xcf8, 0x80001004);
+ qtest_outw(s, 0xcfc, 0x7);
+ qtest_bufwrite(s, 0xe0000005, "\x73", 0x1);
+ qtest_bufwrite(s, 0xe0000028, "\x55", 0x1);
+ qtest_bufwrite(s, 0xe000002c, "\x55", 0x1);
+ qtest_bufwrite(s, 0x0, "\x65", 0x1);
+ qtest_bufwrite(s, 0x7, "\x69", 0x1);
+ qtest_bufwrite(s, 0x8, "\x65", 0x1);
+ qtest_bufwrite(s, 0xf, "\x69", 0x1);
+ qtest_bufwrite(s, 0x10, "\x65", 0x1);
+ qtest_bufwrite(s, 0x17, "\x69", 0x1);
+ qtest_bufwrite(s, 0x18, "\x65", 0x1);
+ qtest_bufwrite(s, 0x1f, "\x69", 0x1);
+ qtest_bufwrite(s, 0x20, "\x65", 0x1);
+ qtest_bufwrite(s, 0x27, "\x69", 0x1);
+ qtest_bufwrite(s, 0x28, "\x65", 0x1);
+ qtest_bufwrite(s, 0x2f, "\x69", 0x1);
+ qtest_bufwrite(s, 0x30, "\x65", 0x1);
+ qtest_bufwrite(s, 0x37, "\x69", 0x1);
+ qtest_bufwrite(s, 0x38, "\x65", 0x1);
+ qtest_bufwrite(s, 0x3f, "\x69", 0x1);
+ qtest_bufwrite(s, 0x40, "\x65", 0x1);
+ qtest_bufwrite(s, 0x47, "\x69", 0x1);
+ qtest_bufwrite(s, 0x48, "\x65", 0x1);
+ qtest_bufwrite(s, 0xe000000c, "\x55", 0x1);
+ qtest_bufwrite(s, 0xe000000e, "\x2c", 0x1);
+ qtest_bufwrite(s, 0xe000000f, "\x5b", 0x1);
+ qtest_bufwrite(s, 0xe0000010, "\x06\x46", 0x2);
+ qtest_bufwrite(s, 0x50, "\x65", 0x1);
+ qtest_bufwrite(s, 0x57, "\x69", 0x1);
+ qtest_bufwrite(s, 0x58, "\x65", 0x1);
+ qtest_bufwrite(s, 0x5f, "\x69", 0x1);
+ qtest_bufwrite(s, 0x60, "\x65", 0x1);
+ qtest_bufwrite(s, 0x67, "\x69", 0x1);
+ qtest_bufwrite(s, 0x68, "\x65", 0x1);
+ qtest_bufwrite(s, 0x6f, "\x69", 0x1);
+ qtest_bufwrite(s, 0x70, "\x65", 0x1);
+ qtest_bufwrite(s, 0x77, "\x69", 0x1);
+ qtest_bufwrite(s, 0x78, "\x65", 0x1);
+ qtest_bufwrite(s, 0x7f, "\x69", 0x1);
+ qtest_bufwrite(s, 0x80, "\x65", 0x1);
+ qtest_bufwrite(s, 0x87, "\x69", 0x1);
+ qtest_bufwrite(s, 0x88, "\x65", 0x1);
+ qtest_bufwrite(s, 0x8f, "\x69", 0x1);
+ qtest_bufwrite(s, 0x90, "\x65", 0x1);
+ qtest_bufwrite(s, 0x97, "\x69", 0x1);
+ qtest_bufwrite(s, 0x98, "\x65", 0x1);
+ qtest_bufwrite(s, 0xe0000026, "\x5a\x06", 0x2);
+ qtest_bufwrite(s, 0xe0000028, "\x46\xc0\xc9\xc9", 0x4);
+ qtest_bufwrite(s, 0xe0000028, "\x55", 0x1);
+ qtest_bufwrite(s, 0xe000002a, "\x5a", 0x1);
+ qtest_bufwrite(s, 0xa0, "\x65", 0x1);
+ qtest_bufwrite(s, 0xa5, "\xff", 0x1);
+ qtest_bufwrite(s, 0xa6, "\xff", 0x1);
+ qtest_bufwrite(s, 0xa7, "\xdf", 0x1);
+ qtest_bufwrite(s, 0xe000000c, "\x27", 0x1);
+ qtest_bufwrite(s, 0xe000000f, "\x55", 0x1);
+
+ qtest_quit(s);
+}
+
int main(int argc, char **argv)
{
const char *arch = qtest_get_arch();
@@ -96,6 +171,7 @@
if (strcmp(arch, "i386") == 0) {
qtest_add_func("fuzz/sdcard/oss_fuzz_29225", oss_fuzz_29225);
qtest_add_func("fuzz/sdcard/oss_fuzz_36217", oss_fuzz_36217);
+ qtest_add_func("fuzz/sdcard/oss_fuzz_36391", oss_fuzz_36391);
}
return g_test_run();
diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
index fc25ccc..a58c98e 100644
--- a/tests/qtest/intel-hda-test.c
+++ b/tests/qtest/intel-hda-test.c
@@ -29,11 +29,45 @@
qtest_end();
}
+/*
+ * https://gitlab.com/qemu-project/qemu/-/issues/542
+ * Used to trigger:
+ * AddressSanitizer: stack-overflow
+ */
+static void test_issue542_ich6(void)
+{
+ QTestState *s;
+
+ s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
+ "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
+
+ qtest_outl(s, 0xcf8, 0x80000804);
+ qtest_outw(s, 0xcfc, 0x06);
+ qtest_bufwrite(s, 0xff0d060f, "\x03", 1);
+ qtest_bufwrite(s, 0x0, "\x12", 1);
+ qtest_bufwrite(s, 0x2, "\x2a", 1);
+ qtest_writeb(s, 0x0, 0x12);
+ qtest_writeb(s, 0x2, 0x2a);
+ qtest_outl(s, 0xcf8, 0x80000811);
+ qtest_outl(s, 0xcfc, 0x006a4400);
+ qtest_bufwrite(s, 0x6a44005a, "\x01", 1);
+ qtest_bufwrite(s, 0x6a44005c, "\x02", 1);
+ qtest_bufwrite(s, 0x6a442050, "\x00\x00\x44\x6a", 4);
+ qtest_bufwrite(s, 0x6a44204a, "\x01", 1);
+ qtest_bufwrite(s, 0x6a44204c, "\x02", 1);
+ qtest_bufwrite(s, 0x6a44005c, "\x02", 1);
+ qtest_bufwrite(s, 0x6a442050, "\x00\x00\x44\x6a", 4);
+ qtest_bufwrite(s, 0x6a44204a, "\x01", 1);
+ qtest_bufwrite(s, 0x6a44204c, "\x02", 1);
+ qtest_quit(s);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
qtest_add_func("/intel-hda/ich6", ich6_test);
qtest_add_func("/intel-hda/ich9", ich9_test);
+ qtest_add_func("/intel-hda/fuzz/issue542", test_issue542_ich6);
return g_test_run();
}
