From: Simon Rowe @ 2023-09-21 16:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: John Snow, qemu-block, f.ebner, Simon Rowe, Felipe Franciosi

When an IDE controller is reset, its internal state is being cleared
before any outstanding I/O is cancelled. If a response to DMA is
received in this window, the aio callback will incorrectly continue
with the next part of the transfer (now using sector 0 from
the cleared controller state).

For a write operation, this results in user data being written to the
MBR, replacing the first stage bootloader and/or partition table. A
malicious user could exploit this bug to first read the MBR and then
rewrite it with user-controller bootloader code.

This addresses the bug by checking if DRQ_STAT is still set in the DMA
callback (as it is otherwise cleared at the start of the bus
reset). If it is not, treat the transfer as ended.

This only appears to affect SATA controllers, plain IDE does not use
aio.

Fixes: CVE-2023-5088
Signed-off-by: Simon Rowe <[email protected]>
Cc: Felipe Franciosi <[email protected]>
---
 hw/ide/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index b5e0dcd29b..826b7eaeeb 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -906,8 +906,12 @@ static void ide_dma_cb(void *opaque, int ret)
         s->nsector -= n;
     }
 
-    /* end of transfer ? */
-    if (s->nsector == 0) {
+    /*
+     * End of transfer ?
+     * If a bus reset occurs immediately before the callback is invoked the
+     * bus state will have been cleared. Terminate the transfer.
+     */
+    if (s->nsector == 0 || !(s->status & DRQ_STAT)) {
         s->status = READY_STAT | SEEK_STAT;
         ide_bus_set_irq(s->bus);
         goto eot;
-- 
2.22.3