From: Kaixuan Li <[email protected]>
In nvme_abort(), the submission queue pointer is dereferenced from the
guest-controlled sqid before validating it with nvme_check_sqid():
NvmeSQueue *sq = n->sq[sqid];
Since sqid is a 16-bit value (range 0-65535) taken directly from CDW10,
and n->sq[] is typically only max_ioqpairs+1 (65) entries, a malicious
guest can trigger an out-of-bounds heap read by sending an Abort command
with a large sqid.
ASan reports this as heap-buffer-overflow in nvme_abort.
Fix this by moving the array dereference to after the nvme_check_sqid()
bounds validation.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3348
Fixes: 75209c071a ("hw/nvme: actually implement abort")
Cc: [email protected]
Signed-off-by: Kaixuan Li <[email protected]>
Signed-off-by: Klaus Jensen <[email protected]>
---
hw/nvme/ctrl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index cc4593cd427a..be6c7028cb58 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6111,7 +6111,7 @@ static uint16_t nvme_abort(NvmeCtrl *n, NvmeRequest *req)
{
uint16_t sqid = le32_to_cpu(req->cmd.cdw10) & 0xffff;
uint16_t cid = (le32_to_cpu(req->cmd.cdw10) >> 16) & 0xffff;
- NvmeSQueue *sq = n->sq[sqid];
+ NvmeSQueue *sq;
NvmeRequest *r, *next;
int i;
@@ -6120,6 +6120,8 @@ static uint16_t nvme_abort(NvmeCtrl *n, NvmeRequest *req)
return NVME_INVALID_FIELD | NVME_DNR;
}
+ sq = n->sq[sqid];
+
if (sqid == 0) {
for (i = 0; i < n->outstanding_aers; i++) {
NvmeRequest *re = n->aer_reqs[i];
--
2.53.0
