[FFmpeg-devel] avformat/hlsenc: fix memleak in hlsenc

Message ID	20170105002022.13822-1-lq@chinaffmpeg.org
State	Accepted
Headers	show Delivered-To: ffmpegpatchwork@gmail.com Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; From: Steven Liu <lq@chinaffmpeg.org> To: ffmpeg-devel@ffmpeg.org Date: Thu, 5 Jan 2017 08:20:22 +0800 Message-Id: <20170105002022.13822-1-lq@chinaffmpeg.org> Subject: [FFmpeg-devel] [PATCH] avformat/hlsenc: fix memleak in hlsenc Precedence: list Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Cc: Steven Liu <lq@chinaffmpeg.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>

Message ID

20170105002022.13822-1-lq@chinaffmpeg.org

State

Accepted

Headers

Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org
	designates 79.124.17.100 as permitted sender)
	client-ip=79.124.17.100; 
From: Steven Liu <lq@chinaffmpeg.org>
To: ffmpeg-devel@ffmpeg.org
Date: Thu,  5 Jan 2017 08:20:22 +0800
Message-Id: <20170105002022.13822-1-lq@chinaffmpeg.org>
Subject: [FFmpeg-devel] [PATCH] avformat/hlsenc: fix memleak in hlsenc
Precedence: list
Reply-To: FFmpeg development discussions and patches
	<ffmpeg-devel@ffmpeg.org>
Cc: Steven Liu <lq@chinaffmpeg.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>

fix CID: 1398364 Resource leak refine the code of the new options Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavformat/hlsenc.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)

Comments

Bodecs Bela Jan. 5, 2017, 7:47 a.m. UTC | #1

2017.01.05. 1:20 keltezéssel, Steven Liu írta:
> fix CID: 1398364 Resource leak
> refine the code of the new options
>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>   libavformat/hlsenc.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 808a797..feeb853 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -446,11 +446,18 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
>       if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE | HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
>           strlen(hls->current_segment_final_filename_fmt)) {
>           char * old_filename = av_strdup(hls->avf->filename);  // %%s will be %s after strftime
> +        if (!old_filename) {
> +            av_free(en);
> +            return AVERROR(ENOMEM);
> +        }
>           av_strlcpy(hls->avf->filename, hls->current_segment_final_filename_fmt, sizeof(hls->avf->filename));
> +        char * filename = av_strdup(hls->avf->filename);  // %%s will be %s after strftime
> +        if (!filename) {
> +            av_free(old_filename);
> +            av_free(en);
> +            return AVERROR(ENOMEM);
> +        }
>           if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
> -            char * filename = av_strdup(hls->avf->filename);  // %%s will be %s after strftime
> -            if (!filename)
> -                return AVERROR(ENOMEM);
>               if (replace_int_data_in_filename(hls->avf->filename, sizeof(hls->avf->filename),
>                   filename, 's', pos + size) < 1) {
>                   av_log(hls, AV_LOG_ERROR,
> @@ -459,14 +466,11 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
>                          filename);
>                   av_free(filename);
>                   av_free(old_filename);
> +                av_free(en);
>                   return AVERROR(EINVAL);
>               }
> -            av_free(filename);
>           }
>           if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_DURATION) {
> -            char * filename = av_strdup(hls->avf->filename);  // %%t will be %t after strftime
> -            if (!filename)
> -                return AVERROR(ENOMEM);
>               if (replace_int_data_in_filename(hls->avf->filename, sizeof(hls->avf->filename),
>                   filename, 't',  (int64_t)round(1000000 * duration)) < 1) {
>                   av_log(hls, AV_LOG_ERROR,
> @@ -475,10 +479,11 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
>                          filename);
>                   av_free(filename);
>                   av_free(old_filename);
> +                av_free(en);
>                   return AVERROR(EINVAL);
>               }
> -            av_free(filename);
>           }
> +        av_free(filename);
>           ff_rename(old_filename, hls->avf->filename, hls);
>           av_free(old_filename);
>       }
if you remove av_strdup after if branch,  this way if

HLS_SECOND_LEVEL_SEGMENT_SIZE and HLS_SECOND_LEVEL_SEGMENT_DURATION also set, only later one will applied because filename will be the original one in second if branch.

bb

Steven Liu Jan. 5, 2017, 8:43 a.m. UTC | #2

2017-01-05 15:47 GMT+08:00 Bodecs Bela <bodecsb@vivanet.hu>:

>
>
> 2017.01.05. 1:20 keltezéssel, Steven Liu írta:
>
>> fix CID: 1398364 Resource leak
>> refine the code of the new options
>>
>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>> ---
>>   libavformat/hlsenc.c | 21 +++++++++++++--------
>>   1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index 808a797..feeb853 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -446,11 +446,18 @@ static int hls_append_segment(struct
>> AVFormatContext *s, HLSContext *hls, double
>>       if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
>> HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
>>           strlen(hls->current_segment_final_filename_fmt)) {
>>           char * old_filename = av_strdup(hls->avf->filename);  // %%s
>> will be %s after strftime
>> +        if (!old_filename) {
>> +            av_free(en);
>> +            return AVERROR(ENOMEM);
>> +        }
>>           av_strlcpy(hls->avf->filename, hls->current_segment_final_filename_fmt,
>> sizeof(hls->avf->filename));
>> +        char * filename = av_strdup(hls->avf->filename);  // %%s will
>> be %s after strftime
>> +        if (!filename) {
>> +            av_free(old_filename);
>> +            av_free(en);
>> +            return AVERROR(ENOMEM);
>> +        }
>>           if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
>> -            char * filename = av_strdup(hls->avf->filename);  // %%s
>> will be %s after strftime
>> -            if (!filename)
>> -                return AVERROR(ENOMEM);
>>               if (replace_int_data_in_filename(hls->avf->filename,
>> sizeof(hls->avf->filename),
>>                   filename, 's', pos + size) < 1) {
>>                   av_log(hls, AV_LOG_ERROR,
>> @@ -459,14 +466,11 @@ static int hls_append_segment(struct
>> AVFormatContext *s, HLSContext *hls, double
>>                          filename);
>>                   av_free(filename);
>>                   av_free(old_filename);
>> +                av_free(en);
>>                   return AVERROR(EINVAL);
>>               }
>> -            av_free(filename);
>>           }
>>           if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_DURATION) {
>> -            char * filename = av_strdup(hls->avf->filename);  // %%t
>> will be %t after strftime
>> -            if (!filename)
>> -                return AVERROR(ENOMEM);
>>               if (replace_int_data_in_filename(hls->avf->filename,
>> sizeof(hls->avf->filename),
>>                   filename, 't',  (int64_t)round(1000000 * duration)) <
>> 1) {
>>                   av_log(hls, AV_LOG_ERROR,
>> @@ -475,10 +479,11 @@ static int hls_append_segment(struct
>> AVFormatContext *s, HLSContext *hls, double
>>                          filename);
>>                   av_free(filename);
>>                   av_free(old_filename);
>> +                av_free(en);
>>                   return AVERROR(EINVAL);
>>               }
>> -            av_free(filename);
>>           }
>> +        av_free(filename);
>>           ff_rename(old_filename, hls->avf->filename, hls);
>>           av_free(old_filename);
>>       }
>>
> if you remove av_strdup after if branch,  this way if
>
> HLS_SECOND_LEVEL_SEGMENT_SIZE and HLS_SECOND_LEVEL_SEGMENT_DURATION also
> set, only later one will applied because filename will be the original one
> in second if branch.
>
> bb
>
>
> new patch update.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Bodecs Bela Jan. 5, 2017, 8:58 a.m. UTC | #3

2017.01.05. 9:43 keltezéssel, Steven Liu írta:
> 2017-01-05 15:47 GMT+08:00 Bodecs Bela <bodecsb@vivanet.hu>:
>
>>
>> 2017.01.05. 1:20 keltezéssel, Steven Liu írta:
>>
>>> fix CID: 1398364 Resource leak
>>> refine the code of the new options
>>>
>>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>>> ---
>>>    libavformat/hlsenc.c | 21 +++++++++++++--------
>>>    1 file changed, 13 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index 808a797..feeb853 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -446,11 +446,18 @@ static int hls_append_segment(struct
>>> AVFormatContext *s, HLSContext *hls, double
>>>        if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
>>> HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
>>>            strlen(hls->current_segment_final_filename_fmt)) {
>>>            char * old_filename = av_strdup(hls->avf->filename);  // %%s
>>> will be %s after strftime
>>> +        if (!old_filename) {
>>> +            av_free(en);
>>> +            return AVERROR(ENOMEM);
>>> +        }
>>>            av_strlcpy(hls->avf->filename, hls->current_segment_final_filename_fmt,
>>> sizeof(hls->avf->filename));
>>> +        char * filename = av_strdup(hls->avf->filename);  // %%s will
>>> be %s after strftime
>>> +        if (!filename) {
>>> +            av_free(old_filename);
>>> +            av_free(en);
>>> +            return AVERROR(ENOMEM);
>>> +        }
>>>            if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
>>> -            char * filename = av_strdup(hls->avf->filename);  // %%s
>>> will be %s after strftime
>>> -            if (!filename)
>>> -                return AVERROR(ENOMEM);
>>>                if (replace_int_data_in_filename(hls->avf->filename,
>>> sizeof(hls->avf->filename),
>>>                    filename, 's', pos + size) < 1) {
>>>                    av_log(hls, AV_LOG_ERROR,
>>> @@ -459,14 +466,11 @@ static int hls_append_segment(struct
>>> AVFormatContext *s, HLSContext *hls, double
>>>                           filename);
>>>                    av_free(filename);
>>>                    av_free(old_filename);
>>> +                av_free(en);
>>>                    return AVERROR(EINVAL);
>>>                }
>>> -            av_free(filename);
>>>            }
>>>            if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_DURATION) {
>>> -            char * filename = av_strdup(hls->avf->filename);  // %%t
>>> will be %t after strftime
>>> -            if (!filename)
>>> -                return AVERROR(ENOMEM);
>>>                if (replace_int_data_in_filename(hls->avf->filename,
>>> sizeof(hls->avf->filename),
>>>                    filename, 't',  (int64_t)round(1000000 * duration)) <
>>> 1) {
>>>                    av_log(hls, AV_LOG_ERROR,
>>> @@ -475,10 +479,11 @@ static int hls_append_segment(struct
>>> AVFormatContext *s, HLSContext *hls, double
>>>                           filename);
>>>                    av_free(filename);
>>>                    av_free(old_filename);
>>> +                av_free(en);
>>>                    return AVERROR(EINVAL);
>>>                }
>>> -            av_free(filename);
>>>            }
>>> +        av_free(filename);
>>>            ff_rename(old_filename, hls->avf->filename, hls);
>>>            av_free(old_filename);
>>>        }
>>>
>> if you remove av_strdup after if branch,  this way if
>>
>> HLS_SECOND_LEVEL_SEGMENT_SIZE and HLS_SECOND_LEVEL_SEGMENT_DURATION also
>> set, only later one will applied because filename will be the original one
>> in second if branch.
>>
>> bb
>>
>>
>> new patch update.
> yes, it os ok now, thank you.
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Steven Liu Jan. 5, 2017, 9:09 a.m. UTC | #4

2017-01-05 16:58 GMT+08:00 Bodecs Bela <bodecsb@vivanet.hu>:

>
>
> 2017.01.05. 9:43 keltezéssel, Steven Liu írta:
>
>> 2017-01-05 15:47 GMT+08:00 Bodecs Bela <bodecsb@vivanet.hu>:
>>
>>
>>> 2017.01.05. 1:20 keltezéssel, Steven Liu írta:
>>>
>>> fix CID: 1398364 Resource leak
>>>> refine the code of the new options
>>>>
>>>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>>>> ---
>>>>    libavformat/hlsenc.c | 21 +++++++++++++--------
>>>>    1 file changed, 13 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>>> index 808a797..feeb853 100644
>>>> --- a/libavformat/hlsenc.c
>>>> +++ b/libavformat/hlsenc.c
>>>> @@ -446,11 +446,18 @@ static int hls_append_segment(struct
>>>> AVFormatContext *s, HLSContext *hls, double
>>>>        if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
>>>> HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
>>>>            strlen(hls->current_segment_final_filename_fmt)) {
>>>>            char * old_filename = av_strdup(hls->avf->filename);  // %%s
>>>> will be %s after strftime
>>>> +        if (!old_filename) {
>>>> +            av_free(en);
>>>> +            return AVERROR(ENOMEM);
>>>> +        }
>>>>            av_strlcpy(hls->avf->filename,
>>>> hls->current_segment_final_filename_fmt,
>>>> sizeof(hls->avf->filename));
>>>> +        char * filename = av_strdup(hls->avf->filename);  // %%s will
>>>> be %s after strftime
>>>> +        if (!filename) {
>>>> +            av_free(old_filename);
>>>> +            av_free(en);
>>>> +            return AVERROR(ENOMEM);
>>>> +        }
>>>>            if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
>>>> -            char * filename = av_strdup(hls->avf->filename);  // %%s
>>>> will be %s after strftime
>>>> -            if (!filename)
>>>> -                return AVERROR(ENOMEM);
>>>>                if (replace_int_data_in_filename(hls->avf->filename,
>>>> sizeof(hls->avf->filename),
>>>>                    filename, 's', pos + size) < 1) {
>>>>                    av_log(hls, AV_LOG_ERROR,
>>>> @@ -459,14 +466,11 @@ static int hls_append_segment(struct
>>>> AVFormatContext *s, HLSContext *hls, double
>>>>                           filename);
>>>>                    av_free(filename);
>>>>                    av_free(old_filename);
>>>> +                av_free(en);
>>>>                    return AVERROR(EINVAL);
>>>>                }
>>>> -            av_free(filename);
>>>>            }
>>>>            if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_DURATION) {
>>>> -            char * filename = av_strdup(hls->avf->filename);  // %%t
>>>> will be %t after strftime
>>>> -            if (!filename)
>>>> -                return AVERROR(ENOMEM);
>>>>                if (replace_int_data_in_filename(hls->avf->filename,
>>>> sizeof(hls->avf->filename),
>>>>                    filename, 't',  (int64_t)round(1000000 * duration)) <
>>>> 1) {
>>>>                    av_log(hls, AV_LOG_ERROR,
>>>> @@ -475,10 +479,11 @@ static int hls_append_segment(struct
>>>> AVFormatContext *s, HLSContext *hls, double
>>>>                           filename);
>>>>                    av_free(filename);
>>>>                    av_free(old_filename);
>>>> +                av_free(en);
>>>>                    return AVERROR(EINVAL);
>>>>                }
>>>> -            av_free(filename);
>>>>            }
>>>> +        av_free(filename);
>>>>            ff_rename(old_filename, hls->avf->filename, hls);
>>>>            av_free(old_filename);
>>>>        }
>>>>
>>>> if you remove av_strdup after if branch,  this way if
>>>
>>> HLS_SECOND_LEVEL_SEGMENT_SIZE and HLS_SECOND_LEVEL_SEGMENT_DURATION also
>>> set, only later one will applied because filename will be the original
>>> one
>>> in second if branch.
>>>
>>> bb
>>>
>>>
>>> new patch update.
>>>
>> yes, it os ok now, thank you.
>
> applied!


Thanks

>
>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 808a797..feeb853 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -446,11 +446,18 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
     if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE | HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
         strlen(hls->current_segment_final_filename_fmt)) {
         char * old_filename = av_strdup(hls->avf->filename);  // %%s will be %s after strftime
+        if (!old_filename) {
+            av_free(en);
+            return AVERROR(ENOMEM);
+        }
         av_strlcpy(hls->avf->filename, hls->current_segment_final_filename_fmt, sizeof(hls->avf->filename));
+        char * filename = av_strdup(hls->avf->filename);  // %%s will be %s after strftime
+        if (!filename) {
+            av_free(old_filename);
+            av_free(en);
+            return AVERROR(ENOMEM);
+        }
         if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
-            char * filename = av_strdup(hls->avf->filename);  // %%s will be %s after strftime
-            if (!filename)
-                return AVERROR(ENOMEM);
             if (replace_int_data_in_filename(hls->avf->filename, sizeof(hls->avf->filename),
                 filename, 's', pos + size) < 1) {
                 av_log(hls, AV_LOG_ERROR,
@@ -459,14 +466,11 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
                        filename);
                 av_free(filename);
                 av_free(old_filename);
+                av_free(en);
                 return AVERROR(EINVAL);
             }
-            av_free(filename);
         }
         if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_DURATION) {
-            char * filename = av_strdup(hls->avf->filename);  // %%t will be %t after strftime
-            if (!filename)
-                return AVERROR(ENOMEM);
             if (replace_int_data_in_filename(hls->avf->filename, sizeof(hls->avf->filename),
                 filename, 't',  (int64_t)round(1000000 * duration)) < 1) {
                 av_log(hls, AV_LOG_ERROR,
@@ -475,10 +479,11 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
                        filename);
                 av_free(filename);
                 av_free(old_filename);
+                av_free(en);
                 return AVERROR(EINVAL);
             }
-            av_free(filename);
         }
+        av_free(filename);
         ff_rename(old_filename, hls->avf->filename, hls);
         av_free(old_filename);
     }

[FFmpeg-devel] avformat/hlsenc: fix memleak in hlsenc

Commit Message

Comments

Patch