Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

[Kevin Wolf]

Am 03.05.2021 um 13:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 03.05.2021 14:05, Kevin Wolf wrote:
> > Like other error paths, this one needs to call tran_finalize() and clean
> > up the BlockReopenQueue, too.
> 
> We don't need the "abort" loop on that path. And clean-up of
> BlockReopenQueue is at "cleanup:" label.

The cleanup of the BlockReopenQueue itself is there, but not of all
fields in it. Specifically, these lines are needed:

    qobject_unref(bs_entry->state.explicit_options);
    qobject_unref(bs_entry->state.options);

The references are taken in bdrv_reopen_queue_child() and either used in
commit or released on abort. Doing nothing with them leaks them.

> So I'd prefer Peter's suggestion (my "[PATCH 2/6] block:
> bdrv_reopen_multiple(): fix leak of tran object")

I don't like it because I think every call that doesn't end in a commit,
should jump to the abort label to make sure that everything that remains
unused because of this is correctly cleaned up.

Kevin