|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple() |
Date: | Mon, 3 May 2021 16:09:07 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 |
03.05.2021 15:41, Kevin Wolf wrote:
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.
Oops. Somehow I decided they are set in _prepare.
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.
agree now.. Still, don't we miss these two qobject_unref() calls on success path? -- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |