[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH] block: nvme: correct the nvme queu

From: P J P
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH] block: nvme: correct the nvme queue id check
Date: Sun, 23 Oct 2016 10:38:58 +0530 (IST)

+-- On Sat, 22 Oct 2016, Peter Maydell wrote --+
| 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?

   nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
       return sqid < n->num_queues && n->sq[sqid] != NULL ? 0 : -1;

Both 'nvme_check_sqid' and 'nvme_check_cqid' return zero(0) when the 
respective queue id's are valid and -1 when they are invalid. The '!' was 
causing it to return invalid QID error when the check function returned zero.

The code seems incosistent quite a bit. The current check returns invalid QID 
error for '!cqid' and '!sqid', but these IDs are used as index into 'n->cq' 
array, not sure why index zero(0) is invalid. But then I found this in the 
NVME specification[0]

   "...One entry in each queue is not available for use due 
     to Head and Tail entry pointer definition."

Not sure if this one entry is at index zero(0). If so, there are other calls 
to the 'nvme_check' functions which are not acompanyed with '!cqid' or 
'!sqid' checks. Ex.

        qid = (addr - 0x1000) >> 3;
        if (nvme_check_sqid(n, qid)) {

        if (!nvme_check_cqid(n, sq->cqid)) {
            cq = n->cq[sq->cqid];

I haven't check these in further details yet. When 'cqid' is invalid it 
returns 'NVME_INVALID_CQID' but when 'sqid' is invalid it returns 
'NVME_INVALID_QID'. Not sure why not use 'NVME_INVALID_QID' for both.


Thank you.
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

reply via email to

[Prev in Thread] Current Thread [Next in Thread]