From 1fe98a3a7bf91c0e25d4427403bc700a1d96765c Mon Sep 17 00:00:00 2001 From: Jiabo Feng Date: Sat, 15 Jun 2024 09:54:38 +0800 Subject: [PATCH] QEMU update to version 4.1.0-84: - tests/qtest: ahci-test: add test exposing reset issue with pending callback (Fix CVE-2023-5088) - hw/ide: reset: cancel async DMA operation before resetting state (Fix CVE-2023-5088) Signed-off-by: Jiabo Feng --- ...cel-async-DMA-operation-before-reset.patch | 110 +++++++++++++++ qemu.spec | 8 +- ...-test-add-test-exposing-reset-issue-.patch | 132 ++++++++++++++++++ 3 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 hw-ide-reset-cancel-async-DMA-operation-before-reset.patch create mode 100644 tests-qtest-ahci-test-add-test-exposing-reset-issue-.patch diff --git a/hw-ide-reset-cancel-async-DMA-operation-before-reset.patch b/hw-ide-reset-cancel-async-DMA-operation-before-reset.patch new file mode 100644 index 0000000..72865d4 --- /dev/null +++ b/hw-ide-reset-cancel-async-DMA-operation-before-reset.patch @@ -0,0 +1,110 @@ +From b3cace2a14691c421b5dcc1a98c5ea837ecd3025 Mon Sep 17 00:00:00 2001 +From: Fiona Ebner +Date: Wed, 6 Sep 2023 15:09:21 +0200 +Subject: [PATCH] hw/ide: reset: cancel async DMA operation before resetting + state (Fix CVE-2023-5088) +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +If there is a pending DMA operation during ide_bus_reset(), the fact +that the IDEState is already reset before the operation is canceled +can be problematic. In particular, ide_dma_cb() might be called and +then use the reset IDEState which contains the signature after the +reset. When used to construct the IO operation this leads to +ide_get_sector() returning 0 and nsector being 1. This is particularly +bad, because a write command will thus destroy the first sector which +often contains a partition table or similar. + +Traces showing the unsolicited write happening with IDEState +0x5595af6949d0 being used after reset: + +> ahci_port_write ahci(0x5595af6923f0)[0]: port write [reg:PxSCTL] @ 0x2c: 0x00000300 +> ahci_reset_port ahci(0x5595af6923f0)[0]: reset port +> ide_reset IDEstate 0x5595af6949d0 +> ide_reset IDEstate 0x5595af694da8 +> ide_bus_reset_aio aio_cancel +> dma_aio_cancel dbs=0x7f64600089a0 +> dma_blk_cb dbs=0x7f64600089a0 ret=0 +> dma_complete dbs=0x7f64600089a0 ret=0 cb=0x5595acd40b30 +> ahci_populate_sglist ahci(0x5595af6923f0)[0] +> ahci_dma_prepare_buf ahci(0x5595af6923f0)[0]: prepare buf limit=512 prepared=512 +> ide_dma_cb IDEState 0x5595af6949d0; sector_num=0 n=1 cmd=DMA WRITE +> dma_blk_io dbs=0x7f6420802010 bs=0x5595ae2c6c30 offset=0 to_dev=1 +> dma_blk_cb dbs=0x7f6420802010 ret=0 + +> (gdb) p *qiov +> $11 = {iov = 0x7f647c76d840, niov = 1, {{nalloc = 1, local_iov = {iov_base = 0x0, +> iov_len = 512}}, {__pad = "\001\000\000\000\000\000\000\000\000\000\000", +> size = 512}}} +> (gdb) bt +> #0 blk_aio_pwritev (blk=0x5595ae2c6c30, offset=0, qiov=0x7f6420802070, flags=0, +> cb=0x5595ace6f0b0 , opaque=0x7f6420802010) +> at ../block/block-backend.c:1682 +> #1 0x00005595ace6f185 in dma_blk_cb (opaque=0x7f6420802010, ret=) +> at ../softmmu/dma-helpers.c:179 +> #2 0x00005595ace6f778 in dma_blk_io (ctx=0x5595ae0609f0, +> sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512, +> io_func=io_func@entry=0x5595ace6ee30 , +> io_func_opaque=io_func_opaque@entry=0x5595ae2c6c30, +> cb=0x5595acd40b30 , opaque=0x5595af6949d0, +> dir=DMA_DIRECTION_TO_DEVICE) at ../softmmu/dma-helpers.c:244 +> #3 0x00005595ace6f90a in dma_blk_write (blk=0x5595ae2c6c30, +> sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512, +> cb=cb@entry=0x5595acd40b30 , opaque=opaque@entry=0x5595af6949d0) +> at ../softmmu/dma-helpers.c:280 +> #4 0x00005595acd40e18 in ide_dma_cb (opaque=0x5595af6949d0, ret=) +> at ../hw/ide/core.c:953 +> #5 0x00005595ace6f319 in dma_complete (ret=0, dbs=0x7f64600089a0) +> at ../softmmu/dma-helpers.c:107 +> #6 dma_blk_cb (opaque=0x7f64600089a0, ret=0) at ../softmmu/dma-helpers.c:127 +> #7 0x00005595ad12227d in blk_aio_complete (acb=0x7f6460005b10) +> at ../block/block-backend.c:1527 +> #8 blk_aio_complete (acb=0x7f6460005b10) at ../block/block-backend.c:1524 +> #9 blk_aio_write_entry (opaque=0x7f6460005b10) at ../block/block-backend.c:1594 +> #10 0x00005595ad258cfb in coroutine_trampoline (i0=, +> i1=) at ../util/coroutine-ucontext.c:177 + +Signed-off-by: Fiona Ebner +Reviewed-by: Philippe Mathieu-Daudé +Tested-by: simon.rowe@nutanix.com +Message-ID: <20230906130922.142845-1-f.ebner@proxmox.com> +Signed-off-by: Philippe Mathieu-Daudé +--- + hw/ide/core.c | 14 +++++++------- + 1 file changed, 7 insertions(+), 7 deletions(-) + +diff --git a/hw/ide/core.c b/hw/ide/core.c +index cf8468ce90..b743f7f22d 100644 +--- a/hw/ide/core.c ++++ b/hw/ide/core.c +@@ -2407,19 +2407,19 @@ static void ide_dummy_transfer_stop(IDEState *s) + + void ide_bus_reset(IDEBus *bus) + { +- bus->unit = 0; +- bus->cmd = 0; +- ide_reset(&bus->ifs[0]); +- ide_reset(&bus->ifs[1]); +- ide_clear_hob(bus); +- +- /* pending async DMA */ ++ /* pending async DMA - needs the IDEState before it is reset */ + if (bus->dma->aiocb) { + trace_ide_bus_reset_aio(); + blk_aio_cancel(bus->dma->aiocb); + bus->dma->aiocb = NULL; + } + ++ bus->unit = 0; ++ bus->cmd = 0; ++ ide_reset(&bus->ifs[0]); ++ ide_reset(&bus->ifs[1]); ++ ide_clear_hob(bus); ++ + /* reset dma provider too */ + if (bus->dma->ops->reset) { + bus->dma->ops->reset(bus->dma); +-- +2.41.0.windows.1 + diff --git a/qemu.spec b/qemu.spec index 21ac2fe..284f375 100644 --- a/qemu.spec +++ b/qemu.spec @@ -1,6 +1,6 @@ Name: qemu Version: 4.1.0 -Release: 83 +Release: 84 Epoch: 10 Summary: QEMU is a generic and open source machine emulator and virtualizer License: GPLv2 and BSD and MIT and CC-BY-SA-4.0 @@ -409,6 +409,8 @@ Patch0396: hw-virtio-Introduce-virtio_bh_new_guarded-helper.patch Patch0397: hw-display-virtio-gpu-Protect-from-DMA-re-entrancy-b.patch Patch0398: hw-char-virtio-serial-bus-Protect-from-DMA-re-entran.patch Patch0399: hw-virtio-virtio-crypto-Protect-from-DMA-re-entrancy.patch +Patch0400: hw-ide-reset-cancel-async-DMA-operation-before-reset.patch +Patch0401: tests-qtest-ahci-test-add-test-exposing-reset-issue-.patch BuildRequires: flex BuildRequires: bison @@ -809,6 +811,10 @@ getent passwd qemu >/dev/null || \ %endif %changelog +* Sat Jun 15 2024 Jiabo Feng +- tests/qtest: ahci-test: add test exposing reset issue with pending callback (Fix CVE-2023-5088) +- hw/ide: reset: cancel async DMA operation before resetting state (Fix CVE-2023-5088) + * Tue Apr 23 2024 Jiabo Feng - hw/virtio/virtio-crypto: Protect from DMA re-entrancy bugs(CVE-2024-3446) - hw/char/virtio-serial-bus: Protect from DMA re-entrancy bugs(CVE-2024-3446) diff --git a/tests-qtest-ahci-test-add-test-exposing-reset-issue-.patch b/tests-qtest-ahci-test-add-test-exposing-reset-issue-.patch new file mode 100644 index 0000000..fee76e3 --- /dev/null +++ b/tests-qtest-ahci-test-add-test-exposing-reset-issue-.patch @@ -0,0 +1,132 @@ +From 3ec7685d0202787cc556f7c4c64a4d44baf78709 Mon Sep 17 00:00:00 2001 +From: Fiona Ebner +Date: Wed, 6 Sep 2023 15:09:22 +0200 +Subject: [PATCH] tests/qtest: ahci-test: add test exposing reset issue with + pending callback (Fix CVE-2023-5088) +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Before commit "hw/ide: reset: cancel async DMA operation before +resetting state", this test would fail, because a reset with a +pending write operation would lead to an unsolicited write to the +first sector of the disk. + +The test writes a pattern to the beginning of the disk and verifies +that it is still intact after a reset with a pending operation. It +also checks that the pending operation actually completes correctly. + +Signed-off-by: Fiona Ebner +Message-ID: <20230906130922.142845-2-f.ebner@proxmox.com> +Signed-off-by: Philippe Mathieu-Daudé +--- + tests/ahci-test.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++- + 1 file changed, 85 insertions(+), 1 deletion(-) + +diff --git a/tests/ahci-test.c b/tests/ahci-test.c +index 086811e602..659623fc22 100644 +--- a/tests/ahci-test.c ++++ b/tests/ahci-test.c +@@ -1426,6 +1426,89 @@ static void test_reset(void) + ahci_shutdown(ahci); + } + ++static void test_reset_pending_callback(void) ++{ ++ AHCIQState *ahci; ++ AHCICommand *cmd; ++ uint8_t port; ++ uint64_t ptr1; ++ uint64_t ptr2; ++ ++ int bufsize = 4 * 1024; ++ int speed = bufsize + (bufsize / 2); ++ int offset1 = 0; ++ int offset2 = bufsize / AHCI_SECTOR_SIZE; ++ ++ g_autofree unsigned char *tx1 = g_malloc(bufsize); ++ g_autofree unsigned char *tx2 = g_malloc(bufsize); ++ g_autofree unsigned char *rx1 = g_malloc0(bufsize); ++ g_autofree unsigned char *rx2 = g_malloc0(bufsize); ++ ++ /* Uses throttling to make test independent of specific environment. */ ++ ahci = ahci_boot_and_enable("-drive if=none,id=drive0,file=%s," ++ "cache=writeback,format=%s," ++ "throttling.bps-write=%d " ++ "-M q35 " ++ "-device ide-hd,drive=drive0 ", ++ tmp_path, imgfmt, speed); ++ ++ port = ahci_port_select(ahci); ++ ahci_port_clear(ahci, port); ++ ++ ptr1 = ahci_alloc(ahci, bufsize); ++ ptr2 = ahci_alloc(ahci, bufsize); ++ ++ g_assert(ptr1 && ptr2); ++ ++ /* Need two different patterns. */ ++ do { ++ generate_pattern(tx1, bufsize, AHCI_SECTOR_SIZE); ++ generate_pattern(tx2, bufsize, AHCI_SECTOR_SIZE); ++ } while (memcmp(tx1, tx2, bufsize) == 0); ++ ++ qtest_bufwrite(ahci->parent->qts, ptr1, tx1, bufsize); ++ qtest_bufwrite(ahci->parent->qts, ptr2, tx2, bufsize); ++ ++ /* Write to beginning of disk to check it wasn't overwritten later. */ ++ ahci_guest_io(ahci, port, CMD_WRITE_DMA_EXT, ptr1, bufsize, offset1); ++ ++ /* Issue asynchronously to get a pending callback during reset. */ ++ cmd = ahci_command_create(CMD_WRITE_DMA_EXT); ++ ahci_command_adjust(cmd, offset2, ptr2, bufsize, 0); ++ ahci_command_commit(ahci, cmd, port); ++ ahci_command_issue_async(ahci, cmd); ++ ++ ahci_set(ahci, AHCI_GHC, AHCI_GHC_HR); ++ ++ ahci_command_free(cmd); ++ ++ /* Wait for throttled write to finish. */ ++ sleep(1); ++ ++ /* Start again. */ ++ ahci_clean_mem(ahci); ++ ahci_pci_enable(ahci); ++ ahci_hba_enable(ahci); ++ port = ahci_port_select(ahci); ++ ahci_port_clear(ahci, port); ++ ++ /* Read and verify. */ ++ ahci_guest_io(ahci, port, CMD_READ_DMA_EXT, ptr1, bufsize, offset1); ++ qtest_bufread(ahci->parent->qts, ptr1, rx1, bufsize); ++ g_assert_cmphex(memcmp(tx1, rx1, bufsize), ==, 0); ++ ++ ahci_guest_io(ahci, port, CMD_READ_DMA_EXT, ptr2, bufsize, offset2); ++ qtest_bufread(ahci->parent->qts, ptr2, rx2, bufsize); ++ g_assert_cmphex(memcmp(tx2, rx2, bufsize), ==, 0); ++ ++ ahci_free(ahci, ptr1); ++ ahci_free(ahci, ptr2); ++ ++ ahci_clean_mem(ahci); ++ ++ ahci_shutdown(ahci); ++} ++ + static void test_ncq_simple(void) + { + AHCIQState *ahci; +@@ -1929,7 +2012,8 @@ int main(int argc, char **argv) + qtest_add_func("/ahci/migrate/dma/halted", test_migrate_halted_dma); + + qtest_add_func("/ahci/max", test_max); +- qtest_add_func("/ahci/reset", test_reset); ++ qtest_add_func("/ahci/reset/simple", test_reset); ++ qtest_add_func("/ahci/reset/pending_callback", test_reset_pending_callback); + + qtest_add_func("/ahci/io/ncq/simple", test_ncq_simple); + qtest_add_func("/ahci/migrate/ncq/simple", test_migrate_ncq); +-- +2.41.0.windows.1 + -- Gitee