[FFmpeg-devel] [PATCH 1/8] avfilter/vf_overlay: fix memory leaks
Ganesh Ajjanagadde
gajjanag at mit.edu
Thu Dec 10 13:54:31 CET 2015
On Thu, Dec 10, 2015 at 3:47 AM, Paul B Mahol <onemda at gmail.com> wrote:
> On 12/10/15, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>> On Wed, Dec 9, 2015 at 6:09 PM, Paul B Mahol <onemda at gmail.com> wrote:
>>> On 12/9/15, Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>>>> On Fri, Dec 4, 2015 at 9:39 AM, 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 1338327.
>>>>>
>>>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>>>> ---
>>>>> libavfilter/vf_overlay.c | 32 +++++++++++++++++++++++---------
>>>>> 1 file changed, 23 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c
>>>>> index 3c61731..68cfb1b 100644
>>>>> --- a/libavfilter/vf_overlay.c
>>>>> +++ b/libavfilter/vf_overlay.c
>>>>> @@ -252,23 +252,31 @@ static int query_formats(AVFilterContext *ctx)
>>>>> switch (s->format) {
>>>>> case OVERLAY_FORMAT_YUV420:
>>>>> if (!(main_formats =
>>>>> ff_make_format_list(main_pix_fmts_yuv420)) ||
>>>>> - !(overlay_formats =
>>>>> ff_make_format_list(overlay_pix_fmts_yuv420)))
>>>>> - return AVERROR(ENOMEM);
>>>>> + !(overlay_formats =
>>>>> ff_make_format_list(overlay_pix_fmts_yuv420))) {
>>>>> + ret = AVERROR(ENOMEM);
>>>>> + goto fail;
>>>>> + }
>>>>> break;
>>>>> case OVERLAY_FORMAT_YUV422:
>>>>> if (!(main_formats =
>>>>> ff_make_format_list(main_pix_fmts_yuv422)) ||
>>>>> - !(overlay_formats =
>>>>> ff_make_format_list(overlay_pix_fmts_yuv422)))
>>>>> - return AVERROR(ENOMEM);
>>>>> + !(overlay_formats =
>>>>> ff_make_format_list(overlay_pix_fmts_yuv422))) {
>>>>> + ret = AVERROR(ENOMEM);
>>>>> + goto fail;
>>>>> + }
>>>>> break;
>>>>> case OVERLAY_FORMAT_YUV444:
>>>>> if (!(main_formats =
>>>>> ff_make_format_list(main_pix_fmts_yuv444)) ||
>>>>> - !(overlay_formats =
>>>>> ff_make_format_list(overlay_pix_fmts_yuv444)))
>>>>> - return AVERROR(ENOMEM);
>>>>> + !(overlay_formats =
>>>>> ff_make_format_list(overlay_pix_fmts_yuv444))) {
>>>>> + ret = AVERROR(ENOMEM);
>>>>> + goto fail;
>>>>> + }
>>>>> break;
>>>>> case OVERLAY_FORMAT_RGB:
>>>>> if (!(main_formats = ff_make_format_list(main_pix_fmts_rgb))
>>>>> ||
>>>>> - !(overlay_formats =
>>>>> ff_make_format_list(overlay_pix_fmts_rgb)))
>>>>> - return AVERROR(ENOMEM);
>>>>> + !(overlay_formats =
>>>>> ff_make_format_list(overlay_pix_fmts_rgb))) {
>>>>> + ret = AVERROR(ENOMEM);
>>>>> + goto fail;
>>>>> + }
>>>>> break;
>>>>> default:
>>>>> av_assert0(0);
>>>>> @@ -277,9 +285,15 @@ static int query_formats(AVFilterContext *ctx)
>>>>> if ((ret = ff_formats_ref(main_formats ,
>>>>> &ctx->inputs[MAIN]->out_formats )) < 0 ||
>>>>> (ret = ff_formats_ref(overlay_formats,
>>>>> &ctx->inputs[OVERLAY]->out_formats)) < 0 ||
>>>>> (ret = ff_formats_ref(main_formats ,
>>>>> &ctx->outputs[MAIN]->in_formats )) < 0)
>>>>> - return ret;
>>>>> + goto fail;
>>>>>
>>>>> return 0;
>>>>> +fail:
>>>>> + av_freep(&main_formats->formats);
>>>>> + av_freep(&main_formats);
>>>>> + av_freep(&overlay_formats->formats);
>>>>> + av_freep(&overlay_formats);
>>>>> + return ret;
>>>>> }
>>>>>
>>>>> static const enum AVPixelFormat alpha_pix_fmts[] = {
>>>>> --
>>>>> 2.6.3
>>>>>
>>>>
>>>> pushed, with the necessary modification described by Clement
>>>
>>> This tries to dereference uninitialized value.
>>
>> See right above; I did the desired modification and believe there is no
>> issue.
>> I might be wrong; but if so, could you please refer to the relevant
>> cvslog message?
>> Thanks.
>
> libavfilter/vf_overlay.c:275:13: warning: variable 'overlay_formats'
> is used uninitialized whenever '||' condition is true
> [-Wsometimes-uninitialized]
> if (!(main_formats = ff_make_format_list(main_pix_fmts_rgb)) ||
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:295:9: note: uninitialized use occurs here
> if (overlay_formats)
> ^~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:275:13: note: remove the '||' if its
> condition is always false
> if (!(main_formats = ff_make_format_list(main_pix_fmts_rgb)) ||
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:268:13: warning: variable 'overlay_formats'
> is used uninitialized whenever '||' condition is true
> [-Wsometimes-uninitialized]
> if (!(main_formats = ff_make_format_list(main_pix_fmts_yuv444)) ||
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:295:9: note: uninitialized use occurs here
> if (overlay_formats)
> ^~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:268:13: note: remove the '||' if its
> condition is always false
> if (!(main_formats = ff_make_format_list(main_pix_fmts_yuv444)) ||
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:261:13: warning: variable 'overlay_formats'
> is used uninitialized whenever '||' condition is true
> [-Wsometimes-uninitialized]
> if (!(main_formats = ff_make_format_list(main_pix_fmts_yuv422)) ||
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:295:9: note: uninitialized use occurs here
> if (overlay_formats)
> ^~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:261:13: note: remove the '||' if its
> condition is always false
> if (!(main_formats = ff_make_format_list(main_pix_fmts_yuv422)) ||
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:254:13: warning: variable 'overlay_formats'
> is used uninitialized whenever '||' condition is true
> [-Wsometimes-uninitialized]
> if (!(main_formats = ff_make_format_list(main_pix_fmts_yuv420)) ||
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:295:9: note: uninitialized use occurs here
> if (overlay_formats)
> ^~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:254:13: note: remove the '||' if its
> condition is always false
> if (!(main_formats = ff_make_format_list(main_pix_fmts_yuv420)) ||
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> libavfilter/vf_overlay.c:249:37: note: initialize the variable
> 'overlay_formats' to silence this warning
> AVFilterFormats *overlay_formats;
> ^
> = NULL
> 4 warnings generated.
Wish my GCC had shown this, thanks. Fixed and pushed.
>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list