[FFmpeg-devel] [PATCH v2 3/7] avformat/hlsenc: Check some unchecked allocations
Steven Liu
lq at chinaffmpeg.org
Fri May 8 14:12:25 EEST 2020
> 2020年5月8日 下午7:07,Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道:
>
> Steven Liu:
>>
>>
>>> 2020年4月9日 下午8:58,Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道:
>>>
>>> Steven Liu:
>>>>
>>>>
>>>>> 2020年4月9日 下午5:55,Steven Liu <lq at chinaffmpeg.org> 写道:
>>>>>
>>>>>
>>>>>
>>>>>> 2020年4月9日 下午5:48,Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道:
>>>>>>
>>>>>> Steven Liu:
>>>>>>>
>>>>>>>
>>>>>>>> 2020年2月28日 下午5:53,Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道:
>>>>>>>>
>>>>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>>>>>>> ---
>>>>>>>> libavformat/hlsenc.c | 13 ++++++++++++-
>>>>>>>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>>>>>>> index a281c379f0..18f40ff3ed 100644
>>>>>>>> --- a/libavformat/hlsenc.c
>>>>>>>> +++ b/libavformat/hlsenc.c
>>>>>>>> @@ -1610,6 +1610,8 @@ static int hls_start(AVFormatContext *s, VariantStream *vs)
>>>>>>>> if (c->use_localtime_mkdir) {
>>>>>>>> const char *dir;
>>>>>>>> char *fn_copy = av_strdup(oc->url);
>>>>>>>> + if (!fn_copy)
>>>>>>>> + return AVERROR(ENOMEM);
>>>>>>>> dir = av_dirname(fn_copy);
>>>>>>>> if (ff_mkdir_p(dir) == -1 && errno != EEXIST) {
>>>>>>>> av_log(oc, AV_LOG_ERROR, "Could not create directory %s with use_localtime_mkdir\n", dir);
>>>>>>>> @@ -1770,6 +1772,8 @@ static int validate_name(int nb_vs, const char *fn)
>>>>>>>> }
>>>>>>>>
>>>>>>>> fn_dup = av_strdup(fn);
>>>>>>>> + if (!fn_dup)
>>>>>>>> + return AVERROR(ENOMEM);
>>>>>>>> filename = av_basename(fn);
>>>>>>>> subdir_name = av_dirname(fn_dup);
>>>>>>>>
>>>>>>>> @@ -2139,6 +2143,8 @@ static int update_master_pl_info(AVFormatContext *s)
>>>>>>>> int ret = 0;
>>>>>>>>
>>>>>>>> fn1 = av_strdup(s->url);
>>>>>>>> + if (!fn1)
>>>>>>>> + return AVERROR(ENOMEM);
>>>>>>> It’s unnecessary here,
>>>>>>> I have checked all strdup return checker in hlsenc some month ago, and double check the workflow in update_master_pl_info,
>>>>>>> It's the safe whether you check the strdup or not.
>>>>>>> Reference commit id: 61aa77272a25d83e5ce5c63d93c64bb9a3e15557
>>>>>>
>>>>>> If these strdups fail, the relevant dirnames will be wrong. While you
>>>>>> don't get segfaults, you will not create the files at the right
>>>>>> destinations. We should rather error out instead.
>>>>>>
>>>>> Maybe need reproduce the problem it let me understand the exception.
>>>>> I need one example, command line is ok.
>>>>>
>>>>>
>>>> I accept the 61aa77272a2 because i don’t know how to reproduce the exception to objection it. I have no example to objection it.
>>>>
>>> Command line:
>>> ffmpeg -i <Input file> -c copy -t 1:30 -f hls -hls_time 2
>>> -hls_playlist_type event -master_pl_name index.m3u8
>>> -hls_segment_filename stream_%v/data%06d.ts -use_localtime_mkdir 1
>>> -var_stream_map 'v:0,a:0' test/test%v/stream.m3u8
>>> (My input file simply had H.264 video and AC-3 audio, but I don't think
>>> that is important.)
>>>
>>> If I run this normally, I get two subfolders in the current directory:
>>> stream_0 and test. stream_0 contains various transport streams, test
>>> contains index.m3u8 and a folder test0 which contains stream.m3u8.
>>>
>>> If I simulate memory allocation failure by replacing the first strdup in
>>> update_master_pl_info() with 'fn1 = NULL;' (instead of 'fn1 =
>>> av_strdup(s->url);'), then the index.m3u8 file is not created in the
>>> test subfolder, but in the current directory.
>> Yes, you are right, I have not reproduced by this way, I double checked this way, it really have the problem,
>>
>>> (If you believed that av_strdup() failures don't affect the outcome,
>>> then you could have replaced all 'temp = av_strdup(str); dir =
>>> av_dirname(temp);' with 'dir = ".";'.)
>>>
>>> - Andreas
>>>
>>> PS: What about the rest of this patchset?
>> I have no option for other patch, only this one, because i remember Limin Wang submit one patch about this patch and I cannot not reproduced the problem which use the same way above that time, maybe I forget something i not attention. So just asked you at this patch.
>> I merge that patch because:
>> 1. I cannot reproduce the bug, so have no reason to object it.
>> 2. The other part maybe exit if strdup failed. (Real no memory for other part.)
>
> And what about the other patches of this patchset? Do you object to me
> applying them? I'll apply them tomorrow if you don't object.
of course, you can apply them ;)
>
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
Thanks
Steven Liu
More information about the ffmpeg-devel
mailing list