[FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[Michael Niedermayer]

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".

[FFmpeg-devel] [PATCH 2/5] avformat/avs: unref packet after avs_read_audio_packet() fail

[Michael Niedermayer]

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".

[FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc

[Michael Niedermayer]

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".

[FFmpeg-devel] [PATCH 4/5] avcodec/hevc_ps: Use get_ue_golomb() instead of get_ue_golomb_long() for depth

[Michael Niedermayer]

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".

[FFmpeg-devel] [PATCH 5/5] avcodec/hevc_ps: Check depth to be within 8 to 16

[Michael Niedermayer]

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".

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[James Almer]

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".

Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc

[Anton Khirnov]

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".

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[Michael Niedermayer]

[-- 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".

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[James Almer]

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".

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[Michael Niedermayer]

[-- 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".

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[Marton Balint]

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".

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[James Almer]

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".

Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc

[Michael Niedermayer]

[-- 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".

Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc

[Vignesh Venkatasubramanian]

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".

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/hevc_ps: Check depth to be within 8 to 16

[Michael Niedermayer]

[-- 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".

Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc

[Michael Niedermayer]

[-- 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".

Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc

[Vignesh Venkat via ffmpeg-devel]

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".

Re: [FFmpeg-devel] [PATCH 3/5] avformat/mov: Better check for duplicate iloc

[Vignesh Venkat via ffmpeg-devel]

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".