Re: [Qemu-devel] [Qemu-block] [PATCH 1/3] blockjob: fix dead pointer in txn list

[John Snow <jsnow@redhat.com>]

On 07/27/2016 06:49 AM, Vladimir Sementsov-Ogievskiy wrote:
> Job may be freed in block_job_unref and in this case this would break
> transaction QLIST.
>
> Fix this by removing job from this list before unref.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  blockjob.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/blockjob.c b/blockjob.c
> index a5ba3be..e045091 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -216,6 +216,7 @@ static void block_job_completed_single(BlockJob *job)
>      }
>      job->cb(job->opaque, job->ret);
>      if (job->txn) {
> +        QLIST_REMOVE(job, txn_list);
>          block_job_txn_unref(job->txn);
>      }
>      block_job_unref(job);
>

Has this caused actual problems for you?

This function is only ever called in a transactional context if the 
transaction is over -- so we're not likely to use the pointers ever 
again anyway.

Still, it's good practice, and the caller uses a safe iteration of the 
list, so I think this should be safe.

But I don't think this SHOULD fix an actual bug. If it does, I think 
something else is wrong.

Tested-by: John Snow <jsnow@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>