qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 2/2] hw/block/nvme: fix resource leak in nvme_format_ns


From: Klaus Jensen
Subject: Re: [PATCH 2/2] hw/block/nvme: fix resource leak in nvme_format_ns
Date: Mon, 22 Mar 2021 11:48:26 +0100

On Mar 22 11:02, Max Reitz wrote:
> 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?

Hi Max,

Thanks for glossing over this.

And, yeah, you are right, nice catch. The reference counting is exactly
to guard against pwrite_zeroes invoking the CB before returning, but if
it happens it is not correctly handling it anyway :(

This stuff is exactly why I proposed converting all this into the
"proper" BlockAIOCB approach (see [1]), but it need a little more work.

I'll v2 this with a fix for this! Thanks!


  [1]: 
20210302111040.289244-1-its@irrelevant.dk/">https://lore.kernel.org/qemu-devel/20210302111040.289244-1-its@irrelevant.dk/

Attachment: signature.asc
Description: PGP signature


reply via email to

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