NULL deref SEGV in malloc.c:unbin()

NULL deref SEGV in malloc.c:unbin()

[David Wuertele]

I wonder if anyone has hit this before?   In unbin(), c->next->prev is set, but
c->next is NULL.   It happens repeatedly, and here's what gdb says:

(gdb) b fopen
Breakpoint 9 at 0x90f78: file src/stdio/fopen.c, line 13.
(gdb) c
Continuing.

Breakpoint 9, fopen (filename=0xaabe4 "/etc/hosts", mode=0xaabf0 "r")
    at src/stdio/fopen.c:13
13	src/stdio/fopen.c: No such file or directory.
	in src/stdio/fopen.c
(gdb) b unbin
Breakpoint 10 at 0x8bc44: file src/malloc/malloc.c, line 239.
(gdb) c
Continuing.

Breakpoint 10, unbin (c=0x21408b8, i=40) at src/malloc/malloc.c:239
239	src/malloc/malloc.c: No such file or directory.
	in src/malloc/malloc.c
(gdb) print *c
$6 = {psize = 2096, csize = 2097, next = 0x2140088, prev = 0x0}
(gdb) s
241	in src/malloc/malloc.c
(gdb) 
Program received signal SIGSEGV, Segmentation fault.
0x0008bcc0 in unbin (c=0x21408b8, i=40) at src/malloc/malloc.c:241
241	in src/malloc/malloc.c
(gdb) 

The root cause was not obvious on scanning the source.
Is this perhaps something that's already been fixed?

Dave

Re: NULL deref SEGV in malloc.c:unbin()

[Rich Felker]

On Fri, Dec 27, 2013 at 06:35:00PM +0000, David Wuertele wrote:
> I wonder if anyone has hit this before?   In unbin(), c->next->prev is set, but
> c->next is NULL.   It happens repeatedly, and here's what gdb says:

It's almost surely a case of memory corruption by the calling program,
most likely using memory after it's already been freed.

Rich

Re: NULL deref SEGV in malloc.c:unbin()

[David Wuertele]

Rich Felker <dalias <at> aerifal.cx> writes:
> On Fri, Dec 27, 2013 at 06:35:00PM +0000, David Wuertele wrote:
> > I wonder if anyone has hit this before?   In unbin(), c->next->prev is set,
> > but c->next is NULL.   It happens repeatedly, and here's what gdb says:
> 
> It's almost surely a case of memory corruption by the calling program,
> most likely using memory after it's already been freed.

Hmm, my program calls malloc() once and never calls free().
Oh, I guess it does call free indirectly when it uses closedir() and fclose().
I will try to use gdb/watch to catch someone red-handed.

Dave

Re: Re: NULL deref SEGV in malloc.c:unbin()

[Rich Felker]

On Fri, Dec 27, 2013 at 07:44:23PM +0000, David Wuertele wrote:
> Rich Felker <dalias <at> aerifal.cx> writes:
> > On Fri, Dec 27, 2013 at 06:35:00PM +0000, David Wuertele wrote:
> > > I wonder if anyone has hit this before?   In unbin(), c->next->prev is set,
> > > but c->next is NULL.   It happens repeatedly, and here's what gdb says:
> > 
> > It's almost surely a case of memory corruption by the calling program,
> > most likely using memory after it's already been freed.
> 
> Hmm, my program calls malloc() once and never calls free().

And this crash happens on the very first call to malloc? Or did you
mean it only called it once successfully?

> Oh, I guess it does call free indirectly when it uses closedir() and fclose().
> I will try to use gdb/watch to catch someone red-handed.

It's also possible you write past the end of the buffer obtained by
malloc.

Rich

Re: NULL deref SEGV in malloc.c:unbin()

[David Wuertele]

Rich Felker <dalias <at> aerifal.cx> writes:

> 
> On Fri, Dec 27, 2013 at 07:44:23PM +0000, David Wuertele wrote:
> > Rich Felker <dalias <at> aerifal.cx> writes:
> > > On Fri, Dec 27, 2013 at 06:35:00PM +0000, David Wuertele wrote:
> > > > I wonder if anyone has hit this before?   In unbin(),
> > > > c->next->prev is set, but c->next is NULL.   It happens
> > > > repeatedly, and here's what gdb says: 
> > > > 
> > > 
> > > It's almost surely a case of memory corruption by the calling
> > > program, most likely using memory after it's already been
> > > freed. 
> > 
> > Hmm, my program calls malloc() once and never calls free().
> 
> And this crash happens on the very first call to malloc? Or did you
> mean it only called it once successfully?
> 

I only call malloc directly once, and it succeeds.  I use the
allocation only for a circular buffer.  I have an extremely high
confidence that the circular buffer does not write outside of its
memory allocation.

I see that malloc is called many times when I run opendir()/closedir()
and other musl library functions.  It is during one of the closedir()
that I see the SEGV.

The SEGV happens when mal.bins[40].head.next is dereferenced but
mal.bins[40].head.next is NULL.  So I am running my program under gdb
while watching mal.bins[40].head.  I see that nobody except for the
musl lib writes to mal.bins[40], but I have not seen any of those
writes result in a NULL value in *->next.

Now I am watching all memory locations that mal.bins[40].head points
to in its history, but the going is very slow.

Dave

Re: NULL deref SEGV in malloc.c:unbin()

[David Wuertele]

I wrote:

> Rich Felker <dalias <at> aerifal.cx> writes:
> > On Fri, Dec 27, 2013 at 07:44:23PM +0000, David Wuertele wrote:
> > > Rich Felker <dalias <at> aerifal.cx> writes:
> > > > On Fri, Dec 27, 2013 at 06:35:00PM +0000, David Wuertele wrote:
> > > > > I wonder if anyone has hit this before?   In unbin(),
> > > > > c->next->prev is set, but c->next is NULL.   It happens
> > > > > repeatedly, and here's what gdb says: 
> > > > > 
> > > > 
> > > > It's almost surely a case of memory corruption by the calling
> > > > program, most likely using memory after it's already been
> > > > freed. 
> > > 
> > > Hmm, my program calls malloc() once and never calls free().
> > 
> > And this crash happens on the very first call to malloc? Or did you
> > mean it only called it once successfully?
> > 
> 
> Now I am watching all memory locations that mal.bins[40].head points
> to in its history, but the going is very slow.

I removed all my calls to malloc(), now it is only musl that is
calling it.  I'm watching everything that I think might be related.
It looks like calloc(), opendir(), or free() are the culprit.

(gdb) info watch
Num     Type           Disp Enb Address    What
2       watchpoint     keep y              mal.bins[40].head
	breakpoint already hit 2 times
        print *mal.bins[40].head
        bt
        cont
3       watchpoint     keep y              mal.bins[40].head.next
	breakpoint already hit 47 times
        print *mal.bins[40].head
        bt
        cont
3.1                         y                mal.bins[40].head.next
3.2                         y                mal.bins[40].head.next
4       watchpoint     keep y              mal.bins[40].head.prev
	breakpoint already hit 32 times
        print *mal.bins[40].head
        bt
        cont
4.1                         y                mal.bins[40].head.prev
4.2                         y                mal.bins[40].head.prev
(gdb) c

Watchpoint 4: mal.bins[40].head.prev

Old value = (struct chunk *) 0xc9690
New value = (struct chunk *) 0x0
0x0008b268 in calloc (m=519, n=2080) at src/malloc/calloc.c:20
20	in src/malloc/calloc.c
$78 = {psize = 2097, csize = 2097, next = 0x0, prev = 0x0}
#0  0x0008b268 in calloc (m=519, n=2080) at src/malloc/calloc.c:20
#1  0x00089f30 in opendir (name=0xaa8d4 ".") at src/dirent/opendir.c:19
#2  0x0000fb50 in mkchdir (path=0xbe83f89b "") at prog.c:1845
Watchpoint 3: mal.bins[40].head.next

Old value = (struct chunk *) 0x0
New value = (struct chunk *) 0x10
opendir (name=0xaa8d4 ".") at src/dirent/opendir.c:24
24	src/dirent/opendir.c: No such file or directory.
	in src/dirent/opendir.c
$79 = {psize = 2097, csize = 2097, next = 0x10, prev = 0x0}
#0  opendir (name=0xaa8d4 ".") at src/dirent/opendir.c:24
#1  0x0000fb50 in mkchdir (path=0xbe83f89b "") at prog.c:1845
Watchpoint 3: mal.bins[40].head.next

Old value = (struct chunk *) 0x10
New value = (struct chunk *) 0x2139018
free (p=0x2139020) at src/malloc/malloc.c:530
530	src/malloc/malloc.c: No such file or directory.
	in src/malloc/malloc.c
$80 = {psize = 2096, csize = 2097, next = 0x2139018, prev = 0x0}
#0  free (p=0x2139020) at src/malloc/malloc.c:530
#1  0x00089e84 in closedir (dir=0x2139020) at src/dirent/closedir.c:9
#2  0x0000fc80 in mkchdir (path=0xd22bc "/var/spool") at prog.c:1873

Program received signal SIGSEGV, Segmentation fault.
0x0008be70 in unbin (c=0x2139848, i=40) at src/malloc/malloc.c:241
241	in src/malloc/malloc.c
(gdb) 


Dave

Re: Re: NULL deref SEGV in malloc.c:unbin()

[Rich Felker]

On Sat, Dec 28, 2013 at 01:28:42AM +0000, David Wuertele wrote:
> I removed all my calls to malloc(), now it is only musl that is
> calling it.  I'm watching everything that I think might be related.
> It looks like calloc(), opendir(), or free() are the culprit.

OK, so it seems like we should look for a possible bug in the dirent.h
functions. Are you aware of anything potentially unusual about the
directories you're reading? Would it be possible to provide an strace
log or reduce this to a minimal self-contained failing test case that
we could debug?

Rich

Re: Re: NULL deref SEGV in malloc.c:unbin()

[Szabolcs Nagy]

* David Wuertele <dave+gmane@wuertele.com> [2013-12-28 01:28:42 +0000]:
> I removed all my calls to malloc(), now it is only musl that is
> calling it.  I'm watching everything that I think might be related.
> It looks like calloc(), opendir(), or free() are the culprit.
> 
> (gdb) info watch
> Num     Type           Disp Enb Address    What
> 2       watchpoint     keep y              mal.bins[40].head
> 	breakpoint already hit 2 times
>         print *mal.bins[40].head
>         bt
>         cont
> 3       watchpoint     keep y              mal.bins[40].head.next
> 	breakpoint already hit 47 times
>         print *mal.bins[40].head
>         bt
>         cont
> 3.1                         y                mal.bins[40].head.next
> 3.2                         y                mal.bins[40].head.next
> 4       watchpoint     keep y              mal.bins[40].head.prev
> 	breakpoint already hit 32 times
>         print *mal.bins[40].head
>         bt
>         cont
> 4.1                         y                mal.bins[40].head.prev
> 4.2                         y                mal.bins[40].head.prev
> (gdb) c
> 
> Watchpoint 4: mal.bins[40].head.prev
> 
> Old value = (struct chunk *) 0xc9690
> New value = (struct chunk *) 0x0
> 0x0008b268 in calloc (m=519, n=2080) at src/malloc/calloc.c:20
> 20	in src/malloc/calloc.c
> $78 = {psize = 2097, csize = 2097, next = 0x0, prev = 0x0}
> #0  0x0008b268 in calloc (m=519, n=2080) at src/malloc/calloc.c:20
> #1  0x00089f30 in opendir (name=0xaa8d4 ".") at src/dirent/opendir.c:19
> #2  0x0000fb50 in mkchdir (path=0xbe83f89b "") at prog.c:1845

if i understand the code correctly the state is already corrupted here

the printed head chunk is returned from malloc (calloc zeros out next/prev)
but it is still in the free list (bin[40].head)

after the last free chunk is used up in a bin i think
head == tail == BIN_TO_CHUNK(i) should hold
(and mal.binmap & (1ULL<<i) should be 0)

> Watchpoint 3: mal.bins[40].head.next
> 
> Old value = (struct chunk *) 0x0
> New value = (struct chunk *) 0x10
> opendir (name=0xaa8d4 ".") at src/dirent/opendir.c:24
> 24	src/dirent/opendir.c: No such file or directory.
> 	in src/dirent/opendir.c
> $79 = {psize = 2097, csize = 2097, next = 0x10, prev = 0x0}
> #0  opendir (name=0xaa8d4 ".") at src/dirent/opendir.c:24
> #1  0x0000fb50 in mkchdir (path=0xbe83f89b "") at prog.c:1845

this is just opendir setting dir->fd (dir == &head->next)

> Watchpoint 3: mal.bins[40].head.next
> 
> Old value = (struct chunk *) 0x10
> New value = (struct chunk *) 0x2139018
> free (p=0x2139020) at src/malloc/malloc.c:530
> 530	src/malloc/malloc.c: No such file or directory.
> 	in src/malloc/malloc.c
> $80 = {psize = 2096, csize = 2097, next = 0x2139018, prev = 0x0}
> #0  free (p=0x2139020) at src/malloc/malloc.c:530
> #1  0x00089e84 in closedir (dir=0x2139020) at src/dirent/closedir.c:9
> #2  0x0000fc80 in mkchdir (path=0xd22bc "/var/spool") at prog.c:1873

a chunk is freed here (0x2139018) which we haven't seen yet
but is put on the same free list that is already broken

self->prev->next = self;

triggers the watchpoint (where &self->prev->next == &bin[40].head)

so the free-list has two chunks now: head is the broken chunk
returned in opendir and the new one freed here

> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x0008be70 in unbin (c=0x2139848, i=40) at src/malloc/malloc.c:241
> 241	in src/malloc/malloc.c
> (gdb) 
> 

i assume 0x2139848 is the broken chunk and unbin stumbles
on the 0 prev pointer

so it seems the corruption starts before opendir

it would be nice to see where 0x2139018 comes from and why
mal.binmap and mal.bin[40] aren't managed properly

Re: Re: NULL deref SEGV in malloc.c:unbin()

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2013-12-29 01:01:12 +0100]:
> 
> so it seems the corruption starts before opendir
> 
> it would be nice to see where 0x2139018 comes from and why
> mal.binmap and mal.bin[40] aren't managed properly

probably unrelated but i dont understand

split = (void *)((char *)self + n);

in pretrim and trim

why is the n enough between the start of self and split
chunks? (and not n + overhead)

Re: Re: NULL deref SEGV in malloc.c:unbin()

[Rich Felker]

On Sun, Dec 29, 2013 at 01:05:15AM +0100, Szabolcs Nagy wrote:
> * Szabolcs Nagy <nsz@port70.net> [2013-12-29 01:01:12 +0100]:
> > 
> > so it seems the corruption starts before opendir
> > 
> > it would be nice to see where 0x2139018 comes from and why
> > mal.binmap and mal.bin[40] aren't managed properly
> 
> probably unrelated but i dont understand
> 
> split = (void *)((char *)self + n);
> 
> in pretrim and trim
> 
> why is the n enough between the start of self and split
> chunks? (and not n + overhead)

The first line of malloc() calls adjust_size(&n). After that, n is
always in terms of total chunk size needed, not caller-usable size.

Rich

Re: NULL deref SEGV in malloc.c:unbin()

[David Wuertele]

I found the root cause of the SEGV, I was calling closedir() on the
same dir pointer twice (quite some time before the SEGV).

I assume that the behavior of closedir() is undefined when used this
way, so my program now makes sure not to do that.

But it seems a poor implementation that a double call to closedir
should result in memory corruption, and it seems a bug in malloc()
that a closedir/opendir sequence can cause it to SEGV.

I tried to reduce my program to just this behavior so that I could
give you a test case, but the SEGV did not occur with just the
opendir/closedir sequence my program calls.

Dave

Re: Re: NULL deref SEGV in malloc.c:unbin()

[Rich Felker]

On Mon, Dec 30, 2013 at 07:17:49PM +0000, David Wuertele wrote:
> I found the root cause of the SEGV, I was calling closedir() on the
> same dir pointer twice (quite some time before the SEGV).
> 
> I assume that the behavior of closedir() is undefined when used this
> way, so my program now makes sure not to do that.
> 
> But it seems a poor implementation that a double call to closedir
> should result in memory corruption, and it seems a bug in malloc()
> that a closedir/opendir sequence can cause it to SEGV.

No, there is fundamentally no way to make double-free errors safe;
once a resource has been freed, the (numerically) same identifier may
be used for new resources, and there is no way in general to
distinguish between valid access via the new resource and invalid
access to the already-freed old resource. In multi-threaded situations
especially, this makes double-free errors (of any sort, even plain
file descriptors via close()) one of the most dangerous programming
errors possible. These types of errors have been responsible for many
real-world remote privilege elevation vulnerabilities.

musl attempts to catch and intentionally crash on those double-free
errors that *can* be caught (a subset of all possible ones) so it's a
bit odd that you're experiencing free-list corruption rather than a
direct crash. I suspect you're actually using the already-closed DIR
handle in other ways before closing it again, perhaps calling readdir
on it, which would access and modify the contents of this memory
(which now belongs to the allocator) as if it were a DIR handle.

Rich