Bug#591525: [PATCH] segfault in playtree.c

Adrian Knoth adi at drcomp.erfurt.thur.de
Thu Aug 5 22:06:13 UTC 2010


On Thu, Aug 05, 2010 at 11:46:48PM +0200, Reimar Döffinger wrote:

> > This is Debian Bug: http://bugs.debian.org/591525
> > 
> > Index: playtree.c
> > ===================================================================
> > --- playtree.c	(revision 31912)
> > +++ playtree.c	(working copy)
> > @@ -223,6 +223,13 @@
> >    assert(pt->entry_type == PLAY_TREE_ENTRY_NODE);
> >  #endif
> >  
> > +  /* Roughly validate input data. Both, pt and child are going to be
> > +   * dereferenced, hence assure they're not NULL.
> > +   */
> > +  if (NULL == pt || NULL == child) {
> > +      return;
> > +  }
> > +
> 
> Checking for NULL after dereferencing makes no sense, even if the
> dereferencing is inside an assert.

Good catch, this part needs to be moved before the #ifdef and the
assert removed. If you like, find attached a revised version.

> Apart from that, pt and child are not _input_ data, they are
> MPlayer-internal data, it is very likely the validation
> should happen far earlier.

Depends. There are two calls to play_tree_set_child in playtreeparser.c.
Of course, you could check for non-NULL pointers before calling them.
Twice the code.

Either one should work.


HTH

-- 
mail: adi at thur.de  	http://adi.thur.de	PGP/GPG: key via keyserver
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 25playlist.patch
Type: text/x-diff
Size: 955 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20100806/6086ebb6/attachment-0001.patch>


More information about the pkg-multimedia-maintainers mailing list