[FFmpeg-devel] [PATCH 1/4] avcodec/hevcdec: Fix memleak of a53_caption

[Michael Niedermayer]

On Mon, Jul 01, 2019 at 07:11:50PM -0300, James Almer wrote:
> On 7/1/2019 11:24 AM, Michael Niedermayer wrote:
> > On Sun, Jun 30, 2019 at 11:18:55PM -0300, James Almer wrote:
> >> On 6/30/2019 10:43 PM, James Almer wrote:
> >>> On 6/30/2019 7:16 PM, Michael Niedermayer wrote:
> >>>> Fixes: 15295/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5675655187922944
> >>>>
> >>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >>>> ---
> >>>>  libavcodec/hevcdec.c | 2 ++
> >>>>  1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> >>>> index 515b346535..b5d918d07d 100644
> >>>> --- a/libavcodec/hevcdec.c
> >>>> +++ b/libavcodec/hevcdec.c
> >>>> @@ -3331,6 +3331,8 @@ static av_cold int hevc_decode_free(AVCodecContext *avctx)
> >>>>  
> >>>>      ff_h2645_packet_uninit(&s->pkt);
> >>>>  
> >>>> +    ff_hevc_reset_sei(&s->sei);
> >>>> +
> >>>>      return 0;
> >>>>  }
> >>>
> >>> LGTM.
> >>
> >> You could also add it to hevc_decode_flush() while at it.
> > 
> > will post a patch once i (lightly) tested it
> 
> Do you have hevc samples with closed captions? I'm not sure if there's a

no, i did not find one on my disk


> case where one such caption would not be attached to the frame (And thus
> removed from the decoder context) before a call to
> avcodec_flush_buffers() takes place. Maybe some sample where slices are
> in different packets.
> 

> For that matter, h264 seems to handle closed captions a lot better than
> hevc. It uses AVBufferRefs to keep them in sync between frame thread
> contexts, and it also makes sure to reset frame-specific SEI state on
> all frames and on flushing, all things hevc currently doesn't.
> Implementing the former should be trivial, but the latter isn't as slice
> handling is different.

for a bugfix that we should backport as it may have some leak->OOM potential
redesigning the handling is problematic. As backporting that would likely
not happen ...

So i suggest to go with a simpler solution like the original patch or
one with the call also in hevc_decode_flush()

Are you ok with this ?
if so do you prefer the call also be done in flush ?
also this issue is approaching the deadline ...

Thanks

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190909/a3ce0b7c/attachment.sig>