On 7 June 2017 at 18:50, Kevin Wolf <[email protected]> wrote:
> The final bdrv_set_backing_hd() could be working on already freed nodes
> because the commit job drops its references (through BlockBackends) to
> both overlay_bs and top already a bit earlier.
>
> One way to trigger the bug is hot unplugging a disk for which
> blockdev_mark_auto_del() cancels the block job.
>
> Fix this by taking BDS-level references while we're still using the
> nodes.
>
> Signed-off-by: Kevin Wolf <[email protected]>
> Reviewed-by: John Snow <[email protected]>
> ---
> block/commit.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/block/commit.c b/block/commit.c
> index a3028b2..af6fa68 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -89,6 +89,10 @@ static void commit_complete(BlockJob *job, void *opaque)
> int ret = data->ret;
> bool remove_commit_top_bs = false;
>
> + /* Make sure overlay_bs and top stay around until bdrv_set_backing_hd()
> */
> + bdrv_ref(top);
> + bdrv_ref(overlay_bs);
> +
> /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before
> * the normal backing chain can be restored. */
> blk_unref(s->base);
Hi -- coverity complains about this change, because bdrv_ref()
assumes that its argument is not NULL, but later on in commit_complete()
we have a check
"if (overlay_bs && ...)"
which assumes its argument might be NULL. (CID 1376205)
Which is correct?
thanks
-- PMM
