On 22 October 2016 at 13:09, P J P <ppan...@redhat.com> wrote: > From: Prasad J Pandit <p...@fedoraproject.org> > > NVME Express Controller has two queues, submission & completion > queue. When creating a new queue object, 'nvme_create_sq' and > 'nvme_create_cq' routines incorrectly check the queue id field. > It could lead to an OOB access issue. Correct the queue id check > to avoid it. > > Reported-by: Qinghao Tang <luodalon...@gmail.com> > Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> > --- > hw/block/nvme.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 2ded247..61bdc9d 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -373,7 +373,7 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeCmd *cmd) > if (!cqid || nvme_check_cqid(n, cqid)) { > return NVME_INVALID_CQID | NVME_DNR; > } > - if (!sqid || (sqid && !nvme_check_sqid(n, sqid))) { > + if (!sqid || nvme_check_sqid(n, sqid)) { > return NVME_INVALID_QID | NVME_DNR; > } > if (!qsize || qsize > NVME_CAP_MQES(n->bar.cap)) { > @@ -447,7 +447,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd) > uint16_t qflags = le16_to_cpu(c->cq_flags); > uint64_t prp1 = le64_to_cpu(c->prp1); > > - if (!cqid || (cqid && !nvme_check_cqid(n, cqid))) { > + if (!cqid || nvme_check_cqid(n, cqid)) { > return NVME_INVALID_CQID | NVME_DNR; > } > if (!qsize || qsize > NVME_CAP_MQES(n->bar.cap)) {
This looks a bit weird. Firstly, it doesn't match the commit message, because it's still going to call nvme_check_cqid() and nvme_check_sqid() in the same situations, so if the commit message is right and this is going to cause a problem then the change doesn't seem to be sufficient. Secondly, it's almost the same as this cleanup patch from Thomas Huth that's already in qemu-trivial: http://patchwork.ozlabs.org/patch/681349/ except that your version is removing the ! negations from the return value. Can you explain in a bit more detail what the bug is here and why this is the right fix for it? thanks -- PMM