Re: [PATCH v2 2/2] hw/block/nvme: fix ref counting in nvme_format_ns

[Gollu Appalanaidu <anaidu.gollu@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

On Mon, Mar 22, 2021 at 01:09:44PM +0100, Klaus Jensen wrote:
>From: Klaus Jensen <k.jensen@samsung.com>
>
>Max noticed that since blk_aio_pwrite_zeroes() may invoke the callback
>before returning, the callbacks will never see *count == 0 and thus
>never free the count variable or decrement num_formats causing a CQE to
>never be posted.
>
>Coverity (CID 1451082) also picked up on the fact that count would not
>be free'ed if the namespace was of zero size.
>
>Fix both of these issues by explicitly checking *count and finalize for
>the given namespace if --(*count) is zero. Enqueing a CQE if there are
>no AIOs outstanding after this case is already handled by nvme_format()
>by inspecting *num_formats.
>
>Reported-by: Max Reitz <mreitz@redhat.com>
>Reported-by: Coverity (CID 1451082)
>Fixes: dc04d25e2f3f ("hw/block/nvme: add support for the format nvm command")
>Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
>---
> hw/block/nvme.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>index 6842b01ab58b..c54ec3c9523c 100644
>--- a/hw/block/nvme.c
>+++ b/hw/block/nvme.c
>@@ -5009,9 +5009,15 @@ static uint16_t nvme_format_ns(NvmeCtrl *n, NvmeNamespace *ns, uint8_t lbaf,
>
>     }
>
>-    (*count)--;
>+    if (--(*count)) {
>+        return NVME_NO_COMPLETE;
>+    }
>
>-    return NVME_NO_COMPLETE;
>+    g_free(count);
>+    ns->status = 0x0;
>+    (*num_formats)--;
>+
>+    return NVME_SUCCESS;
> }
Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>
> static uint16_t nvme_format(NvmeCtrl *n, NvmeRequest *req)
>-- 
>2.31.0
>
>

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]