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

Feel free to contact me if there are any questions.

Best regards,
Kaixuan

Reply via email to