[Top][All Lists]

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

Re: [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when drai

From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS
Date: Fri, 10 Nov 2017 17:13:52 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

On 2017-11-10 17:05, Kevin Wolf wrote:
> Am 10.11.2017 um 16:23 hat Max Reitz geschrieben:
>> On 2017-11-10 14:32, Fam Zheng wrote:
>>> On Fri, 11/10 14:17, Kevin Wolf wrote:
>>>> Do you actually need to keep references to all BDSes in the whole list
>>>> while using the iterator or would it be enough to just keep a reference
>>>> to the current one?
>>> To fix the bug we now see I think keeping the current is enough, but I think
>>> implementing just like this patch is also good with some future-proofing: we
>>> cannot know what will be wedged into the nexted aio_poll()'s over time (and 
>>> yes,
>>> we should really reduce the number of them.)
>> I don't really want to think about whether it's safe to only keep a
>> reference to the current BDS.  I can't imagine any case where destroying
>> one root BDS leads to destroying another, but I'd rather be safe and not
>> have to think about it.  (Unless there is an important reason to only
>> keep a strong reference to the current one.)
> Why would it be a problem if another BDS from the list went away? If it
> is one that was already processed, we don't care, and if it was in the
> yet unprocessed part of the list, we'll just never return it.

You mean from bdrv_next() in its current form?  Well, I know that when I
just put a bdrv_ref()/bdrv_unref() pair around the drain, I got a
segfault in blk_all_next() in bdrv_next().  I can investigate more, but
that's pretty much what I mean by "I don't really want to think about it".

So that's why I want bdrv_next() to copy all BDS into another list
instead of iterating through them on the fly.  And if we do that, a
disappearing BDS of course is an issue because we don't notice until
we're trying to iterate over it, at which point we have a use-after-free.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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