|
From: | Max Reitz |
Subject: | Re: [PATCH 2/2] hw/block/nvme: fix resource leak in nvme_format_ns |
Date: | Mon, 22 Mar 2021 11:02:21 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 |
On 22.03.21 07:19, Klaus Jensen wrote:
From: Klaus Jensen <k.jensen@samsung.com> In nvme_format_ns(), if the namespace is of zero size (which might be useless, but not invalid), the `count` variable will leak. Fix this by returning early in that case.
When looking at the Coverity report, something else caught my eye: As far as I’m aware, blk_aio_pwrite_zeroes() may invoke the CB before returning (if blk_do_pwritev_part() returns without yielding). I don’t think that will happen with real hardware (who knows, though), but it should be possible to see with the null-co block driver.
nvme_format_ns() doesn’t quite look like it takes that into account. For example, because *count starts at 1 and is decremented after the while (len) loop, all nvme_aio_format_cb() invocations (if they are invoked before their blk_aio_pwrite_zeroes() returns) will see
*count == 2, and thus not free it, or call nvme_enqueue_req_completion().I don’t know whether the latter is problematic, but not freeing `count` doesn’t seem right. Perhaps this could be addressed by adding a condition to the `(*count)--` to see whether `(*count)-- == 1` (or rather `--(*count) == 0`), which would indicate that there are no AIO functions still in flight?
Max
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 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 6842b01ab58b..dad275971a84 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -4984,6 +4984,11 @@ static uint16_t nvme_format_ns(NvmeCtrl *n, NvmeNamespace *ns, uint8_t lbaf, ns->status = NVME_FORMAT_IN_PROGRESS;len = ns->size;+ + if (!len) { + return NVME_SUCCESS; + } + offset = 0;count = g_new(int, 1);
[Prev in Thread] | Current Thread | [Next in Thread] |