[PATCH 0/5] fix multiple memory leaks
This patch series fix multiple memory leaks in error path and early return point. -----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 (5): avcodec/utvideodec: fix leaks in decode_plane() and decode_plane10() avformat/rtpdec_latm: fix leak in parse_fmtp_config() swscale/graph: fix leak in adapt_colors() avcodec/sunrast: fix leak in sunrast_decode_frame() avformat/rtpdec_asf: fix leak in ff_wms_parse_sdp_a_line() libavcodec/sunrast.c | 4 +++- libavcodec/utvideodec.c | 6 ++++-- libavformat/rtpdec_asf.c | 4 +++- libavformat/rtpdec_latm.c | 2 +- libswscale/graph.c | 4 +++- 5 files changed, 14 insertions(+), 6 deletions(-) -- 2.50.0.106.gf0135a9047.dirty
In decode_plane() and decode_plane10(), both of these two functions use build_buff() which allocates memory in vlc and multi. And both of them forget to release vlc and multi when build_buff report a symbol to fill slices with. Add cleanup label and goto cleanup first before return 0. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> --- libavcodec/utvideodec.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c index 4c0fa2ca67..f15d623462 100644 --- a/libavcodec/utvideodec.c +++ b/libavcodec/utvideodec.c @@ -179,7 +179,7 @@ static int decode_plane10(UtvideoContext *c, int plane_no, dest += stride; } } - return 0; + goto cleanup; } send = 0; @@ -216,6 +216,7 @@ static int decode_plane10(UtvideoContext *c, int plane_no, "%d bits left after decoding slice\n", get_bits_left(&gb)); } +cleanup: ff_vlc_free(&vlc); ff_vlc_free_multi(&multi); @@ -322,7 +323,7 @@ static int decode_plane(UtvideoContext *c, int plane_no, dest += stride; } } - return 0; + goto cleanup; } src += 256; @@ -361,6 +362,7 @@ static int decode_plane(UtvideoContext *c, int plane_no, "%d bits left after decoding slice\n", get_bits_left(&gb)); } +cleanup: ff_vlc_free(&vlc); ff_vlc_free_multi(&multi); -- 2.50.0.106.gf0135a9047.dirty
On Sun, Jun 29, 2025 at 02:45:22PM +0800, Lidong Yan wrote:
In decode_plane() and decode_plane10(), both of these two functions use build_buff() which allocates memory in vlc and multi. And both of them forget to release vlc and multi when build_buff report a symbol to fill slices with. Add cleanup label and goto cleanup first before return 0.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> --- libavcodec/utvideodec.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c index 4c0fa2ca67..f15d623462 100644 --- a/libavcodec/utvideodec.c +++ b/libavcodec/utvideodec.c @@ -179,7 +179,7 @@ static int decode_plane10(UtvideoContext *c, int plane_no, dest += stride; } } - return 0; + goto cleanup; }
send = 0; @@ -216,6 +216,7 @@ static int decode_plane10(UtvideoContext *c, int plane_no, "%d bits left after decoding slice\n", get_bits_left(&gb)); }
+cleanup: ff_vlc_free(&vlc); ff_vlc_free_multi(&multi);
@@ -322,7 +323,7 @@ static int decode_plane(UtvideoContext *c, int plane_no, dest += stride; } } - return 0; + goto cleanup; }
src += 256; @@ -361,6 +362,7 @@ static int decode_plane(UtvideoContext *c, int plane_no, "%d bits left after decoding slice\n", get_bits_left(&gb)); }
+cleanup: ff_vlc_free(&vlc); ff_vlc_free_multi(&multi);
This is not correct build_huff() does not set these when fsym >= 0 your patch runs free() on random uninitialized variables before submitting memleak fixes, please verify that 1. there is actually a leak 2. your patch does not introduce a new anomaly thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri
Michael Niedermayer <michael@niedermayer.cc> writes:
This is not correct
build_huff() does not set these when fsym >= 0
your patch runs free() on random uninitialized variables
before submitting memleak fixes, please verify that 1. there is actually a leak 2. your patch does not introduce a new anomaly
Got it, I will be more careful. Thanks for your review, Lidong
av_mallocz() allocates memory in config, but we forget to free it if init_get_bits() failed. Replace return ret with goto end. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> --- libavformat/rtpdec_latm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/rtpdec_latm.c b/libavformat/rtpdec_latm.c index 2b4478289e..74523c167d 100644 --- a/libavformat/rtpdec_latm.c +++ b/libavformat/rtpdec_latm.c @@ -104,7 +104,7 @@ static int parse_fmtp_config(AVStream *st, const char *value) ff_hex_to_data(config, value); ret = init_get_bits(&gb, config, len*8); if (ret < 0) - return ret; + goto end; audio_mux_version = get_bits(&gb, 1); same_time_framing = get_bits(&gb, 1); skip_bits(&gb, 6); /* num_sub_frames */ -- 2.50.0.106.gf0135a9047.dirty
On Sun, Jun 29, 2025 at 02:45:23PM +0800, Lidong Yan wrote:
av_mallocz() allocates memory in config, but we forget to free it if init_get_bits() failed. Replace return ret with goto end.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> --- libavformat/rtpdec_latm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship: All citizens are under surveillance, all their steps and actions recorded, for the politicians to enforce control. Democracy: All politicians are under surveillance, all their steps and actions recorded, for the citizens to enforce control.
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.106.gf0135a9047.dirty
In sunrast_decode_frame(), we use av_malloc_array() allocates memory to ptr and ptr2. However if buf_end - buf < 1, this function returns error code without freeing this memory thus cause a leak. Add av_freep() before return. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> --- libavcodec/sunrast.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavcodec/sunrast.c b/libavcodec/sunrast.c index 9e49c4f275..cc27838f5b 100644 --- a/libavcodec/sunrast.c +++ b/libavcodec/sunrast.c @@ -163,8 +163,10 @@ static int sunrast_decode_frame(AVCodecContext *avctx, AVFrame *p, x = 0; while (ptr != end && buf < buf_end) { run = 1; - if (buf_end - buf < 1) + if (buf_end - buf < 1) { + av_freep(&ptr2); return AVERROR_INVALIDDATA; + } if ((value = *buf++) == RLE_TRIGGER) { run = *buf++ + 1; -- 2.50.0.106.gf0135a9047.dirty
On Sun, Jun 29, 2025 at 02:45:25PM +0800, Lidong Yan wrote:
In sunrast_decode_frame(), we use av_malloc_array() allocates memory to ptr and ptr2. However if buf_end - buf < 1, this function returns error code without freeing this memory thus cause a leak. Add av_freep() before return.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> --- libavcodec/sunrast.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
will apply might be nice to factor this so its freed in a common place thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The misfortune of the wise is better than the prosperity of the fool. -- Epicurus
In ff_wms_parse_sdp_a_line(), it allocates memory in buf, but doesn't free buf when avformat_alloc_context() failed. Add av_free(buf) before return to prevent from leak. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> --- libavformat/rtpdec_asf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavformat/rtpdec_asf.c b/libavformat/rtpdec_asf.c index 9664623e57..b3b346f3cc 100644 --- a/libavformat/rtpdec_asf.c +++ b/libavformat/rtpdec_asf.c @@ -120,8 +120,10 @@ int ff_wms_parse_sdp_a_line(AVFormatContext *s, const char *p) avformat_close_input(&rt->asf_ctx); } - if (!(iformat = av_find_input_format("asf"))) + if (!(iformat = av_find_input_format("asf"))) { + av_free(buf); return AVERROR_DEMUXER_NOT_FOUND; + } rt->asf_ctx = avformat_alloc_context(); if (!rt->asf_ctx) { -- 2.50.0.106.gf0135a9047.dirty
On Sun, Jun 29, 2025 at 02:45:26PM +0800, Lidong Yan wrote:
In ff_wms_parse_sdp_a_line(), it allocates memory in buf, but doesn't free buf when avformat_alloc_context() failed. Add av_free(buf) before return to prevent from leak.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> --- libavformat/rtpdec_asf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB it is not once nor twice but times without number that the same ideas make their appearance in the world. -- Aristotle
participants (2)
-
Lidong Yan -
Michael Niedermayer