[PATCH 07/11] hw/ide/ahci: reject a command header with an invalid FIS length

["Denis V. Lunev" <den@openvz.org>]

From: Denis V. Lunev <den@openvz.org>

AHCI 1.3.1 defines CFL in the command header as the "Length of the
Command FIS", where "A length of '0' or '1' is illegal" and "The
maximum value allowed is 10h, or 16 DW". handle_cmd() never looks at
it, so an all-zero command header is executable: its zero tbl_addr maps
a command table at guest physical address 0, and a guest that has put a
valid Register H2D FIS there gets it run.

That is the reachability a guest gains by pointing PxCLB at an MMIO
region, where the CLB is a zero-filled bounce buffer rather than
anything the guest wrote.

Reject a header whose CFL falls outside the legal range. Nothing else
consults it; the command FIS is always mapped at its full 128 bytes.

The slot is dropped without reporting anything, as the unmappable
command table beside it already is. No PxIS bit describes a malformed
command header: HBFS is for a host bus error, "such as a bad software
pointer", which is why the short mapping below raises it and this does
not.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4043
Cc: John Snow <jsnow@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 hw/ide/ahci.c       | 9 +++++++++
 hw/ide/trace-events | 1 +
 2 files changed, 10 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 436a0eaab6..2b2ef873e0 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1334,6 +1334,7 @@ static void handle_cmd(AHCIState *s, int port, uint8_t slot)
     AHCICmdHdr *cmd;
     uint8_t *cmd_fis;
     dma_addr_t cmd_len;
+    uint8_t cfl;
 
     if (s->dev[port].port.ifs[0].status & (BUSY_STAT|DRQ_STAT)) {
         /* Engine currently busy, try again later */
@@ -1346,6 +1347,14 @@ static void handle_cmd(AHCIState *s, int port, uint8_t slot)
         return;
     }
     cmd = get_cmd_header(s, port, slot);
+
+    /* AHCI 1.3.1: a CFL below 2 dwords or above 16 is illegal */
+    cfl = le16_to_cpu(cmd->opts) & AHCI_CMD_HDR_CMD_FIS_LEN;
+    if (cfl < 2 || cfl > 16) {
+        trace_handle_cmd_badcfl(s, port, le16_to_cpu(cmd->opts));
+        return;
+    }
+
     /* remember current slot handle for later */
     s->dev[port].cur_cmd = cmd;
 
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index f1472f5852..3ab5e7bd1d 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -106,6 +106,7 @@ handle_reg_h2d_fis_res(void *s, int port, char b0, char b1, char b2) "ahci(%p)[%
 handle_cmd_busy(void *s, int port) "ahci(%p)[%d]: engine busy"
 handle_cmd_nolist(void *s, int port) "ahci(%p)[%d]: handle_cmd called without s->dev[port].lst"
 handle_cmd_badport(void *s, int port) "ahci(%p)[%d]: guest accessed unused port"
+handle_cmd_badcfl(void *s, int port, uint16_t opts) "ahci(%p)[%d]: guest provided an invalid cmd FIS length: 0x%04x"
 handle_cmd_badfis(void *s, int port) "ahci(%p)[%d]: guest provided an invalid cmd FIS"
 handle_cmd_badmap(void *s, int port, uint64_t len) "ahci(%p)[%d]: dma_memory_map failed, 0x%02"PRIx64" != 0x80"
 handle_cmd_unhandled_fis(void *s, int port, uint8_t b0, uint8_t b1, uint8_t b2) "ahci(%p)[%d]: unhandled FIS type. cmd_fis: 0x%02x-%02x-%02x"
-- 
2.53.0