[FFmpeg-devel,v1,6/8] avformat/mtv: check av_strdup() return value and fix memleak

Submitted by Steven Liu on Oct. 10, 2019, 3:40 a.m.

Details

Message ID 20191010034011.1205-6-lq@chinaffmpeg.org
State New
Headers show

Commit Message

Steven Liu Oct. 10, 2019, 3:40 a.m.
Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/mtv.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Zhao Zhili Oct. 10, 2019, 4:53 a.m.
> On Oct 10, 2019, at 11:40 AM, Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
> libavformat/mtv.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/mtv.c b/libavformat/mtv.c
> index 728f4a4781..c664ae14c7 100644
> --- a/libavformat/mtv.c
> +++ b/libavformat/mtv.c
> @@ -171,13 +171,19 @@ static int mtv_read_header(AVFormatContext *s)
>     st->codecpar->width           = mtv->img_width;
>     st->codecpar->height          = mtv->img_height;
>     st->codecpar->extradata       = av_strdup("BottomUp");
> +    if (!st->codecpar->extradata) {
> +        return AVERROR(ENOMEM);
> +    }
>     st->codecpar->extradata_size  = 9;
> 
>     // audio - mp3
> 
>     st = avformat_new_stream(s, NULL);
> -    if(!st)
> +    if(!st) {
> +        av_freep(&st->codecpar->extradata);
> +        st->codecpar->extradata_size = 0;
>         return AVERROR(ENOMEM);
> +    }

NULL pointer dereference?

> 
>     avpriv_set_pts_info(st, 64, 1, MTV_AUDIO_SAMPLING_RATE);
>     st->codecpar->codec_type      = AVMEDIA_TYPE_AUDIO;
> @@ -187,8 +193,11 @@ static int mtv_read_header(AVFormatContext *s)
> 
>     // Jump over header
> 
> -    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE)
> +    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE) {
> +        av_freep(&st->codecpar->extradata);
> +        st->codecpar->extradata_size = 0;
>         return AVERROR(EIO);
> +    }
> 
>     return 0;
> 
> -- 
> 2.15.1
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Steven Liu Oct. 10, 2019, 4:59 a.m.
> 在 2019年10月10日,12:53,zhilizhao(赵志立) <quinkblack@foxmail.com> 写道:
> 
> 
> 
>> On Oct 10, 2019, at 11:40 AM, Steven Liu <lq@chinaffmpeg.org> wrote:
>> 
>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>> ---
>> libavformat/mtv.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavformat/mtv.c b/libavformat/mtv.c
>> index 728f4a4781..c664ae14c7 100644
>> --- a/libavformat/mtv.c
>> +++ b/libavformat/mtv.c
>> @@ -171,13 +171,19 @@ static int mtv_read_header(AVFormatContext *s)
>>    st->codecpar->width           = mtv->img_width;
>>    st->codecpar->height          = mtv->img_height;
>>    st->codecpar->extradata       = av_strdup("BottomUp");
>> +    if (!st->codecpar->extradata) {
>> +        return AVERROR(ENOMEM);
>> +    }
>>    st->codecpar->extradata_size  = 9;
>> 
>>    // audio - mp3
>> 
>>    st = avformat_new_stream(s, NULL);
>> -    if(!st)
>> +    if(!st) {
>> +        av_freep(&st->codecpar->extradata);
>> +        st->codecpar->extradata_size = 0;
>>        return AVERROR(ENOMEM);
>> +    }
> 
> NULL pointer dereference?
When new streams cannot be create, there have no memory, so release the st->codecpar->extradata,
now st->codecpar->extradata is not NULL pointer. 
> 
> 
>> 
>>    avpriv_set_pts_info(st, 64, 1, MTV_AUDIO_SAMPLING_RATE);
>>    st->codecpar->codec_type      = AVMEDIA_TYPE_AUDIO;
>> @@ -187,8 +193,11 @@ static int mtv_read_header(AVFormatContext *s)
>> 
>>    // Jump over header
>> 
>> -    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE)
>> +    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE) {
>> +        av_freep(&st->codecpar->extradata);
>> +        st->codecpar->extradata_size = 0;
>>        return AVERROR(EIO);
>> +    }
>> 
>>    return 0;
>> 
>> -- 
>> 2.15.1
>> 
>> 
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".

Thanks
Steven
Zhao Zhili Oct. 10, 2019, 5:09 a.m.
> On Oct 10, 2019, at 12:59 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> 
> 
>> 在 2019年10月10日,12:53,zhilizhao(赵志立) <quinkblack@foxmail.com> 写道:
>> 
>> 
>> 
>>> On Oct 10, 2019, at 11:40 AM, Steven Liu <lq@chinaffmpeg.org> wrote:
>>> 
>>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>>> ---
>>> libavformat/mtv.c | 13 +++++++++++--
>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/libavformat/mtv.c b/libavformat/mtv.c
>>> index 728f4a4781..c664ae14c7 100644
>>> --- a/libavformat/mtv.c
>>> +++ b/libavformat/mtv.c
>>> @@ -171,13 +171,19 @@ static int mtv_read_header(AVFormatContext *s)
>>>   st->codecpar->width           = mtv->img_width;
>>>   st->codecpar->height          = mtv->img_height;
>>>   st->codecpar->extradata       = av_strdup("BottomUp");
>>> +    if (!st->codecpar->extradata) {
>>> +        return AVERROR(ENOMEM);
>>> +    }
>>>   st->codecpar->extradata_size  = 9;
>>> 
>>>   // audio - mp3
>>> 
>>>   st = avformat_new_stream(s, NULL);
>>> -    if(!st)
>>> +    if(!st) {
>>> +        av_freep(&st->codecpar->extradata);
>>> +        st->codecpar->extradata_size = 0;
>>>       return AVERROR(ENOMEM);
>>> +    }
>> 
>> NULL pointer dereference?
> When new streams cannot be create, there have no memory, so release the st->codecpar->extradata,
> now st->codecpar->extradata is not NULL pointer.

“st”  is NULL, st->codecpar is NULL pointer dereference.

PS: I don’t think here is a memory leak.

>  
>> 
>> 
>>> 
>>>   avpriv_set_pts_info(st, 64, 1, MTV_AUDIO_SAMPLING_RATE);
>>>   st->codecpar->codec_type      = AVMEDIA_TYPE_AUDIO;
>>> @@ -187,8 +193,11 @@ static int mtv_read_header(AVFormatContext *s)
>>> 
>>>   // Jump over header
>>> 
>>> -    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE)
>>> +    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE) {
>>> +        av_freep(&st->codecpar->extradata);
>>> +        st->codecpar->extradata_size = 0;
>>>       return AVERROR(EIO);
>>> +    }
>>> 
>>>   return 0;
>>> 
>>> -- 
>>> 2.15.1
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> 
>> 
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
> 
> Thanks
> Steven
> 
> 
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
Steven Liu Oct. 10, 2019, 5:19 a.m.
> 在 2019年10月10日,13:09,zhilizhao(赵志立) <quinkblack@foxmail.com> 写道:
> 
> 
> 
>> On Oct 10, 2019, at 12:59 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
>> 
>> 
>> 
>>> 在 2019年10月10日,12:53,zhilizhao(赵志立) <quinkblack@foxmail.com> 写道:
>>> 
>>> 
>>> 
>>>> On Oct 10, 2019, at 11:40 AM, Steven Liu <lq@chinaffmpeg.org> wrote:
>>>> 
>>>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>>>> ---
>>>> libavformat/mtv.c | 13 +++++++++++--
>>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/libavformat/mtv.c b/libavformat/mtv.c
>>>> index 728f4a4781..c664ae14c7 100644
>>>> --- a/libavformat/mtv.c
>>>> +++ b/libavformat/mtv.c
>>>> @@ -171,13 +171,19 @@ static int mtv_read_header(AVFormatContext *s)
>>>>  st->codecpar->width           = mtv->img_width;
>>>>  st->codecpar->height          = mtv->img_height;
>>>>  st->codecpar->extradata       = av_strdup("BottomUp");
>>>> +    if (!st->codecpar->extradata) {
>>>> +        return AVERROR(ENOMEM);
>>>> +    }
>>>>  st->codecpar->extradata_size  = 9;
>>>> 
>>>>  // audio - mp3
>>>> 
>>>>  st = avformat_new_stream(s, NULL);
>>>> -    if(!st)
>>>> +    if(!st) {
>>>> +        av_freep(&st->codecpar->extradata);
>>>> +        st->codecpar->extradata_size = 0;
>>>>      return AVERROR(ENOMEM);
>>>> +    }
>>> 
>>> NULL pointer dereference?
>> When new streams cannot be create, there have no memory, so release the st->codecpar->extradata,
>> now st->codecpar->extradata is not NULL pointer.
> 
> “st”  is NULL, st->codecpar is NULL pointer dereference.
> 
Ah, get your point.
> PS: I don’t think here is a memory leak.

I think it should free the st->codecpar->extradata before this st = avformat_new_stream(s, NULL);
> 
> 
>> 
>>> 
>>> 
>>>> 
>>>>  avpriv_set_pts_info(st, 64, 1, MTV_AUDIO_SAMPLING_RATE);
>>>>  st->codecpar->codec_type      = AVMEDIA_TYPE_AUDIO;
>>>> @@ -187,8 +193,11 @@ static int mtv_read_header(AVFormatContext *s)
>>>> 
>>>>  // Jump over header
>>>> 
>>>> -    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE)
>>>> +    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE) {
>>>> +        av_freep(&st->codecpar->extradata);
>>>> +        st->codecpar->extradata_size = 0;
>>>>      return AVERROR(EIO);
>>>> +    }
>>>> 
>>>>  return 0;
>>>> 
>>>> -- 
>>>> 2.15.1
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel@ffmpeg.org
>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>> 
>>>> To unsubscribe, visit link above, or email
>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
>> 
>> Thanks
>> Steven
>> 
>> 
>> 
>> 
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".

Thanks
Steven
Steven Liu Oct. 10, 2019, 5:22 a.m.
> 在 2019年10月10日,13:19,Steven Liu <lq@chinaffmpeg.org> 写道:
> 
> 
> 
>> 在 2019年10月10日,13:09,zhilizhao(赵志立) <quinkblack@foxmail.com> 写道:
>> 
>> 
>> 
>>> On Oct 10, 2019, at 12:59 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
>>> 
>>> 
>>> 
>>>> 在 2019年10月10日,12:53,zhilizhao(赵志立) <quinkblack@foxmail.com> 写道:
>>>> 
>>>> 
>>>> 
>>>>> On Oct 10, 2019, at 11:40 AM, Steven Liu <lq@chinaffmpeg.org> wrote:
>>>>> 
>>>>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>>>>> ---
>>>>> libavformat/mtv.c | 13 +++++++++++--
>>>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>>> 
>>>>> diff --git a/libavformat/mtv.c b/libavformat/mtv.c
>>>>> index 728f4a4781..c664ae14c7 100644
>>>>> --- a/libavformat/mtv.c
>>>>> +++ b/libavformat/mtv.c
>>>>> @@ -171,13 +171,19 @@ static int mtv_read_header(AVFormatContext *s)
>>>>> st->codecpar->width           = mtv->img_width;
>>>>> st->codecpar->height          = mtv->img_height;
>>>>> st->codecpar->extradata       = av_strdup("BottomUp");
>>>>> +    if (!st->codecpar->extradata) {
>>>>> +        return AVERROR(ENOMEM);
>>>>> +    }
>>>>> st->codecpar->extradata_size  = 9;
>>>>> 
>>>>> // audio - mp3
>>>>> 
>>>>> st = avformat_new_stream(s, NULL);
>>>>> -    if(!st)
>>>>> +    if(!st) {
>>>>> +        av_freep(&st->codecpar->extradata);
>>>>> +        st->codecpar->extradata_size = 0;
>>>>>     return AVERROR(ENOMEM);
>>>>> +    }
>>>> 
>>>> NULL pointer dereference?
>>> When new streams cannot be create, there have no memory, so release the st->codecpar->extradata,
>>> now st->codecpar->extradata is not NULL pointer.
>> 
>> “st”  is NULL, st->codecpar is NULL pointer dereference.
>> 
> Ah, get your point.
>> PS: I don’t think here is a memory leak.
> 
> I think it should free the st->codecpar->extradata before this st = avformat_new_stream(s, NULL);
or make deferent name of the AVStream
>> 
>> 
>>> 
>>>> 
>>>> 
>>>>> 
>>>>> avpriv_set_pts_info(st, 64, 1, MTV_AUDIO_SAMPLING_RATE);
>>>>> st->codecpar->codec_type      = AVMEDIA_TYPE_AUDIO;
>>>>> @@ -187,8 +193,11 @@ static int mtv_read_header(AVFormatContext *s)
>>>>> 
>>>>> // Jump over header
>>>>> 
>>>>> -    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE)
>>>>> +    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE) {
>>>>> +        av_freep(&st->codecpar->extradata);
>>>>> +        st->codecpar->extradata_size = 0;
>>>>>     return AVERROR(EIO);
>>>>> +    }
>>>>> 
>>>>> return 0;
>>>>> 
>>>>> -- 
>>>>> 2.15.1
>>>>> 
>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> ffmpeg-devel mailing list
>>>>> ffmpeg-devel@ffmpeg.org
>>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>> 
>>>>> To unsubscribe, visit link above, or email
>>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>>>> 
>>>> To unsubscribe, visit link above, or email
>>>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
>>> 
>>> Thanks
>>> Steven
>>> 
>>> 
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".
> 
> Thanks
> Steven

Thanks
Steven

Patch hide | download patch | download mbox

diff --git a/libavformat/mtv.c b/libavformat/mtv.c
index 728f4a4781..c664ae14c7 100644
--- a/libavformat/mtv.c
+++ b/libavformat/mtv.c
@@ -171,13 +171,19 @@  static int mtv_read_header(AVFormatContext *s)
     st->codecpar->width           = mtv->img_width;
     st->codecpar->height          = mtv->img_height;
     st->codecpar->extradata       = av_strdup("BottomUp");
+    if (!st->codecpar->extradata) {
+        return AVERROR(ENOMEM);
+    }
     st->codecpar->extradata_size  = 9;
 
     // audio - mp3
 
     st = avformat_new_stream(s, NULL);
-    if(!st)
+    if(!st) {
+        av_freep(&st->codecpar->extradata);
+        st->codecpar->extradata_size = 0;
         return AVERROR(ENOMEM);
+    }
 
     avpriv_set_pts_info(st, 64, 1, MTV_AUDIO_SAMPLING_RATE);
     st->codecpar->codec_type      = AVMEDIA_TYPE_AUDIO;
@@ -187,8 +193,11 @@  static int mtv_read_header(AVFormatContext *s)
 
     // Jump over header
 
-    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE)
+    if(avio_seek(pb, MTV_HEADER_SIZE, SEEK_SET) != MTV_HEADER_SIZE) {
+        av_freep(&st->codecpar->extradata);
+        st->codecpar->extradata_size = 0;
         return AVERROR(EIO);
+    }
 
     return 0;