Thank you for your patient response! I have submitted the new patch as
requested. The link is
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240412164441.1727089-1-lumingyindet...@163.com/
At 2024-04-12 21:41:25, "James Almer" <jamr...@gmail.com> wrote:
>On 4/12/2024 10:09 AM, LuMingYin wrote:
>> In the file ffmpeg_mux_init.c located at /FFmpeg/fftools/, a pointer
>> variable named pts is defined at line 2830. At line 2836, this pointer is
>> allocated a dynamic memory area using the function av_malloc_array. When the
>> if statement at line 2852 evaluates to true, there are two possible
>> scenarios. The first scenario is when nb_ch > INT_MAX - size. In this case,
>> the program should release the dynamic memory area pointed to by pts before
>> returning. The second scenario is when the realloc operation for the pts
>> pointer fails. In this case, since the av_realloc_f function releases the
>> original memory in case of reallocation failure, there is no need to release
>> the memory area pointed to by pts. Because the handling of these two
>> scenarios differs, to fix the memory leak issue caused by the first
>> scenario, my patch separates the treatment of these cases.
>>
>> Signed-off-by: LuMingYin <lumingyindet...@163.com>
>> ---
>> fftools/ffmpeg_mux_init.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
>> index 6d8bd5bcdf..1f27e4e4f4 100644
>> --- a/fftools/ffmpeg_mux_init.c
>> +++ b/fftools/ffmpeg_mux_init.c
>> @@ -2849,8 +2849,12 @@ static int parse_forced_key_frames(void *log,
>> KeyframeForceCtx *kf,
>> unsigned int nb_ch = mux->fc->nb_chapters;
>> int j;
>>
>> - if (nb_ch > INT_MAX - size ||
>> - !(pts = av_realloc_f(pts, size += nb_ch - 1,
>> + if (nb_ch > INT_MAX - size) {
>> + av_freep(&pts);
>> + return AVERROR(ENOMEM);
>
>There's a fail label at the end that frees pts. You should set ret to
>AVERROR(ENOMEM) and goto it.
>
>An alternative could be something like
>
>> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
>> index 1d0a210450..0d93f0eff2 100644
>> --- a/fftools/ffmpeg_mux_init.c
>> +++ b/fftools/ffmpeg_mux_init.c
>> @@ -2959,10 +2959,11 @@ static int parse_forced_key_frames(void *log,
>> KeyframeForceCtx *kf,
>> unsigned int nb_ch = mux->fc->nb_chapters;
>> int j;
>>
>> - if (nb_ch > INT_MAX - size ||
>> - !(pts = av_realloc_f(pts, size += nb_ch - 1,
>> - sizeof(*pts))))
>> - return AVERROR(ENOMEM);
>> + if (nb_ch > (INT_MAX - size) / sizeof(*pts) ||
>> + av_reallocp(&pts, (size += nb_ch - 1) * sizeof(*pts))) {
>> + ret = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>>
>> if (p[8]) {
>> ret = av_parse_time(&t, p + 8, 1);
>
>Where av_reallocp() will set pts to NULL after freeing, so the free in
>the fail label will be a no-op, and we always jump to fail on failure in
>case it's extended to do further cleaning.
>_______________________________________________
>ffmpeg-devel mailing list
>ffmpeg-devel@ffmpeg.org
>https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>To unsubscribe, visit link above, or email
>ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
