Re: bsdtar core dumps

On 2004-08-19 14:37, Sean McNeil <sean_at_mcneil.com> wrote:
> Here is a backtrace of the error:
>
> #0  0x0000000200926d7e in __vfprintf (fp=0x7fffffffe360,
>     fmt0=0x4161d9 "Failed to open '%s'", ap=0x7fffffffe640)
>     at /usr/src/lib/libc/stdio/vfprintf.c:1052
> #1  0x00000002008c4006 in vsnprintf (str=0x32 <Address 0x32 out of bounds>,
>     n=4284889, fmt=0x4161d9 "Failed to open '%s'", ap=0x7fffffffe640)
>     at /usr/src/lib/libc/stdio/vsnprintf.c:75
> #2  0x0000000000411478 in __archive_string_vsprintf (as=0x520240,
>     fmt=0x4161d9 "Failed to open '%s'", ap=0x7fffffffe640)
>     at /usr/src/lib/libarchive/archive_string_sprintf.c:60
> #3  0x00000000004112f5 in archive_set_error (a=0x520000, error_number=2,
>     fmt=0x0) at /usr/src/lib/libarchive/archive_util.c:133
> #4  0x00000000004080cd in file_open (a=0x520000, client_data=0x4161d9)
>     at /usr/src/lib/libarchive/archive_read_open_file.c:90
> #5  0x0000000000411639 in archive_read_open (a=0x520000, client_data=0x51e0c0,
>     opener=0x408060 <file_open>, reader=0x408130 <file_read>,
>     closer=0x408160 <file_close>) at /usr/src/lib/libarchive/archive_read.c:124
> #6  0x0000000000408039 in archive_read_open_file (a=0x520000,
>     filename=0x7fffffffec02 "nonexistent.tar.gz", block_size=10240)
>     at /usr/src/lib/libarchive/archive_read_open_file.c:75
> #7  0x0000000000403a90 in read_archive (bsdtar=0x7fffffffe8b0, mode=120 'x')
>     at /usr/src/usr.bin/tar/read.c:86
> #8  0x000000000040398e in tar_mode_x (bsdtar=0x32)
>     at /usr/src/usr.bin/tar/read.c:62
> #9  0x0000000000402e1a in main (argc=7, argv=0xffffffff)
>     at /usr/src/usr.bin/tar/bsdtar.c:525
>
> Looks like something wrote past the end of a buffer as the str argument
> to vsnprintf is not correct.
>
> (gdb) p *as
> $8 = {s = 0x51e100 "", length = 0, buffer_length = 64}
>
> should be
>
> str=0x51e100, n=0
>
> and on the callback trace it is
>
> str=0x32 <Address 0x32 out of bounds>, n=4284889
>
> Could be a compiler bug I suppose, but more likely I think it is this
> code:
>
> 	if (n == 0) {
> 		if (on > 0)
> 	  		*str = '\0';
> 		str = dummy;
> 		n = 1;
> 	}

I might be wrong here, but looking at the code here's what I managed to
understood about the way it works:

Originally the value passed as `n' is saved in `on' (original n).
Then, only if n != 0, n is decremented.

In the special case that (n == 1) the vsnprintf() function can only write up
to (n - 1) == 0 bytes to the output buffer, according to its manpage.

In this case the check at line 63 (n == 0) will be true after the initial
magic in lines 57-61:

     57     on = n;
     58     if (n != 0)
     59         n--;
     60     if (n > INT_MAX)
     61         n = INT_MAX;
     62     /* Stdio internals do not deal correctly with zero length buffer */
     63     if (n == 0) {
     64         if (on > 0)
     65             *str = '\0';
     66         str = dummy;
     67         n = 1;
     68     }

The code in lines 63-68 is an attempt to pass a buffer with space for at least
1 character to __vfprintf(), in order to let __vfprintf() count the bytes of
the buffer that would be used if the size was infinite (as per the manpage).

> It seems very inappropriate to be messing with an input parameter like
> that and then using an uninitialized string in it's place.

Yeah, the dummy[] buffer is uninitialized, but that shouldn't be a problem
since it's passed to __vfprintf() with a buffer size of 1 byte.

Summing up, there is probably something wrong with the code but it's not the
fact that dummy[] is uninitialized.  It's more likely that something else is
going on.  I'm not sure what, though...