Am 10.11.2017 um 17:13 hat Max Reitz geschrieben: > 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".
No, I mean a bdrv_next() that is modified to bdrv_ref() only what it->blk/it->bs point to currently instead of allocating a whole list. Kevin > 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. > > Max
signature.asc
Description: PGP signature
