[FFmpeg-devel] [PATCH v2 0/3] resend fixes for leaks

[FFmpeg-devel] [PATCH v2 0/3] resend fixes for leaks

[Lidong Yan]

I am resending patches:
  https://ffmpeg.org/pipermail/ffmpeg-devel/2025-June/345974.html
  https://ffmpeg.org/pipermail/ffmpeg-devel/2025-June/345897.html

Since these patches haven't been noticed for a long time.

I also revised my patch in avformat/sapenc according to Michael's request.

-----BEGIN PGP PUBLIC KEY BLOCK-----

mDMEaEpkmRYJKwYBBAHaRw8BAQdAGwGqH/Dwod+i6kR0/Rhn5GanJ7wK8mM9tWP/
W2qu8Ti0HTUwMjAyNDMzMDA1NkBzbWFpbC5uanUuZWR1LmNuiJkEExYKAEEWIQQC
zskBcOehk1y8GoKZR31bPD+6owUCaEpkmQIbAwUJBaOagAULCQgHAgIiAgYVCgkI
CwIEFgIDAQIeBwIXgAAKCRCZR31bPD+6o8wHAQCLomsA4XfTd8IdG983gGULUJe/
0432buy4nX7AsAc87QEA+/QIsWTR6XLJaLa1sLSQCsZkb86U3c17JzG9oivL8gW4
OARoSmSZEgorBgEEAZdVAQUBAQdAfYrEAWd+6bOXkKvHpFmMvKzxAtlhm6ZQKdAq
+MlJ7wQDAQgHiHgEGBYKACAWIQQCzskBcOehk1y8GoKZR31bPD+6owUCaEpkmQIb
DAAKCRCZR31bPD+6ozWxAQC9OFisWrP/hHXUfj8AnC39r5pf5fEBz7lHvFgWNk2b
XwD7Bl6kvIIW7ReqtgXvcl7u78vEo+e9YeTGTlmAogjpeQk=
=rP+W
-----END PGP PUBLIC KEY BLOCK-----

Lidong Yan (3):
  avformat/sapenc: fix leak in sap_write_header()
  avformat/iamf_writer: fix leaks of avio_open_dyn_buf() allocated
    memory
  swscale/graph: fix leak in adapt_colors()

 libavformat/iamf_writer.c | 47 +++++++++++++++++++++++++--------------
 libavformat/sapenc.c      | 13 +++++++----
 libswscale/graph.c        |  4 +++-
 3 files changed, 42 insertions(+), 22 deletions(-)

-- 
2.50.0.107.g33b6ec8c79

_______________________________________________
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 v2 1/3] avformat/sapenc: fix leak in sap_write_header()

[Lidong Yan]

In sap_write_header(), ff_format_set_url() assign new allocated new_url
to contexts[i]->url but forgot to free it later. Add for loop to free
contexts[i]->url before av_free(context).

To prevent from writing free-for-loop in every return point, replace
`return 0` with `ret = 0` so normal execution can fall through fail
code.

Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
---
 libavformat/sapenc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/libavformat/sapenc.c b/libavformat/sapenc.c
index 87a834a8d8..0567a754e2 100644
--- a/libavformat/sapenc.c
+++ b/libavformat/sapenc.c
@@ -233,7 +233,6 @@ static int sap_write_header(AVFormatContext *s)
         ret = AVERROR_INVALIDDATA;
         goto fail;
     }
-    av_freep(&contexts);
     av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", &sap->ann[pos]);
     pos += strlen(&sap->ann[pos]);
     sap->ann_size = pos;
@@ -244,11 +243,17 @@ static int sap_write_header(AVFormatContext *s)
         goto fail;
     }
 
-    return 0;
+    ret = 0;
 
 fail:
-    av_free(contexts);
-    sap_write_close(s);
+    if (contexts) {
+        for (i = 0; i < s->nb_streams; i++)
+            if (contexts[i])
+                av_free(contexts[i]->url);
+        av_free(contexts);
+    }
+    if (ret < 0)
+        sap_write_close(s);
     return ret;
 }
 
-- 
2.50.0.107.g33b6ec8c79

_______________________________________________
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 v2 2/3] avformat/iamf_writer: fix leaks of avio_open_dyn_buf() allocated memory

[Lidong Yan]

In iamf_write_codec_config(), if codec_id equals to AV_CODEC_ID_AAC,
avio_open_dyn_buf() allocated memory would leak. Add ffio_free_dyn_buf()
to free dyn_bc before return.

In iamf_write_audio_element(), multiple places returns without free
dyn_bc, replace return AVERROR* with goto cleanup and add cleanup code
to free dyn_bc. Do the same thing for iamf_write_audio_element() and
write_parameter_block().

Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
---
 libavformat/iamf_writer.c | 47 +++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/libavformat/iamf_writer.c b/libavformat/iamf_writer.c
index f88987790d..2e8df602b1 100644
--- a/libavformat/iamf_writer.c
+++ b/libavformat/iamf_writer.c
@@ -523,6 +523,7 @@ static int iamf_write_codec_config(const IAMFContext *iamf,
         avio_write(dyn_bc, codec_config->extradata, codec_config->extradata_size);
         break;
     case AV_CODEC_ID_AAC:
+        ffio_free_dyn_buf(&dyn_bc);
         return AVERROR_PATCHWELCOME;
     case AV_CODEC_ID_FLAC:
         avio_w8(dyn_bc, 0x80);
@@ -774,7 +775,8 @@ static int iamf_write_audio_element(const IAMFContext *iamf,
                 if (layout == 3 || layout == 4 || layout == 6 || layout == 7) {
                     av_log(log_ctx, AV_LOG_ERROR, "demixing_info needed but not set in Stream Group #%u\n",
                            audio_element->audio_element_id);
-                    return AVERROR(EINVAL);
+                    ret = AVERROR(EINVAL);
+                    goto cleanup;
                 }
             }
             param_definition_types &= ~AV_IAMF_PARAMETER_DEFINITION_DEMIXING;
@@ -794,7 +796,7 @@ static int iamf_write_audio_element(const IAMFContext *iamf,
         param_def = ff_iamf_get_param_definition(iamf, param->parameter_id);
         ret = param_definition(iamf, param_def, dyn_bc, log_ctx);
         if (ret < 0)
-            return ret;
+            goto cleanup;
 
         avio_w8(dyn_bc, demix->dmixp_mode << 5); // dmixp_mode
         avio_w8(dyn_bc, element->default_w << 4); // default_w
@@ -806,24 +808,25 @@ static int iamf_write_audio_element(const IAMFContext *iamf,
         if (!param) {
             av_log(log_ctx, AV_LOG_ERROR, "recon_gain_info needed but not set in Stream Group #%u\n",
                    audio_element->audio_element_id);
-            return AVERROR(EINVAL);
+            ret = AVERROR(EINVAL);
+            goto cleanup;
         }
         ffio_write_leb(dyn_bc, AV_IAMF_PARAMETER_DEFINITION_RECON_GAIN); // type
 
         param_def = ff_iamf_get_param_definition(iamf, param->parameter_id);
         ret = param_definition(iamf, param_def, dyn_bc, log_ctx);
         if (ret < 0)
-            return ret;
+            goto cleanup;
     }
 
     if (element->audio_element_type == AV_IAMF_AUDIO_ELEMENT_TYPE_CHANNEL) {
         ret = scalable_channel_layout_config(audio_element, dyn_bc);
         if (ret < 0)
-            return ret;
+            goto cleanup;
     } else {
         ret = ambisonics_config(audio_element, dyn_bc);
         if (ret < 0)
-            return ret;
+            goto cleanup;
     }
 
     init_put_bits(&pbc, header, sizeof(header));
@@ -835,9 +838,11 @@ static int iamf_write_audio_element(const IAMFContext *iamf,
     avio_write(pb, header, put_bytes_count(&pbc, 1));
     ffio_write_leb(pb, dyn_size);
     avio_write(pb, dyn_buf, dyn_size);
-    ffio_free_dyn_buf(&dyn_bc);
+    ret = 0;
 
-    return 0;
+cleanup:
+    ffio_free_dyn_buf(&dyn_bc);
+    return ret;
 }
 
 static int iamf_write_mixing_presentation(const IAMFContext *iamf,
@@ -886,7 +891,8 @@ static int iamf_write_mixing_presentation(const IAMFContext *iamf,
             if (av_dict_count(submix_element->annotations) != av_dict_count(mix->annotations)) {
                 av_log(log_ctx, AV_LOG_ERROR, "Inconsistent amount of labels in submix %d from Mix Presentation id #%u\n",
                        j, audio_element->audio_element_id);
-                return AVERROR(EINVAL);
+                ret = AVERROR(EINVAL);
+                goto cleanup;
             }
             while ((tag = av_dict_iterate(submix_element->annotations, tag)))
                 avio_put_str(dyn_bc, tag->value);
@@ -901,7 +907,7 @@ static int iamf_write_mixing_presentation(const IAMFContext *iamf,
             param_def = ff_iamf_get_param_definition(iamf, submix_element->element_mix_config->parameter_id);
             ret = param_definition(iamf, param_def, dyn_bc, log_ctx);
             if (ret < 0)
-                return ret;
+                goto cleanup;
 
             avio_wb16(dyn_bc, rescale_rational(submix_element->default_mix_gain, 1 << 8));
         }
@@ -909,7 +915,7 @@ static int iamf_write_mixing_presentation(const IAMFContext *iamf,
         param_def = ff_iamf_get_param_definition(iamf, sub_mix->output_mix_config->parameter_id);
         ret = param_definition(iamf, param_def, dyn_bc, log_ctx);
         if (ret < 0)
-            return ret;
+            goto cleanup;
         avio_wb16(dyn_bc, rescale_rational(sub_mix->default_mix_gain, 1 << 8));
 
         ffio_write_leb(dyn_bc, sub_mix->nb_layouts); // nb_layouts
@@ -928,11 +934,13 @@ static int iamf_write_mixing_presentation(const IAMFContext *iamf,
                 }
                 if (layout == FF_ARRAY_ELEMS(ff_iamf_sound_system_map)) {
                     av_log(log_ctx, AV_LOG_ERROR, "Invalid Sound System value in a submix\n");
-                    return AVERROR(EINVAL);
+                    ret = AVERROR(EINVAL);
+                    goto cleanup;
                 }
             } else if (submix_layout->layout_type != AV_IAMF_SUBMIX_LAYOUT_TYPE_BINAURAL) {
                 av_log(log_ctx, AV_LOG_ERROR, "Unsupported Layout Type value in a submix\n");
-                return AVERROR(EINVAL);
+                ret = AVERROR(EINVAL);
+                goto cleanup;
             }
             init_put_bits(&pbc, header, sizeof(header));
             put_bits(&pbc, 2, submix_layout->layout_type); // layout_type
@@ -974,9 +982,11 @@ static int iamf_write_mixing_presentation(const IAMFContext *iamf,
     avio_write(pb, header, put_bytes_count(&pbc, 1));
     ffio_write_leb(pb, dyn_size);
     avio_write(pb, dyn_buf, dyn_size);
-    ffio_free_dyn_buf(&dyn_bc);
+    ret = 0;
 
-    return 0;
+cleanup:
+    ffio_free_dyn_buf(&dyn_bc);
+    return ret;
 }
 
 int ff_iamf_write_descriptors(const IAMFContext *iamf, AVIOContext *pb, void *log_ctx)
@@ -1098,7 +1108,8 @@ static int write_parameter_block(const IAMFContext *iamf, AVIOContext *pb,
 
             if (!audio_element) {
                 av_log(log_ctx, AV_LOG_ERROR, "Invalid Parameter Definition with ID %u referenced by a packet\n", param->parameter_id);
-                return AVERROR(EINVAL);
+                ret = AVERROR(EINVAL);
+                goto cleanup;
             }
 
             for (int j = 0; j < audio_element->nb_layers; j++) {
@@ -1132,8 +1143,10 @@ static int write_parameter_block(const IAMFContext *iamf, AVIOContext *pb,
     dyn_size = avio_get_dyn_buf(dyn_bc, &dyn_buf);
     ffio_write_leb(pb, dyn_size);
     avio_write(pb, dyn_buf, dyn_size);
-    ffio_free_dyn_buf(&dyn_bc);
+    ret = 0;
 
+cleanup:
+    ffio_free_dyn_buf(&dyn_bc);
     return 0;
 }
 
-- 
2.50.0.107.g33b6ec8c79

_______________________________________________
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 v2 3/3] swscale/graph: fix leak in adapt_colors()

[Lidong Yan]

In adapt_colors(), ff_sws_lut3d_generate() allocates memory in lut.
However if add_legacy_sws_pass() failed, lut leaks. free lut before
return ret.

Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
---
 libswscale/graph.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libswscale/graph.c b/libswscale/graph.c
index dc7784aa49..975a2b6065 100644
--- a/libswscale/graph.c
+++ b/libswscale/graph.c
@@ -523,8 +523,10 @@ static int adapt_colors(SwsGraph *graph, SwsFormat src, SwsFormat dst,
         SwsFormat tmp = src;
         tmp.format = fmt_in;
         ret = add_legacy_sws_pass(graph, src, tmp, input, &input);
-        if (ret < 0)
+        if (ret < 0) {
+            ff_sws_lut3d_free(&lut);
             return ret;
+        }
     }
 
     ret = ff_sws_lut3d_generate(lut, fmt_in, fmt_out, &map);
-- 
2.50.0.107.g33b6ec8c79

_______________________________________________
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 v2 1/3] avformat/sapenc: fix leak in sap_write_header()

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1929 bytes --]

On Thu, Jul 10, 2025 at 10:20:45AM +0800, Lidong Yan wrote:
> In sap_write_header(), ff_format_set_url() assign new allocated new_url
> to contexts[i]->url but forgot to free it later. Add for loop to free
> contexts[i]->url before av_free(context).
> 
> To prevent from writing free-for-loop in every return point, replace
> `return 0` with `ret = 0` so normal execution can fall through fail
> code.
> 
> Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
> ---
>  libavformat/sapenc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/sapenc.c b/libavformat/sapenc.c
> index 87a834a8d8..0567a754e2 100644
> --- a/libavformat/sapenc.c
> +++ b/libavformat/sapenc.c
> @@ -233,7 +233,6 @@ static int sap_write_header(AVFormatContext *s)
>          ret = AVERROR_INVALIDDATA;
>          goto fail;
>      }
> -    av_freep(&contexts);
>      av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", &sap->ann[pos]);
>      pos += strlen(&sap->ann[pos]);
>      sap->ann_size = pos;
> @@ -244,11 +243,17 @@ static int sap_write_header(AVFormatContext *s)
>          goto fail;
>      }
>  
> -    return 0;
> +    ret = 0;
>  
>  fail:
> -    av_free(contexts);
> -    sap_write_close(s);
> +    if (contexts) {
> +        for (i = 0; i < s->nb_streams; i++)
> +            if (contexts[i])
> +                av_free(contexts[i]->url);
> +        av_free(contexts);
> +    }

contexts is an array of AVFormatContext, this does not look right
also freeing the url of the AVFormatContexts that have been stored
in priv_data a few lines earlier looks wrong

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 v2 1/3] avformat/sapenc: fix leak in sap_write_header()

[Lidong Yan]

Michael Niedermayer <michael@niedermayer.cc> write:

> On Thu, Jul 10, 2025 at 10:20:45AM +0800, Lidong Yan wrote:
> > In sap_write_header(), ff_format_set_url() assign new allocated new_url
> > to contexts[i]->url but forgot to free it later. Add for loop to free
> > contexts[i]->url before av_free(context).
> >
> > To prevent from writing free-for-loop in every return point, replace
> > `return 0` with `ret = 0` so normal execution can fall through fail
> > code.
> >
> > Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
> > ---
> >  libavformat/sapenc.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/sapenc.c b/libavformat/sapenc.c
> > index 87a834a8d8..0567a754e2 100644
> > --- a/libavformat/sapenc.c
> > +++ b/libavformat/sapenc.c
> > @@ -233,7 +233,6 @@ static int sap_write_header(AVFormatContext *s)
> >          ret = AVERROR_INVALIDDATA;
> >          goto fail;
> >      }
> > -    av_freep(&contexts);
> >      av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", &sap->ann[pos]);
> >      pos += strlen(&sap->ann[pos]);
> >      sap->ann_size = pos;
> > @@ -244,11 +243,17 @@ static int sap_write_header(AVFormatContext *s)
> >          goto fail;
> >      }
> >
> > -    return 0;
> > +    ret = 0;
> >
> >  fail:
> > -    av_free(contexts);
> > -    sap_write_close(s);
> > +    if (contexts) {
> > +        for (i = 0; i < s->nb_streams; i++)
> > +            if (contexts[i])
> > +                av_free(contexts[i]->url);
> > +        av_free(contexts);
> > +    }
>
> contexts is an array of AVFormatContext, this does not look right
> also freeing the url of the AVFormatContexts that have been stored
> in priv_data a few lines earlier looks wrong
>

I am not sure how to fix then. Maybe I should leave this code alone.

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