On Mar 19 17:12, Kaixuan Li wrote: > Dear Sir or Madam, > > Since my institution email address doesn't allow git email, I sent the > patch manually as follows: > > 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]> > --- > 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 cc4593cd42..be6c7028cb 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.34.1 >
Hi, Thanks! I'll queue it up on nvme-next. Klaus
signature.asc
Description: PGP signature
