Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160914142842.GZ15995@brightrain.aerifal.cx>
Date: Wed, 14 Sep 2016 10:28:42 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: incompatibility between libtheora/mmx and musl ?

On Wed, Sep 14, 2016 at 04:04:50PM +0200, u-uy74@...ey.se wrote:
> On Wed, Sep 14, 2016 at 01:24:00PM +0200, Szabolcs Nagy wrote:
> > >  #define _ogg_malloc(x)  malloc((x)+256)
> > >  #define _ogg_calloc(x,y)  calloc((x)+256,(y))
> > >  #define _ogg_realloc(y,x) realloc((y),(x)+256)
> > >  #define _ogg_free    free
> > > 
> > > instead of the default
> > > 
> > >  #define _ogg_malloc  malloc
> > >  #define _ogg_calloc  calloc
> > >  #define _ogg_realloc realloc
> > >  #define _ogg_free    free
> > > 
> > > did not make any difference. The crash on a test file occurs in the same
> > > way and the resulting partial output file is as long as otherwise.
> > > 
> > > This may mean that this is not a simple overflowing but rather
> > > overwriting or reading distant "random" places (?) (register corruption?)
> 
> > can be underflow (or the way they align the pointer returned by malloc)
> 
> Pointer alignment yes they do in some cases but in a different layer,
> inside the malloc()-ed buffers, it is plain C and looks harmless to me.
> 
> > you can increase/decrease alignment of musl's alloc by
> > changing SIZE_ALIGN in src/malloc/malloc.c
> 
> Doubling the alignment did not apparently change the crashing.
> 
> Reducing the alignment in half did not apparently change the crashing.
> 
> (A single test file with a single quality setting tested
> crashed the same way, at the same place in the output stream)

I think this was a bad recommendation to test. musl's SIZE_ALIGN is a
macro to avoid magic numbers and make the code (very minimally)
self-documenting, but that doesn't mean it's a parameter. It's tied
into assumptions about the actual metadata structure and changing it
is almost surely going to introduce internal inconsistency/corruption
in the malloc implementation.

> > (or you can try some hack in _ogg_malloc/free if you are
> > sure that's what they are using)
> 
> Yes it is present/used for this very purpose, to enable easy "hijacking".
> 
> OTOH when I checked the arguments in gdb they looked always sane, up to
> the last and crashing realloc() call. That's why I do not expect seeing
> anything unusual there.
> 
> Valgrind did not see any bad free()s either.
> 
> > there can be some call abi issue (register clobbering,
> > stack alignment,..) because of the asm, but that's hard
> > to check.
> 
> Is musl in any way special compared to glibc/uclibc in its register usage?

Not in principle; this is mandated by the ABI. But it's possible that
their violation of ABI contracts is visible with some implementations
but not others. For example if they're calling malloc from code that's
using asm it's possible that they assume the floating point registers
(or mmx state) are call-saved rather than call-clobbered. This is an
invalid assumption that might happen to actively break on musl but not
glibc. IIRC you need some special instructions to switch between x87
and (original) mmx usage; perhaps they're missing this somewhere.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.