[FFmpeg-devel] [PATCH 3/8] avfilter/af_channelmap: fix memory leak

[Ganesh Ajjanagadde]

On Sat, Dec 5, 2015 at 3:55 PM, Ganesh Ajjanagadde
<gajjanagadde at gmail.com> wrote:
> On Sat, Dec 5, 2015 at 3:50 PM, Paul B Mahol <onemda at gmail.com> wrote:
>> On 12/4/15, Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>>> Recent commits 6aaac24d72a7da631173209841a3944fcb4a3309 and
>>> 3835554bf8ed78539a3492c239f979c0ab03a15f made progress towards cleaning
>>> up usage of the formats API, and in particular fixed possible NULL pointer
>>> dereferences.
>>>
>>> This commit addresses the issue of possible resource leaks when some
>>> intermediate
>>> call fails.
>>>
>>> Tested with valgrind --leak-check=full --show-leak-kinds=all, and manual
>>> simulation
>>> of malloc/realloc failures.
>>>
>>> Fixes: CID 1338330.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>> ---
>>>  libavfilter/af_channelmap.c | 10 +++++++++-
>>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
>>> index 9e95a98..dfe3d48 100644
>>> --- a/libavfilter/af_channelmap.c
>>> +++ b/libavfilter/af_channelmap.c
>>> @@ -292,14 +292,22 @@ static int channelmap_query_formats(AVFilterContext
>>> *ctx)
>>>      int ret;
>>>
>>>      layouts = ff_all_channel_layouts();
>>> +    if (!layouts) {
>>> +        ret = AVERROR(ENOMEM);
>>
>> Consider this: ff_all_channel_layouts returns NULL.
>>
>>> +        goto fail;
>>
>> Ok, we do not return immediately but use gotos, whatever...
>>
>>> +    }
>>>      if ((ret = ff_add_channel_layout     (&channel_layouts,
>>> s->output_layout                    )) < 0 ||
>>>          (ret = ff_set_common_formats     (ctx             ,
>>> ff_planar_sample_fmts()             )) < 0 ||
>>>          (ret = ff_set_common_samplerates (ctx             ,
>>> ff_all_samplerates()                )) < 0 ||
>>>          (ret = ff_channel_layouts_ref    (layouts         ,
>>> &ctx->inputs[0]->out_channel_layouts)) < 0 ||
>>>          (ret = ff_channel_layouts_ref    (channel_layouts ,
>>> &ctx->outputs[0]->in_channel_layouts)) < 0)
>>> -        return ret;
>>> +            goto fail;
>>>
>>>      return 0;
>>> +fail:
>>> +    av_freep(&layouts->channel_layouts);
>>
>> What happens here if layouts is NULL ?
>
> Clement has asked this for another one of these, and I replied there
> saying that I fixed it locally, and that such a remark applies to all
> other patches sent in the set. I did not want to ping all the other
> patches. Nevertheless, thanks for pointing it out.

pushed, thanks

>
>>
>>> +    av_freep(&layouts);
>>> +    return ret;
>>>  }
>>>
>>>  static int channelmap_filter_frame(AVFilterLink *inlink, AVFrame *buf)
>>> --
>>> 2.6.3
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>