* [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in @ 2023-04-16 22:25 Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 2/5] avformat/avs: unref packet after avs_read_audio_packet() fail Michael Niedermayer ` (4 more replies) 0 siblings, 5 replies; 18+ messages in thread From: Michael Niedermayer @ 2023-04-16 22:25 UTC (permalink / raw) To: FFmpeg development discussions and patches Fixes: memleak Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_BSF_PCM_RECHUNK_fuzzer-5562089618407424 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/pcm_rechunk_bsf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c index 108d9e90b99..3f43934fe9a 100644 --- a/libavcodec/pcm_rechunk_bsf.c +++ b/libavcodec/pcm_rechunk_bsf.c @@ -153,6 +153,7 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) } } + av_packet_unref(s->in_pkt); ret = ff_bsf_get_packet_ref(ctx, s->in_pkt); if (ret == AVERROR_EOF && s->out_pkt->size) { if (s->pad) { -- 2.17.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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* [FFmpeg-devel] [PATCH 2/5] avformat/avs: unref packet after avs_read_audio_packet() fail 2023-04-16 22:25 [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in Michael Niedermayer @ 2023-04-16 22:25 ` Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc Michael Niedermayer ` (3 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Michael Niedermayer @ 2023-04-16 22:25 UTC (permalink / raw) To: FFmpeg development discussions and patches Fixes: memleak Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_dem_AVS_fuzzer-6738814988320768 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/avs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavformat/avs.c b/libavformat/avs.c index ab47980a11c..2ce8b19c412 100644 --- a/libavformat/avs.c +++ b/libavformat/avs.c @@ -156,9 +156,11 @@ static int avs_read_packet(AVFormatContext * s, AVPacket * pkt) uint8_t palette[4 + 3 * 256]; int ret; - if (avs->remaining_audio_size > 0) + if (avs->remaining_audio_size > 0) { if (avs_read_audio_packet(s, pkt) > 0) return 0; + av_packet_unref(pkt); + } while (1) { if (avs->remaining_frame_size <= 0) { -- 2.17.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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc 2023-04-16 22:25 [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 2/5] avformat/avs: unref packet after avs_read_audio_packet() fail Michael Niedermayer @ 2023-04-16 22:25 ` Michael Niedermayer 2023-04-17 10:36 ` Anton Khirnov 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 4/5] avcodec/hevc_ps: Use get_ue_golomb() instead of get_ue_golomb_long() for depth Michael Niedermayer ` (2 subsequent siblings) 4 siblings, 1 reply; 18+ messages in thread From: Michael Niedermayer @ 2023-04-16 22:25 UTC (permalink / raw) To: FFmpeg development discussions and patches Fixes: memleak Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6674082962997248 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/mov.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 057fd872b10..6853bb324cf 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7777,7 +7777,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) return 0; } - if (c->fc->nb_streams) { + if (c->fc->nb_streams || c->avif_info) { av_log(c->fc, AV_LOG_INFO, "Duplicate iloc box found\n"); return 0; } -- 2.17.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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc Michael Niedermayer @ 2023-04-17 10:36 ` Anton Khirnov 2023-04-17 23:18 ` Michael Niedermayer 0 siblings, 1 reply; 18+ messages in thread From: Anton Khirnov @ 2023-04-17 10:36 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting Michael Niedermayer (2023-04-17 00:25:16) > Fixes: memleak > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6674082962997248 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/mov.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 057fd872b10..6853bb324cf 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -7777,7 +7777,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > return 0; > } > > - if (c->fc->nb_streams) { > + if (c->fc->nb_streams || c->avif_info) { This first condition is now redundant, is it not? -- Anton Khirnov _______________________________________________ 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc 2023-04-17 10:36 ` Anton Khirnov @ 2023-04-17 23:18 ` Michael Niedermayer 2023-04-25 22:22 ` Vignesh Venkatasubramanian 0 siblings, 1 reply; 18+ messages in thread From: Michael Niedermayer @ 2023-04-17 23:18 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Vignesh Venkatasubramanian [-- Attachment #1.1: Type: text/plain, Size: 1393 bytes --] On Mon, Apr 17, 2023 at 12:36:26PM +0200, Anton Khirnov wrote: > Quoting Michael Niedermayer (2023-04-17 00:25:16) > > Fixes: memleak > > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6674082962997248 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/mov.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index 057fd872b10..6853bb324cf 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -7777,7 +7777,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > return 0; > > } > > > > - if (c->fc->nb_streams) { > > + if (c->fc->nb_streams || c->avif_info) { > > This first condition is now redundant, is it not? Iam not sure what exactly happens if a trak occurs before Iam also not sure what happens if multiple meta tags occur triggering the avif stream addition, i may be missing something but the code seems not to expect that Adding the author of this code to CC thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In a rich man's house there is no place to spit but his face. -- Diogenes of Sinope [-- 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc 2023-04-17 23:18 ` Michael Niedermayer @ 2023-04-25 22:22 ` Vignesh Venkatasubramanian 2023-09-29 19:21 ` Michael Niedermayer 0 siblings, 1 reply; 18+ messages in thread From: Vignesh Venkatasubramanian @ 2023-04-25 22:22 UTC (permalink / raw) To: Michael Niedermayer; +Cc: FFmpeg development discussions and patches On Mon, Apr 17, 2023 at 4:18 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Mon, Apr 17, 2023 at 12:36:26PM +0200, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2023-04-17 00:25:16) > > > Fixes: memleak > > > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6674082962997248 > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavformat/mov.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > index 057fd872b10..6853bb324cf 100644 > > > --- a/libavformat/mov.c > > > +++ b/libavformat/mov.c > > > @@ -7777,7 +7777,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > > return 0; > > > } > > > > > > - if (c->fc->nb_streams) { > > > + if (c->fc->nb_streams || c->avif_info) { > > > > This first condition is now redundant, is it not? > > Iam not sure I think the second condition alone should be enough here. Either way, lgtm (if the current patch is more clearer for readers). > what exactly happens if a trak occurs before > If a trak occurs before, then the condition in the line above should take care of that case (!c->is_still_picture_avif). Because if a trak was found, it will not be considered a still picture. > Iam also not sure what happens if multiple meta tags occur triggering > the avif stream addition, i may be missing something but the code seems > not to expect that > Multiple meta tags are not allowed in the AVIF/HEIF specification. > Adding the author of this code to CC > > thx > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > In a rich man's house there is no place to spit but his face. > -- Diogenes of Sinope -- Vignesh _______________________________________________ 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc 2023-04-25 22:22 ` Vignesh Venkatasubramanian @ 2023-09-29 19:21 ` Michael Niedermayer 2023-10-03 22:56 ` Vignesh Venkat via ffmpeg-devel 0 siblings, 1 reply; 18+ messages in thread From: Michael Niedermayer @ 2023-09-29 19:21 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 2233 bytes --] On Tue, Apr 25, 2023 at 03:22:50PM -0700, Vignesh Venkatasubramanian wrote: > On Mon, Apr 17, 2023 at 4:18 PM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > On Mon, Apr 17, 2023 at 12:36:26PM +0200, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2023-04-17 00:25:16) > > > > Fixes: memleak > > > > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6674082962997248 > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > --- > > > > libavformat/mov.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > > index 057fd872b10..6853bb324cf 100644 > > > > --- a/libavformat/mov.c > > > > +++ b/libavformat/mov.c > > > > @@ -7777,7 +7777,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > > > return 0; > > > > } > > > > > > > > - if (c->fc->nb_streams) { > > > > + if (c->fc->nb_streams || c->avif_info) { > > > > > > This first condition is now redundant, is it not? > > > > Iam not sure > > I think the second condition alone should be enough here. Either way, > lgtm (if the current patch is more clearer for readers). Ill apply with teh first condition converted to an assert then > > > what exactly happens if a trak occurs before > > > > If a trak occurs before, then the condition in the line above should > take care of that case (!c->is_still_picture_avif). Because if a trak > was found, it will not be considered a still picture. > > > Iam also not sure what happens if multiple meta tags occur triggering > > the avif stream addition, i may be missing something but the code seems > > not to expect that > > > > Multiple meta tags are not allowed in the AVIF/HEIF specification. sure but what happens if the occur anyway, does the code handle that with no undefined behavior ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have often repented speaking, but never of holding my tongue. -- Xenocrates [-- 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc 2023-09-29 19:21 ` Michael Niedermayer @ 2023-10-03 22:56 ` Vignesh Venkat via ffmpeg-devel 2023-10-04 4:22 ` Vignesh Venkat via ffmpeg-devel 0 siblings, 1 reply; 18+ messages in thread From: Vignesh Venkat via ffmpeg-devel @ 2023-10-03 22:56 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Vignesh Venkat On Fri, Sep 29, 2023 at 12:21 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Tue, Apr 25, 2023 at 03:22:50PM -0700, Vignesh Venkatasubramanian wrote: > > On Mon, Apr 17, 2023 at 4:18 PM Michael Niedermayer > > <michael@niedermayer.cc> wrote: > > > > > > On Mon, Apr 17, 2023 at 12:36:26PM +0200, Anton Khirnov wrote: > > > > Quoting Michael Niedermayer (2023-04-17 00:25:16) > > > > > Fixes: memleak > > > > > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6674082962997248 > > > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > > --- > > > > > libavformat/mov.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > > > index 057fd872b10..6853bb324cf 100644 > > > > > --- a/libavformat/mov.c > > > > > +++ b/libavformat/mov.c > > > > > @@ -7777,7 +7777,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > > > > return 0; > > > > > } > > > > > > > > > > - if (c->fc->nb_streams) { > > > > > + if (c->fc->nb_streams || c->avif_info) { > > > > > > > > This first condition is now redundant, is it not? > > > > > > Iam not sure > > > > I think the second condition alone should be enough here. Either way, > > lgtm (if the current patch is more clearer for readers). > > Ill apply with teh first condition converted to an assert then > > sounds good. > > > > > what exactly happens if a trak occurs before > > > > > > > If a trak occurs before, then the condition in the line above should > > take care of that case (!c->is_still_picture_avif). Because if a trak > > was found, it will not be considered a still picture. > > > > > Iam also not sure what happens if multiple meta tags occur triggering > > > the avif stream addition, i may be missing something but the code seems > > > not to expect that > > > > > > > Multiple meta tags are not allowed in the AVIF/HEIF specification. > > sure but what happens if the occur anyway, does the code handle that > with no undefined behavior ? > yeah, the current code will treat each meta tag as a separate track (AVStream). This should be disallowed, i will send a patch to error out if more than one top-level meta box is seen. > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I have often repented speaking, but never of holding my tongue. > -- Xenocrates > _______________________________________________ > 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". -- Vignesh _______________________________________________ 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc 2023-10-03 22:56 ` Vignesh Venkat via ffmpeg-devel @ 2023-10-04 4:22 ` Vignesh Venkat via ffmpeg-devel 0 siblings, 0 replies; 18+ messages in thread From: Vignesh Venkat via ffmpeg-devel @ 2023-10-04 4:22 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Vignesh Venkat On Tue, Oct 3, 2023 at 3:56 PM Vignesh Venkat <vigneshv@google.com> wrote: > > On Fri, Sep 29, 2023 at 12:21 PM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > On Tue, Apr 25, 2023 at 03:22:50PM -0700, Vignesh Venkatasubramanian wrote: > > > On Mon, Apr 17, 2023 at 4:18 PM Michael Niedermayer > > > <michael@niedermayer.cc> wrote: > > > > > > > > On Mon, Apr 17, 2023 at 12:36:26PM +0200, Anton Khirnov wrote: > > > > > Quoting Michael Niedermayer (2023-04-17 00:25:16) > > > > > > Fixes: memleak > > > > > > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6674082962997248 > > > > > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > > > --- > > > > > > libavformat/mov.c | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > > > > index 057fd872b10..6853bb324cf 100644 > > > > > > --- a/libavformat/mov.c > > > > > > +++ b/libavformat/mov.c > > > > > > @@ -7777,7 +7777,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > > > > > return 0; > > > > > > } > > > > > > > > > > > > - if (c->fc->nb_streams) { > > > > > > + if (c->fc->nb_streams || c->avif_info) { > > > > > > > > > > This first condition is now redundant, is it not? > > > > > > > > Iam not sure > > > > > > I think the second condition alone should be enough here. Either way, > > > lgtm (if the current patch is more clearer for readers). > > > > Ill apply with teh first condition converted to an assert then > > > > > > sounds good. > > > > > > > > what exactly happens if a trak occurs before > > > > > > > > > > If a trak occurs before, then the condition in the line above should > > > take care of that case (!c->is_still_picture_avif). Because if a trak > > > was found, it will not be considered a still picture. > > > > > > > Iam also not sure what happens if multiple meta tags occur triggering > > > > the avif stream addition, i may be missing something but the code seems > > > > not to expect that > > > > > > > > > > Multiple meta tags are not allowed in the AVIF/HEIF specification. > > > > sure but what happens if the occur anyway, does the code handle that > > with no undefined behavior ? > > > > yeah, the current code will treat each meta tag as a separate track > (AVStream). This should be disallowed, i will send a patch to error > out if more than one top-level meta box is seen. > The patch for disallowing multiple meta boxes is here: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20231003230423.951161-1-vigneshv@google.com/ > > thx > > > > [...] > > -- > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > > > I have often repented speaking, but never of holding my tongue. > > -- Xenocrates > > _______________________________________________ > > 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". > > > > -- > Vignesh -- Vignesh _______________________________________________ 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* [FFmpeg-devel] [PATCH 4/5] avcodec/hevc_ps: Use get_ue_golomb() instead of get_ue_golomb_long() for depth 2023-04-16 22:25 [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 2/5] avformat/avs: unref packet after avs_read_audio_packet() fail Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc Michael Niedermayer @ 2023-04-16 22:25 ` Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 5/5] avcodec/hevc_ps: Check depth to be within 8 to 16 Michael Niedermayer 2023-04-16 22:57 ` [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in James Almer 4 siblings, 0 replies; 18+ messages in thread From: Michael Niedermayer @ 2023-04-16 22:25 UTC (permalink / raw) To: FFmpeg development discussions and patches Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/hevc_ps.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c index be1d668c263..2f505ba2f7a 100644 --- a/libavcodec/hevc_ps.c +++ b/libavcodec/hevc_ps.c @@ -1542,9 +1542,9 @@ static int pps_scc_extension(GetBitContext *gb, AVCodecContext *avctx, if (pps->pps_palette_predictor_initializers_present_flag = get_bits1(gb)) { if ((pps->pps_num_palette_predictor_initializers = get_ue_golomb_long(gb)) > 0) { pps->monochrome_palette_flag = get_bits1(gb); - pps->luma_bit_depth_entry_minus8 = get_ue_golomb_long(gb); + pps->luma_bit_depth_entry_minus8 = get_ue_golomb(gb); if (!pps->monochrome_palette_flag) - pps->chroma_bit_depth_entry_minus8 = get_ue_golomb_long(gb); + pps->chroma_bit_depth_entry_minus8 = get_ue_golomb(gb); num_comps = pps->monochrome_palette_flag ? 1 : 3; for (int comp = 0; comp < num_comps; comp++) for (int i = 0; i < pps->pps_num_palette_predictor_initializers; i++) -- 2.17.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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* [FFmpeg-devel] [PATCH 5/5] avcodec/hevc_ps: Check depth to be within 8 to 16 2023-04-16 22:25 [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in Michael Niedermayer ` (2 preceding siblings ...) 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 4/5] avcodec/hevc_ps: Use get_ue_golomb() instead of get_ue_golomb_long() for depth Michael Niedermayer @ 2023-04-16 22:25 ` Michael Niedermayer 2023-04-30 22:32 ` Michael Niedermayer 2023-04-16 22:57 ` [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in James Almer 4 siblings, 1 reply; 18+ messages in thread From: Michael Niedermayer @ 2023-04-16 22:25 UTC (permalink / raw) To: FFmpeg development discussions and patches Fixes: assertion failure in bitreader Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-627318668066816 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/hevc_ps.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c index 2f505ba2f7a..d8664ebda7b 100644 --- a/libavcodec/hevc_ps.c +++ b/libavcodec/hevc_ps.c @@ -1545,6 +1545,10 @@ static int pps_scc_extension(GetBitContext *gb, AVCodecContext *avctx, pps->luma_bit_depth_entry_minus8 = get_ue_golomb(gb); if (!pps->monochrome_palette_flag) pps->chroma_bit_depth_entry_minus8 = get_ue_golomb(gb); + + if (pps->chroma_bit_depth_entry_minus8 > 8 || pps->chroma_bit_depth_entry_minus8 > 8) + return AVERROR_INVALIDDATA; + num_comps = pps->monochrome_palette_flag ? 1 : 3; for (int comp = 0; comp < num_comps; comp++) for (int i = 0; i < pps->pps_num_palette_predictor_initializers; i++) -- 2.17.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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 5/5] avcodec/hevc_ps: Check depth to be within 8 to 16 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 5/5] avcodec/hevc_ps: Check depth to be within 8 to 16 Michael Niedermayer @ 2023-04-30 22:32 ` Michael Niedermayer 0 siblings, 0 replies; 18+ messages in thread From: Michael Niedermayer @ 2023-04-30 22:32 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 717 bytes --] On Mon, Apr 17, 2023 at 12:25:18AM +0200, Michael Niedermayer wrote: > Fixes: assertion failure in bitreader > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-627318668066816 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/hevc_ps.c | 4 ++++ > 1 file changed, 4 insertions(+) will apply with adaptions to current git master [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus [-- 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in 2023-04-16 22:25 [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in Michael Niedermayer ` (3 preceding siblings ...) 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 5/5] avcodec/hevc_ps: Check depth to be within 8 to 16 Michael Niedermayer @ 2023-04-16 22:57 ` James Almer 2023-04-17 11:26 ` Michael Niedermayer 4 siblings, 1 reply; 18+ messages in thread From: James Almer @ 2023-04-16 22:57 UTC (permalink / raw) To: ffmpeg-devel On 4/16/2023 7:25 PM, Michael Niedermayer wrote: > Fixes: memleak > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_BSF_PCM_RECHUNK_fuzzer-5562089618407424 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/pcm_rechunk_bsf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c > index 108d9e90b99..3f43934fe9a 100644 > --- a/libavcodec/pcm_rechunk_bsf.c > +++ b/libavcodec/pcm_rechunk_bsf.c > @@ -153,6 +153,7 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) > } > } > > + av_packet_unref(s->in_pkt); This could be pointing to a bug in the above code, and unreffing here like this would result in data loss. Does the following change also fix the memleak? > diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c > index 108d9e90b9..032f914916 100644 > --- a/libavcodec/pcm_rechunk_bsf.c > +++ b/libavcodec/pcm_rechunk_bsf.c > @@ -151,7 +151,8 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) > av_packet_move_ref(pkt, s->in_pkt); > return send_packet(s, nb_samples, pkt); > } > - } > + } else > + av_packet_unref(s->in_pkt); > > ret = ff_bsf_get_packet_ref(ctx, s->in_pkt); > if (ret == AVERROR_EOF && s->out_pkt->size) { If it does then a zero sized packet with either only side data or that went through the av_packet_make_refcounted() in av_bsf_send_packet() that left it with an allocated padding buffer was fed to this bsf. _______________________________________________ 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in 2023-04-16 22:57 ` [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in James Almer @ 2023-04-17 11:26 ` Michael Niedermayer 2023-04-17 11:28 ` James Almer 0 siblings, 1 reply; 18+ messages in thread From: Michael Niedermayer @ 2023-04-17 11:26 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 2408 bytes --] On Sun, Apr 16, 2023 at 07:57:00PM -0300, James Almer wrote: > On 4/16/2023 7:25 PM, Michael Niedermayer wrote: > > Fixes: memleak > > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_BSF_PCM_RECHUNK_fuzzer-5562089618407424 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/pcm_rechunk_bsf.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c > > index 108d9e90b99..3f43934fe9a 100644 > > --- a/libavcodec/pcm_rechunk_bsf.c > > +++ b/libavcodec/pcm_rechunk_bsf.c > > @@ -153,6 +153,7 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) > > } > > } > > + av_packet_unref(s->in_pkt); > > This could be pointing to a bug in the above code, and unreffing here like > this would result in data loss. > > Does the following change also fix the memleak? > > > diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c > > index 108d9e90b9..032f914916 100644 > > --- a/libavcodec/pcm_rechunk_bsf.c > > +++ b/libavcodec/pcm_rechunk_bsf.c > > @@ -151,7 +151,8 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) > > av_packet_move_ref(pkt, s->in_pkt); > > return send_packet(s, nb_samples, pkt); > > } > > - } > > + } else > > + av_packet_unref(s->in_pkt); > > > > ret = ff_bsf_get_packet_ref(ctx, s->in_pkt); > > if (ret == AVERROR_EOF && s->out_pkt->size) { > > If it does then a zero sized packet with either only side data or that went > through the av_packet_make_refcounted() in av_bsf_send_packet() that left it > with an allocated padding buffer was fed to this bsf. yes this works too and this memleak is open since a year, its the 2nd time i submited this patch, last time the discussions didnt lead to any consensus on what to do I hope this time some fix from someone ends up in git thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 2 "100% positive feedback" - "All either got their money back or didnt complain" "Best seller ever, very honest" - "Seller refunded buyer after failed scam" [-- 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in 2023-04-17 11:26 ` Michael Niedermayer @ 2023-04-17 11:28 ` James Almer 2023-04-17 11:34 ` Michael Niedermayer 0 siblings, 1 reply; 18+ messages in thread From: James Almer @ 2023-04-17 11:28 UTC (permalink / raw) To: ffmpeg-devel On 4/17/2023 8:26 AM, Michael Niedermayer wrote: > On Sun, Apr 16, 2023 at 07:57:00PM -0300, James Almer wrote: >> On 4/16/2023 7:25 PM, Michael Niedermayer wrote: >>> Fixes: memleak >>> Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_BSF_PCM_RECHUNK_fuzzer-5562089618407424 >>> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavcodec/pcm_rechunk_bsf.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c >>> index 108d9e90b99..3f43934fe9a 100644 >>> --- a/libavcodec/pcm_rechunk_bsf.c >>> +++ b/libavcodec/pcm_rechunk_bsf.c >>> @@ -153,6 +153,7 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) >>> } >>> } >>> + av_packet_unref(s->in_pkt); >> >> This could be pointing to a bug in the above code, and unreffing here like >> this would result in data loss. >> >> Does the following change also fix the memleak? >> >>> diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c >>> index 108d9e90b9..032f914916 100644 >>> --- a/libavcodec/pcm_rechunk_bsf.c >>> +++ b/libavcodec/pcm_rechunk_bsf.c >>> @@ -151,7 +151,8 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) >>> av_packet_move_ref(pkt, s->in_pkt); >>> return send_packet(s, nb_samples, pkt); >>> } >>> - } >>> + } else >>> + av_packet_unref(s->in_pkt); >>> >>> ret = ff_bsf_get_packet_ref(ctx, s->in_pkt); >>> if (ret == AVERROR_EOF && s->out_pkt->size) { >> >> If it does then a zero sized packet with either only side data or that went >> through the av_packet_make_refcounted() in av_bsf_send_packet() that left it >> with an allocated padding buffer was fed to this bsf. > > yes this works too > and this memleak is open since a year, its the 2nd time i submited this Yes, i had a sense of deja vu. > patch, last time the discussions didnt lead to any consensus on what to do > I hope this time some fix from someone ends up in git > > thx Just apply the suggested change i made above. _______________________________________________ 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in 2023-04-17 11:28 ` James Almer @ 2023-04-17 11:34 ` Michael Niedermayer 2023-04-17 17:13 ` Marton Balint 0 siblings, 1 reply; 18+ messages in thread From: Michael Niedermayer @ 2023-04-17 11:34 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 2769 bytes --] On Mon, Apr 17, 2023 at 08:28:37AM -0300, James Almer wrote: > On 4/17/2023 8:26 AM, Michael Niedermayer wrote: > > On Sun, Apr 16, 2023 at 07:57:00PM -0300, James Almer wrote: > > > On 4/16/2023 7:25 PM, Michael Niedermayer wrote: > > > > Fixes: memleak > > > > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_BSF_PCM_RECHUNK_fuzzer-5562089618407424 > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > --- > > > > libavcodec/pcm_rechunk_bsf.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c > > > > index 108d9e90b99..3f43934fe9a 100644 > > > > --- a/libavcodec/pcm_rechunk_bsf.c > > > > +++ b/libavcodec/pcm_rechunk_bsf.c > > > > @@ -153,6 +153,7 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) > > > > } > > > > } > > > > + av_packet_unref(s->in_pkt); > > > > > > This could be pointing to a bug in the above code, and unreffing here like > > > this would result in data loss. > > > > > > Does the following change also fix the memleak? > > > > > > > diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c > > > > index 108d9e90b9..032f914916 100644 > > > > --- a/libavcodec/pcm_rechunk_bsf.c > > > > +++ b/libavcodec/pcm_rechunk_bsf.c > > > > @@ -151,7 +151,8 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) > > > > av_packet_move_ref(pkt, s->in_pkt); > > > > return send_packet(s, nb_samples, pkt); > > > > } > > > > - } > > > > + } else > > > > + av_packet_unref(s->in_pkt); > > > > > > > > ret = ff_bsf_get_packet_ref(ctx, s->in_pkt); > > > > if (ret == AVERROR_EOF && s->out_pkt->size) { > > > > > > If it does then a zero sized packet with either only side data or that went > > > through the av_packet_make_refcounted() in av_bsf_send_packet() that left it > > > with an allocated padding buffer was fed to this bsf. > > > > yes this works too > > and this memleak is open since a year, its the 2nd time i submited this > > Yes, i had a sense of deja vu. > > > patch, last time the discussions didnt lead to any consensus on what to do > > I hope this time some fix from someone ends up in git > > > > thx > > Just apply the suggested change i made above. ok 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 [-- 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in 2023-04-17 11:34 ` Michael Niedermayer @ 2023-04-17 17:13 ` Marton Balint 2023-04-17 17:23 ` James Almer 0 siblings, 1 reply; 18+ messages in thread From: Marton Balint @ 2023-04-17 17:13 UTC (permalink / raw) To: FFmpeg development discussions and patches On Mon, 17 Apr 2023, Michael Niedermayer wrote: > On Mon, Apr 17, 2023 at 08:28:37AM -0300, James Almer wrote: >> On 4/17/2023 8:26 AM, Michael Niedermayer wrote: >>> On Sun, Apr 16, 2023 at 07:57:00PM -0300, James Almer wrote: >>>> On 4/16/2023 7:25 PM, Michael Niedermayer wrote: >>>>> Fixes: memleak >>>>> Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_BSF_PCM_RECHUNK_fuzzer-5562089618407424 >>>>> >>>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>>>> --- >>>>> libavcodec/pcm_rechunk_bsf.c | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c >>>>> index 108d9e90b99..3f43934fe9a 100644 >>>>> --- a/libavcodec/pcm_rechunk_bsf.c >>>>> +++ b/libavcodec/pcm_rechunk_bsf.c >>>>> @@ -153,6 +153,7 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) >>>>> } >>>>> } >>>>> + av_packet_unref(s->in_pkt); >>>> >>>> This could be pointing to a bug in the above code, and unreffing here like >>>> this would result in data loss. >>>> >>>> Does the following change also fix the memleak? >>>> >>>>> diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c >>>>> index 108d9e90b9..032f914916 100644 >>>>> --- a/libavcodec/pcm_rechunk_bsf.c >>>>> +++ b/libavcodec/pcm_rechunk_bsf.c >>>>> @@ -151,7 +151,8 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) >>>>> av_packet_move_ref(pkt, s->in_pkt); >>>>> return send_packet(s, nb_samples, pkt); >>>>> } >>>>> - } >>>>> + } else >>>>> + av_packet_unref(s->in_pkt); >>>>> >>>>> ret = ff_bsf_get_packet_ref(ctx, s->in_pkt); >>>>> if (ret == AVERROR_EOF && s->out_pkt->size) { >>>> >>>> If it does then a zero sized packet with either only side data or that went >>>> through the av_packet_make_refcounted() in av_bsf_send_packet() that left it >>>> with an allocated padding buffer was fed to this bsf. >>> >>> yes this works too >>> and this memleak is open since a year, its the 2nd time i submited this >> >> Yes, i had a sense of deja vu. >> >>> patch, last time the discussions didnt lead to any consensus on what to do >>> I hope this time some fix from someone ends up in git >>> >>> thx >> >> Just apply the suggested change i made above. > > ok That is fine by me as well to fix the leak. However I still wonder if it is valid to pass empty packets around. AFAIK the only documented case is when some final side data is passed at the end of the encoding, and these fuzzing issues are typically not that, but that some demuxer generates 0-sized packets for corrupt files. Regards, Marton _______________________________________________ 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in 2023-04-17 17:13 ` Marton Balint @ 2023-04-17 17:23 ` James Almer 0 siblings, 0 replies; 18+ messages in thread From: James Almer @ 2023-04-17 17:23 UTC (permalink / raw) To: ffmpeg-devel On 4/17/2023 2:13 PM, Marton Balint wrote: > > > On Mon, 17 Apr 2023, Michael Niedermayer wrote: > >> On Mon, Apr 17, 2023 at 08:28:37AM -0300, James Almer wrote: >>> On 4/17/2023 8:26 AM, Michael Niedermayer wrote: >>>> On Sun, Apr 16, 2023 at 07:57:00PM -0300, James Almer wrote: >>>>> On 4/16/2023 7:25 PM, Michael Niedermayer wrote: >>>>>> Fixes: memleak >>>>>> Fixes: >>>>>> 45982/clusterfuzz-testcase-minimized-ffmpeg_BSF_PCM_RECHUNK_fuzzer-5562089618407424 >>>>>> >>>>>> Found-by: continuous fuzzing process >>>>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>>>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>>>>> --- >>>>>> libavcodec/pcm_rechunk_bsf.c | 1 + >>>>>> 1 file changed, 1 insertion(+) >>>>>> >>>>>> diff --git a/libavcodec/pcm_rechunk_bsf.c >>>>>> b/libavcodec/pcm_rechunk_bsf.c >>>>>> index 108d9e90b99..3f43934fe9a 100644 >>>>>> --- a/libavcodec/pcm_rechunk_bsf.c >>>>>> +++ b/libavcodec/pcm_rechunk_bsf.c >>>>>> @@ -153,6 +153,7 @@ static int rechunk_filter(AVBSFContext *ctx, >>>>>> AVPacket *pkt) >>>>>> } >>>>>> } >>>>>> + av_packet_unref(s->in_pkt); >>>>> >>>>> This could be pointing to a bug in the above code, and unreffing >>>>> here like >>>>> this would result in data loss. >>>>> >>>>> Does the following change also fix the memleak? >>>>> >>>>>> diff --git a/libavcodec/pcm_rechunk_bsf.c >>>>>> b/libavcodec/pcm_rechunk_bsf.c >>>>>> index 108d9e90b9..032f914916 100644 >>>>>> --- a/libavcodec/pcm_rechunk_bsf.c >>>>>> +++ b/libavcodec/pcm_rechunk_bsf.c >>>>>> @@ -151,7 +151,8 @@ static int rechunk_filter(AVBSFContext *ctx, >>>>>> AVPacket *pkt) >>>>>> av_packet_move_ref(pkt, s->in_pkt); >>>>>> return send_packet(s, nb_samples, pkt); >>>>>> } >>>>>> - } >>>>>> + } else >>>>>> + av_packet_unref(s->in_pkt); >>>>>> >>>>>> ret = ff_bsf_get_packet_ref(ctx, s->in_pkt); >>>>>> if (ret == AVERROR_EOF && s->out_pkt->size) { >>>>> >>>>> If it does then a zero sized packet with either only side data or >>>>> that went >>>>> through the av_packet_make_refcounted() in av_bsf_send_packet() >>>>> that left it >>>>> with an allocated padding buffer was fed to this bsf. >>>> >>>> yes this works too >>>> and this memleak is open since a year, its the 2nd time i submited this >>> >>> Yes, i had a sense of deja vu. >>> >>>> patch, last time the discussions didnt lead to any consensus on what >>>> to do >>>> I hope this time some fix from someone ends up in git >>>> >>>> thx >>> >>> Just apply the suggested change i made above. >> >> ok > > That is fine by me as well to fix the leak. > > However I still wonder if it is valid to pass empty packets around. The bsf API accepts zero sized packets as long as pkt->data or pkt->side_data are not NULL. > AFAIK the only documented case is when some final side data is passed at > the end of the encoding, and these fuzzing issues are typically not > that, but that some demuxer generates 0-sized packets for corrupt files. No, the fuzzer uses tools/target_bsf_fuzzer.c to create arbitrary packets that are passed to the bsf public API. In some cases that apparently includes zero sized packets. > > Regards, > Marton _______________________________________________ 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-10-04 4:23 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-16 22:25 [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 2/5] avformat/avs: unref packet after avs_read_audio_packet() fail Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc Michael Niedermayer 2023-04-17 10:36 ` Anton Khirnov 2023-04-17 23:18 ` Michael Niedermayer 2023-04-25 22:22 ` Vignesh Venkatasubramanian 2023-09-29 19:21 ` Michael Niedermayer 2023-10-03 22:56 ` Vignesh Venkat via ffmpeg-devel 2023-10-04 4:22 ` Vignesh Venkat via ffmpeg-devel 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 4/5] avcodec/hevc_ps: Use get_ue_golomb() instead of get_ue_golomb_long() for depth Michael Niedermayer 2023-04-16 22:25 ` [FFmpeg-devel] [PATCH 5/5] avcodec/hevc_ps: Check depth to be within 8 to 16 Michael Niedermayer 2023-04-30 22:32 ` Michael Niedermayer 2023-04-16 22:57 ` [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in James Almer 2023-04-17 11:26 ` Michael Niedermayer 2023-04-17 11:28 ` James Almer 2023-04-17 11:34 ` Michael Niedermayer 2023-04-17 17:13 ` Marton Balint 2023-04-17 17:23 ` James Almer
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git