[FFmpeg-user] ffmpeg crashes playing chained streams

Ilja Friedel ihf at google.com
Tue Jun 28 06:18:54 CEST 2011


This is a discussion of the crashes I see with the new ffmpeg library and
change http://codereview.chromium.org/6993042 to glue things together for
Chromium. It looks like there might be a double free in oggdec.c.

Whenever I play one of the chained streams files
http://v2v.cc/~j/theora_testsuite/chained_streams.ogg
http://v2v.cc/~j/theora_testsuite/multi2.ogg
and especially after hitting reload a few times I see crashes (but not with
any other sample movie). To make a long story short this is caused by

av_free(ogg->streams[n].private);
that appeared in
http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=47dec30edb8565b7e0e8716dc6d0dc36d5b7bc40

and made worse by the merge of
@@ -474,17 +477,30 @@ static int ogg_get_length(AVFormatContext *s)
in
http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/oggdec.c;h=e33535860155917c630933589a2816ca8e2ce4ed;hp=f1ad630c5e43e9666dd3d004ea72de98785da41f;hb=99eb31e263a24bc6c5a7a3f455a2bcb04a60e70e;hpb=b751f611065f1fe1d7216971c4b100c928a3b0d5


To discuss what is happening I am referring to the line numbering in
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/oggdec.c;h=e33535860155917c630933589a2816ca8e2ce4ed;hb=e33535860155917c630933589a2816ca8e2ce4ed

447 static int ogg_get_length(AVFormatContext *s)
448 {
449     struct ogg *ogg = s->priv_data;
[...]
465     ogg_save (s);                                    <-- saves shallow
copy of s
466     avio_seek (s->pb, end, SEEK_SET);
467
468     while (!ogg_read_page (s, &i)){           <-- calls
av_free(ogg->streams[n].private); and deletes pointer from shallow copy
[...]
478     ogg_restore (s, 0);                             <-- restores shallow
copy (but memory that private points to has been already reused by Chrome
479
480     ogg_save (s);
481     avio_seek (s->pb, 0, SEEK_SET);
482     while (!ogg_read_page (s, &i)){           <-- in theora_gptopts it
reads private from shallow copy, e.g. memory that has been overwritten by
Chrome
[...]
490     ogg_restore (s, 0);
491
492     return 0;
493 }

Now if I replace in ogg_read_page
                   //av_freep(&ogg->streams[n].buf);
                   //av_freep(&ogg->streams[n].private);
                   printf("   ogg_read_page skipped av_freep from ogg=0x%08x
&private=0x%8x private=0x%08x\n", ogg, &ogg->streams[n].private,
ogg->streams[n].private);
                   ogg->streams[n].buf = NULL;
                   ogg->streams[n].private = NULL;

I get the first trace below which has no memory leak and no double free. To
me it looks like
http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=47dec30edb8565b7e0e8716dc6d0dc36d5b7bc40
assumed that the copy was a deep one. Or maybe I am missing something
completely. In any case, my proposed fix would be the previous five lines of
code, e.g. no free but zeroing out the pointers of the shallow copy just in
case. Any thoughts?

Thanks,


Ilja.

1) Trace with new code:
++ theora_header &os->private=0x0ec1d8e8  os->private=0x0ec521c0
-> ogg_get_length(s=0x0eb12500)
  private=0x0ec521c0
  ogg_save(s=0x0eb12500)  private=0x0ec521c0

  ogg_read_page skipped av_freep from ogg=0x0ec52080 &private=0x ec1d8e8
private=0x0ec521c0
  ogg_read_page skipped av_freep from ogg=0x0ec52080 &private=0x ec1d8e8
private=0x00000000
   [...]
  ogg_read_page skipped av_freep from ogg=0x0ec52080 &private=0x ec1d8e8
private=0x00000000

-> ogg_restore(s=0x0eb12500, discard=0)  private=0x00000000       on
entering ogg_restore private is NULL
<- ogg_restore(s=0x0eb12500, discard=0)  private=0x0ec521c0        on
leaving pointer has been copied back
-> ogg_save(s=0x0eb12500) private=0x0ec521c0
-> ogg_restore(s=0x0eb12500, discard=0) private=0x0ec521c0
<- ogg_get_length(0x0eb12500)   private=0x0ec521c0

[hitting reload]
** ogg_read_close av_free from ogg=0x0ec52080 &private=0x ec1d8e8
private=0x0ec521c0



2) And finally a trace using valgrind showing the original issue without
fix. With fix there is no output in this part of the code.
==436== Thread 4:
==436== Invalid read of size 1
==436==    at 0x1E6A1C15: theora_gptopts (oggparsetheora.c:132)
==436==    by 0x1E69FB39: ogg_gptopts (oggdec.h:137)
==436==    by 0x1E6A10E0: ogg_get_length (oggdec.c:498)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
==436==  Address 0x2baefa80 is 0 bytes inside a block of size 12 free'd
==436==    at 0x7DE8ABE: free (vg_replace_malloc.c:913)
==436==    by 0x1E6B6A9A: av_free (mem.c:152)
==436==    by 0x1E6B6ABD: av_freep (mem.c:159)
==436==    by 0x1E6A0251: ogg_read_page (oggdec.c:253)
==436==    by 0x1E6A0F54: ogg_get_length (oggdec.c:480)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
==436==
==436==
==436== ---- Attach to debugger ? --- [Return/N/n/Y/y/C/c] ---- ==436==
Invalid read of size 4
==436==    at 0x1E6A1C30: theora_gptopts (oggparsetheora.c:133)
==436==    by 0x1E69FB39: ogg_gptopts (oggdec.h:137)
==436==    by 0x1E6A10E0: ogg_get_length (oggdec.c:498)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
==436==  Address 0x2baefa84 is 4 bytes inside a block of size 12 free'd
==436==    at 0x7DE8ABE: free (vg_replace_malloc.c:913)
==436==    by 0x1E6B6A9A: av_free (mem.c:152)
==436==    by 0x1E6B6ABD: av_freep (mem.c:159)
==436==    by 0x1E6A0251: ogg_read_page (oggdec.c:253)
==436==    by 0x1E6A0F54: ogg_get_length (oggdec.c:480)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
==436==
==436== Invalid read of size 4
==436==    at 0x1E6A1C4E: theora_gptopts (oggparsetheora.c:135)
==436==    by 0x1E69FB39: ogg_gptopts (oggdec.h:137)
==436==    by 0x1E6A10E0: ogg_get_length (oggdec.c:498)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
==436==  Address 0x2baefa88 is 8 bytes inside a block of size 12 free'd
==436==    at 0x7DE8ABE: free (vg_replace_malloc.c:913)
==436==    by 0x1E6B6A9A: av_free (mem.c:152)
==436==    by 0x1E6B6ABD: av_freep (mem.c:159)
==436==    by 0x1E6A0251: ogg_read_page (oggdec.c:253)
==436==    by 0x1E6A0F54: ogg_get_length (oggdec.c:480)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
==436==
==436== Invalid read of size 1
==436==    at 0x1E6A1C15: theora_gptopts (oggparsetheora.c:132)
==436==    by 0x1E69FB39: ogg_gptopts (oggdec.h:137)
==436==    by 0x1E69FCD3: ogg_calc_pts (oggdec.c:552)
==436==    by 0x1E6A0DBF: ogg_read_packet (oggdec.c:578)
==436==  Address 0x2baefa80 is 0 bytes inside a block of size 12 free'd
==436==    at 0x7DE8ABE: free (vg_replace_malloc.c:913)
==436==    by 0x1E6B6A9A: av_free (mem.c:152)
==436==    by 0x1E6B6ABD: av_freep (mem.c:159)
==436==    by 0x1E6A0251: ogg_read_page (oggdec.c:253)
==436==    by 0x1E6A0F54: ogg_get_length (oggdec.c:480)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
==436==
==436== Invalid read of size 4
==436==    at 0x1E6A1C30: theora_gptopts (oggparsetheora.c:133)
==436==    by 0x1E69FB39: ogg_gptopts (oggdec.h:137)
==436==    by 0x1E69FCD3: ogg_calc_pts (oggdec.c:552)
==436==    by 0x1E6A0DBF: ogg_read_packet (oggdec.c:578)
==436==  Address 0x2baefa84 is 4 bytes inside a block of size 12 free'd
==436==    at 0x7DE8ABE: free (vg_replace_malloc.c:913)
==436==    by 0x1E6B6A9A: av_free (mem.c:152)
==436==    by 0x1E6B6ABD: av_freep (mem.c:159)
==436==    by 0x1E6A0251: ogg_read_page (oggdec.c:253)
==436==    by 0x1E6A0F54: ogg_get_length (oggdec.c:480)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
==436==
==436== Invalid read of size 4
==436==    at 0x1E6A1C4E: theora_gptopts (oggparsetheora.c:135)
==436==    by 0x1E69FB39: ogg_gptopts (oggdec.h:137)
==436==    by 0x1E69FCD3: ogg_calc_pts (oggdec.c:552)
==436==    by 0x1E6A0DBF: ogg_read_packet (oggdec.c:578)
==436==  Address 0x2baefa88 is 8 bytes inside a block of size 12 free'd
==436==    at 0x7DE8ABE: free (vg_replace_malloc.c:913)
==436==    by 0x1E6B6A9A: av_free (mem.c:152)
==436==    by 0x1E6B6ABD: av_freep (mem.c:159)
==436==    by 0x1E6A0251: ogg_read_page (oggdec.c:253)
==436==    by 0x1E6A0F54: ogg_get_length (oggdec.c:480)
==436==    by 0x1E6A1199: ogg_read_header (oggdec.c:523)
[436:557:72143085483:ERROR:ffmpeg_demuxer.cc(547)] IHF: max_duration=0
[436:436:72229627775:WARNING:resource_dispatcher.cc(461)] unknown request
[436:436:72229646826:WARNING:resource_dispatcher.cc(287)] Received message
for a nonexistent or finished request
[436:436:72229648743:WARNING:resource_dispatcher.cc(287)] Received message
for a nonexistent or finished request
[436:436:72229696045:WARNING:resource_dispatcher.cc(287)] Received message
for a nonexistent or finished request
[436:436:72229870268:WARNING:resource_dispatcher.cc(287)] Received message
for a nonexistent or finished request
[436:436:72229871559:WARNING:resource_dispatcher.cc(287)] Received message
for a nonexistent or finished request
==436== Invalid free() / delete / delete[]
==436==    at 0x7DE8ABE: free (vg_replace_malloc.c:913)
==436==    by 0x1E6B6A9A: av_free (mem.c:152)
==436==    by 0x1E69FD56: ogg_read_close (oggdec.c:606)
==436==    by 0x1E6ABBCC: av_close_input_stream (utils.c:2590)
==436==    by 0xB2ADCA0: media::FFmpegDemuxer::~FFmpegDemuxer()
(ffmpeg_demuxer.cc:301)
==436==    by 0xAD5B19C: base::RefCountedThreadSafe<media::Filter,
base::DefaultRefCountedThreadSafeTraits<media::Filter>
>::DeleteInternal(media::Filter const*) (ref_counted.h:149)
==436==    by 0xAD59244:
base::DefaultRefCountedThreadSafeTraits<media::Filter>::Destruct(media::Filter
const*) (ref_counted.h:114)
==436==    by 0xAD56D74: base::RefCountedThreadSafe<media::Filter,
base::DefaultRefCountedThreadSafeTraits<media::Filter> >::Release() const
(ref_counted.h:143)
==436==    by 0xAF69AEA: scoped_refptr<media::Filter>::~scoped_refptr()
(ref_counted.h:241)
==436==    by 0xB2AA7C3: void std::_Destroy<scoped_refptr<media::Filter>
>(scoped_refptr<media::Filter>*) (stl_construct.h:83)
==436==    by 0xB2AA697: void
std::_Destroy_aux<false>::__destroy<scoped_refptr<media::Filter>*>(scoped_refptr<media::Filter>*,
scoped_refptr<media::Filter>*) (stl_construct.h:93)
==436==    by 0xB2AA29C: void
std::_Destroy<scoped_refptr<media::Filter>*>(scoped_refptr<media::Filter>*,
scoped_refptr<media::Filter>*) (stl_construct.h:116)
==436==    by 0xB2A9B3B: void std::_Destroy<scoped_refptr<media::Filter>*,
scoped_refptr<media::Filter> >(scoped_refptr<media::Filter>*,
scoped_refptr<media::Filter>*, std::allocator<scoped_refptr<media::Filter>
>&) (stl_construct.h:142)
==436==    by 0xB2A9BA3: std::vector<scoped_refptr<media::Filter>,
std::allocator<scoped_refptr<media::Filter> >
>::_M_erase_at_end(scoped_refptr<media::Filter>*) (stl_vector.h:1150)
==436==    by 0xB2A957F: std::vector<scoped_refptr<media::Filter>,
std::allocator<scoped_refptr<media::Filter> > >::clear() (stl_vector.h:951)
==436==    by 0xB2A71FA: media::CompositeFilter::~CompositeFilter()
(composite_filter.cc:59)
==436==    by 0xAD5B19C: base::RefCountedThreadSafe<media::Filter,
base::DefaultRefCountedThreadSafeTraits<media::Filter>
>::DeleteInternal(media::Filter const*) (ref_counted.h:149)
==436==    by 0xAD59244:
base::DefaultRefCountedThreadSafeTraits<media::Filter>::Destruct(media::Filter
const*) (ref_counted.h:114)
==436==    by 0xAD56D74: base::RefCountedThreadSafe<media::Filter,
base::DefaultRefCountedThreadSafeTraits<media::Filter> >::Release() const
(ref_counted.h:143)
==436==    by 0xAF69C30:
scoped_refptr<media::Filter>::operator=(media::Filter*) (ref_counted.h:264)
==436==    by 0xB283F14: media::PipelineImpl::FinishDestroyingFiltersTask()
(pipeline_impl.cc:1060)
==436==    by 0xB283C08: media::PipelineImpl::TeardownStateTransitionTask()
(pipeline_impl.cc:1019)
==436==    by 0xB287E70: void DispatchToMethod<media::PipelineImpl, void
(media::PipelineImpl::*)()>(media::PipelineImpl*, void
(media::PipelineImpl::*)(), Tuple0 const&) (tuple.h:541)
==436==    by 0xB28761C: RunnableMethod<media::PipelineImpl, void
(media::PipelineImpl::*)(), Tuple0>::Run() (task.h:338)
==436==    by 0x8D1415F: (anonymous namespace)::TaskClosureAdapter::Run()
(message_loop.cc:102)
==436==    by 0x8D17AB5: base::internal::Invoker1<false,
base::internal::InvokerStorage1<void ((anonymous
namespace)::TaskClosureAdapter::*)(), (anonymous
namespace)::TaskClosureAdapter*>, void ((anonymous
namespace)::TaskClosureAdapter::*)()>::DoInvoke(base::internal::InvokerStorageBase*)
(bind_internal.h:547)
==436==    by 0x8830647: base::Callback<void ()()>::Run() const
(callback.h:261)
==436==    by 0x8D16941: MessageLoop::RunTask(MessageLoop::PendingTask
const&) (message_loop.cc:482)
==436==    by 0x8D16A6A:
MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&)
(message_loop.cc:500)
==436==    by 0x8D1726C: MessageLoop::DoWork() (message_loop.cc:691)
==436==  Address 0x2baefa80 is 0 bytes inside a block of size 13,176 free'd
==436==    at 0x7DE8ABE: free (vg_replace_malloc.c:913)
==436==    by 0x1DBF11AA: pixman_image_unref (pixman-image.c:217)
==436==    by 0x1D0552BB: _cairo_image_surface_finish
(cairo-image-surface.c:719)
==436==    by 0x1D06CF6D: cairo_surface_finish (cairo-surface.c:531)
==436==    by 0x1D06D007: cairo_surface_destroy (cairo-surface.c:436)
==436==    by 0x904D3AC:
skia::BitmapPlatformDevice::BitmapPlatformDeviceData::~BitmapPlatformDeviceData()
(bitmap_platform_device_linux.cc:58)
==436==    by 0x904DA44:
base::RefCounted<skia::BitmapPlatformDevice::BitmapPlatformDeviceData>::Release()
const (ref_counted.h:95)
==436==    by 0x904D9D0:
scoped_refptr<skia::BitmapPlatformDevice::BitmapPlatformDeviceData>::~scoped_refptr()
(ref_counted.h:241)
==436==    by 0x904D81E: skia::BitmapPlatformDevice::~BitmapPlatformDevice()
(bitmap_platform_device_linux.cc:137)
==436==    by 0x810C719: SkRefCnt::unref() const (SkRefCnt.h:62)
==436==    by 0x8F87D68: DeviceCM::~DeviceCM() (SkCanvas.cpp:96)
==436==    by 0x8F841F3: SkCanvas::internalRestore() (SkCanvas.cpp:799)
==436==    by 0x8F84092: SkCanvas::restore() (SkCanvas.cpp:765)
==436==    by 0x9E4EFD7: WebCore::GraphicsContext::endTransparencyLayer()
(GraphicsContextSkia.cpp:290)
==436==    by 0xA4C8D09:
WebCore::RenderBox::paintBoxDecorations(WebCore::PaintInfo&,
WebCore::IntPoint const&) (RenderBox.cpp:878)
==436==    by 0xA48AC75:
WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::IntPoint
const&) (RenderBlock.cpp:2486)
==436==    by 0xA489DC6: WebCore::RenderBlock::paint(WebCore::PaintInfo&,
WebCore::IntPoint const&) (RenderBlock.cpp:2281)
==436==    by 0xA48A905:
WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::IntPoint
const&) (RenderBlock.cpp:2439)
==436==    by 0xA48A63B:
WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::IntPoint
const&) (RenderBlock.cpp:2397)
==436==    by 0xA48ADAD:
WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::IntPoint
const&) (RenderBlock.cpp:2510)
==436==    by 0xA489DC6: WebCore::RenderBlock::paint(WebCore::PaintInfo&,
WebCore::IntPoint const&) (RenderBlock.cpp:2281)
==436==    by 0xA51A8BE:
WebCore::RenderLayerBacking::paintIntoLayer(WebCore::RenderLayer*,
WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int,
WebCore::GraphicsLayerPaintingPhase, WebCore::RenderObject*)
(RenderLayerBacking.cpp:1164)
==436==    by 0xA51AEAE:
WebCore::RenderLayerBacking::paintContents(WebCore::GraphicsLayer const*,
WebCore::GraphicsContext&, WebCore::GraphicsLayerPaintingPhase,
WebCore::IntRect const&) (RenderLayerBacking.cpp:1242)
==436==    by 0x9E9CBCE:
WebCore::GraphicsLayer::paintGraphicsLayerContents(WebCore::GraphicsContext&,
WebCore::IntRect const&) (GraphicsLayer.cpp:248)
==436==    by 0x9EA37FC:
WebCore::ContentLayerPainter::paint(WebCore::GraphicsContext&,
WebCore::IntRect const&) (ContentLayerChromium.cpp:71)
==436==    by 0x9E22282:
WebCore::LayerTextureUpdaterCanvas::paintContents(WebCore::GraphicsContext&,
WebCore::IntRect const&) (LayerTextureUpdaterCanvas.cpp:60)
==436==    by 0x9E2245C:
WebCore::LayerTextureUpdaterBitmap::prepareToUpdate(WebCore::IntRect const&,
WebCore::IntSize const&, int) (LayerTextureUpdaterCanvas.cpp:82)
==436==    by 0x9E249B5:
WebCore::LayerTilerChromium::prepareToUpdate(WebCore::IntRect const&)
(LayerTilerChromium.cpp:265)
==436==    by 0x9EA2A3D:
WebCore::ContentLayerChromium::paintContentsIfDirty(WebCore::IntRect const&)
(ContentLayerChromium.cpp:115)
==436==    by 0x9E17717:
WebCore::LayerRendererChromium::paintLayerContents(WTF::Vector<WTF::RefPtr<WebCore::CCLayerImpl>,
0u> const&) (LayerRendererChromium.cpp:430)
==436==


More information about the ffmpeg-user mailing list