After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 619502 - [mpegvideoparse] segfault because of access to a NULL buffer gotten from mpeg_packetizer_get_block
[mpegvideoparse] segfault because of access to a NULL buffer gotten from mpeg...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal blocker
: 0.10.19
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-05-24 11:19 UTC by Zaheer Abbas Merali
Modified: 2010-05-24 23:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fixes segfault (1.28 KB, patch)
2010-05-24 11:19 UTC, Zaheer Abbas Merali
none Details | Review

Description Zaheer Abbas Merali 2010-05-24 11:19:26 UTC
Created attachment 161851 [details] [review]
fixes segfault

mpeg_packetizer_get_block in some circumstances returns a block but does not set the buffer causing mpegvideoparse_drain_avail to cause invalid memory access.
Comment 1 Zaheer Abbas Merali 2010-05-24 15:52:39 UTC
This file http://dl.dropbox.com/u/219349/gazach4_first100mb.ts triggers the segfault.
Comment 2 Tim-Philipp Müller 2010-05-24 19:56:47 UTC
Test pipeline:

gst-launch-0.10 -v filesrc location=gazach4_first100mb.ts ! decodebin2 ! queue ! audioconvert ! vorbisenc ! fakesink silent=true

ie. the crash only happens if mpegvideoparse isn't linked.
Comment 3 Tim-Philipp Müller 2010-05-24 20:05:20 UTC
Something still doesn't feel entirely right to me here. I would expect the parse/packetizer logic to work exactly the same, whether downstream is linked or not.

I'm also not entirely convinced the additional if (buf != NULL) check, which is obviously needed with the current semantics of the _get_block() function, is in the right place here. If we put it into the while() loop, then the packetizer will never advance to the next block from the looks of it.

Maybe it should instead be:


-    if (cur->flags & MPEG_BLOCK_FLAG_SEQUENCE) {
+   if ((cur->flags & MPEG_BLOCK_FLAG_SEQUENCE) && buf != NULL) {

just below the while()?
Comment 4 Tim-Philipp Müller 2010-05-24 23:31:14 UTC
commit 721643431e403a5360a975c8e06255d5f2396f55
Author: Zaheer Abbas Merali <zaheerabbas@merali.org>
Date:   Tue May 25 00:27:17 2010 +0100

    mpegvideoparse: fix crash if downstream is unliked
    
    mpeg_packetizer_get_block() in some circumstances (here: if
    downstream was unlinked) returns a block but does not set the
    buffer causing mpegvideoparse_drain_avail() to cause invalid memory
    access.
    
    Fixes #619502.