Re: [Qemu-devel] [PATCH] blockjob: update nodes head while removing all bdrv

[Sergio Lopez <slp@redhat.com>]

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


Max Reitz <mreitz@redhat.com> writes:

> On 10.09.19 13:07, Sergio Lopez wrote:
>> block_job_remove_all_bdrv() iterates through job->nodes, calling
>> bdrv_root_unref_child() for each entry. The call to the latter may
>> reach child_job_[can_]set_aio_ctx(), which will also attempt to
>> traverse job->nodes, potentially finding entries that where freed
>> on previous iterations.
>> 
>> To avoid this situation, update job->nodes head on each iteration to
>> ensure that already freed entries are no longer linked to the list.
>> 
>> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1746631
>> Signed-off-by: Sergio Lopez <slp@redhat.com>
>> ---
>>  blockjob.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/blockjob.c b/blockjob.c
>> index 6e32d1a0c0..7b1551d981 100644
>> --- a/blockjob.c
>> +++ b/blockjob.c
>> @@ -192,6 +192,12 @@ void block_job_remove_all_bdrv(BlockJob *job)
>>          BdrvChild *c = l->data;
>>          bdrv_op_unblock_all(c->bs, job->blocker);
>>          bdrv_root_unref_child(c);
>> +        /*
>> +         * The call above may reach child_job_[can_]set_aio_ctx(), which will
>> +         * also traverse job->nodes, so update the head here to make sure it
>> +         * doesn't attempt to process an already freed BdrvChild.
>> +         */
>> +        job->nodes = l->next;
>>      }
>>      g_slist_free(job->nodes);
>
> But this will leak the whole list.

Ouch. This is what happens when you rush up things. I'll send a v2 ASAP.

Thanks,
Sergio.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]