diff mbox

[FFmpeg-devel] Fix memory leak in lrcdec.c

Message ID 20180119211707.157574-1-nbowe@google.com
State Accepted
Commit ef5994e09d07ace62a672fcdc84761231288edad
Headers show

Commit Message

Niki Bowe Jan. 19, 2018, 9:17 p.m. UTC
---
 libavformat/lrcdec.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Michael Niedermayer Jan. 20, 2018, 12:57 a.m. UTC | #1
On Fri, Jan 19, 2018 at 01:17:07PM -0800, Nikolas Bowe wrote:
> ---
>  libavformat/lrcdec.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/lrcdec.c b/libavformat/lrcdec.c
> index 12f74b22a0..f4e9a4efa9 100644
> --- a/libavformat/lrcdec.c
> +++ b/libavformat/lrcdec.c
> @@ -212,6 +212,7 @@ static int lrc_read_header(AVFormatContext *s)
>      }
>      ff_subtitles_queue_finalize(s, &lrc->q);
>      ff_metadata_conv_ctx(s, NULL, ff_lrc_metadata_conv);
> +    av_bprint_finalize(&line, NULL);
>      return 0;
>  }

How did you find this ?

iam asking because ideally all fuzzing stuff should be run by
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
while currently thats not the case and disk space restrictions
togther with "no shared libs allowed" and a few other things
like one seperate tool for each test make it hard to extend it.
None the less, if some restriction could be lifted or some other
thing could be done to run more complete fuzzing in FFmpeg oss-fuzz
that would be great.

[...]
Carl Eugen Hoyos Jan. 20, 2018, 1:39 a.m. UTC | #2
2018-01-20 1:57 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
> On Fri, Jan 19, 2018 at 01:17:07PM -0800, Nikolas Bowe wrote:
>> ---
>>  libavformat/lrcdec.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/lrcdec.c b/libavformat/lrcdec.c
>> index 12f74b22a0..f4e9a4efa9 100644
>> --- a/libavformat/lrcdec.c
>> +++ b/libavformat/lrcdec.c
>> @@ -212,6 +212,7 @@ static int lrc_read_header(AVFormatContext *s)
>>      }
>>      ff_subtitles_queue_finalize(s, &lrc->q);
>>      ff_metadata_conv_ctx(s, NULL, ff_lrc_metadata_conv);
>> +    av_bprint_finalize(&line, NULL);
>>      return 0;
>>  }
>
> How did you find this ?
>
> iam asking because ideally all fuzzing stuff should be run by
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> while currently thats not the case and disk space restrictions

> togther with "no shared libs allowed" and a few other things

(Sorry if I misread this)
Why is "no shared libs" an issue with FFmpeg?

> like one seperate tool for each test make it hard to extend it.

Carl Eugen
Michael Niedermayer Jan. 20, 2018, 2:10 a.m. UTC | #3
On Sat, Jan 20, 2018 at 02:39:02AM +0100, Carl Eugen Hoyos wrote:
> 2018-01-20 1:57 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
> > On Fri, Jan 19, 2018 at 01:17:07PM -0800, Nikolas Bowe wrote:
> >> ---
> >>  libavformat/lrcdec.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/libavformat/lrcdec.c b/libavformat/lrcdec.c
> >> index 12f74b22a0..f4e9a4efa9 100644
> >> --- a/libavformat/lrcdec.c
> >> +++ b/libavformat/lrcdec.c
> >> @@ -212,6 +212,7 @@ static int lrc_read_header(AVFormatContext *s)
> >>      }
> >>      ff_subtitles_queue_finalize(s, &lrc->q);
> >>      ff_metadata_conv_ctx(s, NULL, ff_lrc_metadata_conv);
> >> +    av_bprint_finalize(&line, NULL);
> >>      return 0;
> >>  }
> >
> > How did you find this ?
> >
> > iam asking because ideally all fuzzing stuff should be run by
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > while currently thats not the case and disk space restrictions
> 
> > togther with "no shared libs allowed" and a few other things
> 
> (Sorry if I misread this)
> Why is "no shared libs" an issue with FFmpeg?

its an issue for the oss fuzz testcases it seems.
all code is statically linked even libs which on a modern system
are linked as shared libs normally
dont ask me why. I very much would love to switch to shared linking
also i would be very happy to find out this is a misunderstanding 
somehow and we can use shared linking ...

that together with one binary per tested codec causes space issues
which should not come as much of a surprise. Duplicating libavcodec
for each test is not great.
On top of that fate samples are partly duplicated too

also see:
https://github.com/google/oss-fuzz/issues/567

[...]
Carl Eugen Hoyos Jan. 20, 2018, 2:40 a.m. UTC | #4
2018-01-20 3:10 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
> On Sat, Jan 20, 2018 at 02:39:02AM +0100, Carl Eugen Hoyos wrote:
>> 2018-01-20 1:57 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
>> > On Fri, Jan 19, 2018 at 01:17:07PM -0800, Nikolas Bowe wrote:
>> >> ---
>> >>  libavformat/lrcdec.c | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/libavformat/lrcdec.c b/libavformat/lrcdec.c
>> >> index 12f74b22a0..f4e9a4efa9 100644
>> >> --- a/libavformat/lrcdec.c
>> >> +++ b/libavformat/lrcdec.c
>> >> @@ -212,6 +212,7 @@ static int lrc_read_header(AVFormatContext *s)
>> >>      }
>> >>      ff_subtitles_queue_finalize(s, &lrc->q);
>> >>      ff_metadata_conv_ctx(s, NULL, ff_lrc_metadata_conv);
>> >> +    av_bprint_finalize(&line, NULL);
>> >>      return 0;
>> >>  }
>> >
>> > How did you find this ?
>> >
>> > iam asking because ideally all fuzzing stuff should be run by
>> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> > while currently thats not the case and disk space restrictions
>>
>> > togther with "no shared libs allowed" and a few other things
>>
>> (Sorry if I misread this)
>> Why is "no shared libs" an issue with FFmpeg?

What I actually meant was:
Is there a dependency for which static linking does not work...

> its an issue for the oss fuzz testcases it seems.
> all code is statically linked even libs which on a modern system
> are linked as shared libs normally
> dont ask me why. I very much would love to switch to shared linking
> also i would be very happy to find out this is a misunderstanding
> somehow and we can use shared linking ...
>
> that together with one binary per tested codec causes space issues
> which should not come as much of a surprise. Duplicating libavcodec
> for each test is not great.
> On top of that fate samples are partly duplicated too
>
> also see:
> https://github.com/google/oss-fuzz/issues/567

... but thank you for this explanation!

Carl Eugen
Michael Niedermayer Jan. 20, 2018, 8:21 p.m. UTC | #5
On Fri, Jan 19, 2018 at 01:17:07PM -0800, Nikolas Bowe wrote:
> ---
>  libavformat/lrcdec.c | 1 +
>  1 file changed, 1 insertion(+)

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavformat/lrcdec.c b/libavformat/lrcdec.c
index 12f74b22a0..f4e9a4efa9 100644
--- a/libavformat/lrcdec.c
+++ b/libavformat/lrcdec.c
@@ -212,6 +212,7 @@  static int lrc_read_header(AVFormatContext *s)
     }
     ff_subtitles_queue_finalize(s, &lrc->q);
     ff_metadata_conv_ctx(s, NULL, ff_lrc_metadata_conv);
+    av_bprint_finalize(&line, NULL);
     return 0;
 }