[FFmpeg-devel] [PATCH 02/10] avformat/hls: fix leak of init section when dynarray_add fail

[FFmpeg-devel] [PATCH 02/10] avformat/hls: fix leak of init section when dynarray_add fail

[Zhao Zhili]

When av_dynarray_add failed, pls->init_sections will be freed,
which leads to leak of all entries of pls->init_sections.
---
 libavformat/hls.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 67c9650e0b..e249810bce 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -463,7 +463,13 @@ static struct segment *get_init_section(struct playlist *pls,
         av_free(sec_ptr);
         return NULL;
     }
-    dynarray_add(&pls->init_sections, &pls->n_init_sections, sec_ptr);
+    if (av_dynarray_add_nofree(&pls->init_sections,
+                               &pls->n_init_sections,
+                               sec_ptr) < 0) {
+        av_free(sec_ptr->url);
+        av_free(sec_ptr);
+        return NULL;
+    }
 
     return sec_ptr;
 }
-- 
2.31.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] [PATCH 03/10] avformat/hls: extract free_playlist method

[Zhao Zhili]

---
 libavformat/hls.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index e249810bce..ecb6237d2e 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -253,30 +253,35 @@ static void free_init_section_list(struct playlist *pls)
     pls->n_init_sections = 0;
 }
 
+static void free_playlist(HLSContext *c, struct playlist *pls)
+{
+    free_segment_list(pls);
+    free_init_section_list(pls);
+    av_freep(&pls->main_streams);
+    av_freep(&pls->renditions);
+    av_freep(&pls->id3_buf);
+    av_dict_free(&pls->id3_initial);
+    ff_id3v2_free_extra_meta(&pls->id3_deferred_extra);
+    av_freep(&pls->init_sec_buf);
+    av_packet_free(&pls->pkt);
+    av_freep(&pls->pb.pub.buffer);
+    ff_format_io_close(c->ctx, &pls->input);
+    pls->input_read_done = 0;
+    ff_format_io_close(c->ctx, &pls->input_next);
+    pls->input_next_requested = 0;
+    if (pls->ctx) {
+        pls->ctx->pb = NULL;
+        avformat_close_input(&pls->ctx);
+    }
+    av_free(pls);
+}
+
 static void free_playlist_list(HLSContext *c)
 {
     int i;
     for (i = 0; i < c->n_playlists; i++) {
         struct playlist *pls = c->playlists[i];
-        free_segment_list(pls);
-        free_init_section_list(pls);
-        av_freep(&pls->main_streams);
-        av_freep(&pls->renditions);
-        av_freep(&pls->id3_buf);
-        av_dict_free(&pls->id3_initial);
-        ff_id3v2_free_extra_meta(&pls->id3_deferred_extra);
-        av_freep(&pls->init_sec_buf);
-        av_packet_free(&pls->pkt);
-        av_freep(&pls->pb.pub.buffer);
-        ff_format_io_close(c->ctx, &pls->input);
-        pls->input_read_done = 0;
-        ff_format_io_close(c->ctx, &pls->input_next);
-        pls->input_next_requested = 0;
-        if (pls->ctx) {
-            pls->ctx->pb = NULL;
-            avformat_close_input(&pls->ctx);
-        }
-        av_free(pls);
+        free_playlist(c, pls);
     }
     av_freep(&c->playlists);
     c->n_playlists = 0;
-- 
2.31.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] [PATCH 04/10] avformat/hls: fix leak of playlist when dynarray_add fail

[Zhao Zhili]

---
 libavformat/hls.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index ecb6237d2e..d5c3009d07 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -330,7 +330,10 @@ static struct playlist *new_playlist(HLSContext *c, const char *url,
     pls->is_id3_timestamped = -1;
     pls->id3_mpegts_timestamp = AV_NOPTS_VALUE;
 
-    dynarray_add(&c->playlists, &c->n_playlists, pls);
+    if (av_dynarray_add_nofree(&c->playlists, &c->n_playlists, pls) < 0) {
+        free_playlist(c, pls);
+        return NULL;
+    }
     return pls;
 }
 
-- 
2.31.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] [PATCH 05/10] avformat/hls: fix leak of variant when dynarray_add fail

[Zhao Zhili]

---
 libavformat/hls.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index d5c3009d07..b5cdf158c6 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -366,8 +366,16 @@ static struct variant *new_variant(HLSContext *c, struct variant_info *info,
         strcpy(var->subtitles_group, info->subtitles);
     }
 
-    dynarray_add(&c->variants, &c->n_variants, var);
-    dynarray_add(&var->playlists, &var->n_playlists, pls);
+    if (av_dynarray_add_nofree(&var->playlists, &var->n_playlists, pls) < 0) {
+        /* Don't free pls since it's not owned by variant */
+        av_free(var);
+        return NULL;
+    }
+    if (av_dynarray_add_nofree(&c->variants, &c->n_variants, var) < 0) {
+        av_free(var->playlists);
+        av_free(var);
+        return NULL;
+    }
     return var;
 }
 
-- 
2.31.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".

Re: [FFmpeg-devel] [PATCH 05/10] avformat/hls: fix leak of variant when dynarray_add fail

[Steven Liu]

Zhao Zhili <quinkblack@foxmail.com> 于2022年4月12日周二 16:16写道：
>
> ---
>  libavformat/hls.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index d5c3009d07..b5cdf158c6 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -366,8 +366,16 @@ static struct variant *new_variant(HLSContext *c, struct variant_info *info,
>          strcpy(var->subtitles_group, info->subtitles);
>      }
>
> -    dynarray_add(&c->variants, &c->n_variants, var);
> -    dynarray_add(&var->playlists, &var->n_playlists, pls);
> +    if (av_dynarray_add_nofree(&var->playlists, &var->n_playlists, pls) < 0) {
> +        /* Don't free pls since it's not owned by variant */
> +        av_free(var);
> +        return NULL;
> +    }
> +    if (av_dynarray_add_nofree(&c->variants, &c->n_variants, var) < 0) {
> +        av_free(var->playlists);
> +        av_free(var);
Why don't use av_freep about these contexts?
> +        return NULL;
> +    }
>      return var;
>  }
>
> --
> 2.31.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".

Thanks
Steven
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 05/10] avformat/hls: fix leak of variant when dynarray_add fail

["zhilizhao(赵志立)"]

> On Apr 12, 2022, at 4:42 PM, Steven Liu <lingjiujianke@gmail.com> wrote:
> 
> Zhao Zhili <quinkblack@foxmail.com> 于2022年4月12日周二 16:16写道：
>> 
>> ---
>> libavformat/hls.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>> index d5c3009d07..b5cdf158c6 100644
>> --- a/libavformat/hls.c
>> +++ b/libavformat/hls.c
>> @@ -366,8 +366,16 @@ static struct variant *new_variant(HLSContext *c, struct variant_info *info,
>>         strcpy(var->subtitles_group, info->subtitles);
>>     }
>> 
>> -    dynarray_add(&c->variants, &c->n_variants, var);
>> -    dynarray_add(&var->playlists, &var->n_playlists, pls);
>> +    if (av_dynarray_add_nofree(&var->playlists, &var->n_playlists, pls) < 0) {
>> +        /* Don't free pls since it's not owned by variant */
>> +        av_free(var);
>> +        return NULL;
>> +    }
>> +    if (av_dynarray_add_nofree(&c->variants, &c->n_variants, var) < 0) {
>> +        av_free(var->playlists);
>> +        av_free(var);
> Why don't use av_freep about these contexts?

It’s a local variable, won’t outlive any longer, so use after free not gone happen.

>> +        return NULL;
>> +    }
>>     return var;
>> }
>> 
>> --
>> 2.31.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".
> 
> Thanks
> Steven
> _______________________________________________
> 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".

[FFmpeg-devel] [PATCH 06/10] avformat/hls: fix leak of rendition when dynarray_add fail

[Zhao Zhili]

---
 libavformat/hls.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index b5cdf158c6..3ed6007d0d 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -556,7 +556,10 @@ static struct rendition *new_rendition(HLSContext *c, struct rendition_info *inf
     if (!rend)
         return NULL;
 
-    dynarray_add(&c->renditions, &c->n_renditions, rend);
+    if (av_dynarray_add_nofree(&c->renditions, &c->n_renditions, rend) < 0) {
+        av_free(rend);
+        return NULL;
+    }
 
     rend->type = type;
     strcpy(rend->group_id, info->group_id);
@@ -566,9 +569,14 @@ static struct rendition *new_rendition(HLSContext *c, struct rendition_info *inf
     /* add the playlist if this is an external rendition */
     if (info->uri[0]) {
         rend->playlist = new_playlist(c, info->uri, url_base);
-        if (rend->playlist)
-            dynarray_add(&rend->playlist->renditions,
-                         &rend->playlist->n_renditions, rend);
+        if (rend->playlist) {
+            if (av_dynarray_add_nofree(&rend->playlist->renditions,
+                                       &rend->playlist->n_renditions,
+                                       rend) < 0) {
+                /* Don't free rend since it's owned by c->renditions */
+                return NULL;
+            }
+        }
     }
 
     if (info->assoc_language[0]) {
-- 
2.31.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] [PATCH 07/10] avformat/hls: extract free_segment method

[Zhao Zhili]

---
 libavformat/hls.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 3ed6007d0d..65937120d5 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -225,13 +225,18 @@ typedef struct HLSContext {
     HLSCryptoContext  crypto_ctx;
 } HLSContext;
 
+static void free_segment(struct segment *segment) {
+    av_free(segment->key);
+    av_free(segment->url);
+    av_free(segment);
+}
+
 static void free_segment_dynarray(struct segment **segments, int n_segments)
 {
     int i;
     for (i = 0; i < n_segments; i++) {
-        av_freep(&segments[i]->key);
-        av_freep(&segments[i]->url);
-        av_freep(&segments[i]);
+        free_segment(segments[i]);
+        segments[i] = NULL;
     }
 }
 
-- 
2.31.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] [PATCH 08/10] avformat/hls: fix leak of segments when dynarray_add fail

[Zhao Zhili]

---
 libavformat/hls.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 65937120d5..b9e2e8a04d 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1024,7 +1024,12 @@ static int parse_playlist(HLSContext *c, const char *url,
                 }
                 seg->duration = duration;
                 seg->key_type = key_type;
-                dynarray_add(&pls->segments, &pls->n_segments, seg);
+                if (av_dynarray_add_nofree(&pls->segments, &pls->n_segments,
+                                           seg) < 0) {
+                    free_segment(seg);
+                    ret = AVERROR(ENOMEM);
+                    goto fail;
+                }
                 is_segment = 0;
 
                 seg->size = seg_size;
-- 
2.31.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] [PATCH 09/10] avformat/hls: do error check in add_renditions_to_variant

[Zhao Zhili]

---
 libavformat/hls.c | 49 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index b9e2e8a04d..c102e36f52 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1666,9 +1666,10 @@ reload:
     goto restart;
 }
 
-static void add_renditions_to_variant(HLSContext *c, struct variant *var,
-                                      enum AVMediaType type, const char *group_id)
+static int add_renditions_to_variant(HLSContext *c, struct variant *var,
+                                     enum AVMediaType type, const char *group_id)
 {
+    int ret;
     int i;
 
     for (i = 0; i < c->n_renditions; i++) {
@@ -1676,18 +1677,26 @@ static void add_renditions_to_variant(HLSContext *c, struct variant *var,
 
         if (rend->type == type && !strcmp(rend->group_id, group_id)) {
 
-            if (rend->playlist)
+            if (rend->playlist) {
                 /* rendition is an external playlist
                  * => add the playlist to the variant */
-                dynarray_add(&var->playlists, &var->n_playlists, rend->playlist);
-            else
+                ret = av_dynarray_add_nofree(&var->playlists, &var->n_playlists,
+                                             rend->playlist);
+                if (ret < 0)
+                    return ret;
+            } else {
                 /* rendition is part of the variant main Media Playlist
                  * => add the rendition to the main Media Playlist */
-                dynarray_add(&var->playlists[0]->renditions,
-                             &var->playlists[0]->n_renditions,
-                             rend);
+                ret = av_dynarray_add_nofree(&var->playlists[0]->renditions,
+                                             &var->playlists[0]->n_renditions,
+                                             rend);
+                if (ret < 0)
+                    return ret;
+            }
         }
     }
+
+    return 0;
 }
 
 static void add_metadata_from_renditions(AVFormatContext *s, struct playlist *pls,
@@ -1987,12 +1996,24 @@ static int hls_read_header(AVFormatContext *s)
     for (i = 0; i < c->n_variants; i++) {
         struct variant *var = c->variants[i];
 
-        if (var->audio_group[0])
-            add_renditions_to_variant(c, var, AVMEDIA_TYPE_AUDIO, var->audio_group);
-        if (var->video_group[0])
-            add_renditions_to_variant(c, var, AVMEDIA_TYPE_VIDEO, var->video_group);
-        if (var->subtitles_group[0])
-            add_renditions_to_variant(c, var, AVMEDIA_TYPE_SUBTITLE, var->subtitles_group);
+        if (var->audio_group[0]) {
+            ret = add_renditions_to_variant(c, var, AVMEDIA_TYPE_AUDIO,
+                                            var->audio_group);
+            if (ret < 0)
+                return ret;
+        }
+        if (var->video_group[0]) {
+            ret = add_renditions_to_variant(c, var, AVMEDIA_TYPE_VIDEO,
+                                            var->video_group);
+            if (ret < 0)
+                return ret;
+        }
+        if (var->subtitles_group[0]) {
+            ret = add_renditions_to_variant(c, var, AVMEDIA_TYPE_SUBTITLE,
+                                            var->subtitles_group);
+            if (ret < 0)
+                return ret;
+        }
     }
 
     /* Create a program for each variant */
-- 
2.31.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] [PATCH 10/10] avformat/hls: check dynarray_add error when add stream

[Zhao Zhili]

---
 libavformat/hls.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index c102e36f52..4ab565f8e0 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1882,7 +1882,10 @@ static int update_streams_from_subdemuxer(AVFormatContext *s, struct playlist *p
             return AVERROR(ENOMEM);
 
         st->id = pls->index;
-        dynarray_add(&pls->main_streams, &pls->n_main_streams, st);
+        err = av_dynarray_add_nofree(&pls->main_streams, &pls->n_main_streams,
+                                     st);
+        if (err < 0)
+            return err;
 
         add_stream_to_programs(s, pls, st);
 
-- 
2.31.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".