[FFmpeg-devel] [PATCH 16/21] avfilter/af_channelmap: Fix double-free of AVFilterChannelLayouts on error

[Nicolas George]

Andreas Rheinhardt (12020-08-09):
> The query_formats function of the channelmap filter tries to allocate
> a list of channel layouts which on success are attached to more permanent
> objects (an AVFilterLink) for storage afterwards. If attaching succeeds,
> the link becomes one of the common owners (in this case, the only owner)
> of the list. Yet if the list has been successfully attached to the link
> and an error happens lateron, the list was manually freed, which is wrong,
> because it is owned by its link so that the link's pointer to the list will
> become dangling and there will be a double-free/use-after-free when the link
> is later cleaned up automatically.
> 
> This commit fixes this by removing the custom freeing code; this will
> temporarily add a leaking codepath (if attaching the list fails, the list
> will leak), but this will be fixed soon by making sure that an
> AVFilterChannelLayouts without owner will be automatically freed when
> attaching it to an AVFilterLink fails.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavfilter/af_channelmap.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)

Patches 8-16 LGTM.

As a matter of style, I would like it better if the last case was
handled the same way as the others rather than using the final return,
i.e., instead of:

    if ((ret = a()) < 0 ||
        (ret = b()) < 0)
        return ret;
    return c();

I like better :

    if ((ret = a()) < 0 ||
        (ret = b()) < 0 ||
        (ret = c()) < 0)
        return ret;
    return 0;

But it is minor and mostly a matter of taste.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200823/9e2df645/attachment.sig>