Date: Sun, 22 Aug 2004 18:48:13 -0700 From: Sean McNeil <sean@mcneil.com> To: Giorgos Keramidas <keramida@linux.gr> Cc: freebsd-current@freebsd.org Subject: Re: bsdtar core dumps Message-ID: <1093225693.43845.7.camel@server.mcneil.com> In-Reply-To: <20040822233414.GA79931@gothmog.gr> References: <1092777586.92327.9.camel@server.mcneil.com> <20040817213813.GE3827@gothmog.gr><4127841D.6050104@freebsd.org> <1093213755.72863.0.camel@server.mcneil.com> <20040822233414.GA79931@gothmog.gr>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 2004-08-22 at 16:34, Giorgos Keramidas wrote:
> On 2004-08-22 15:29, Sean McNeil <sean@mcneil.com> wrote:
> > On Sat, 2004-08-21 at 10:19, Tim Kientzle wrote:
> > > The code you've pointed to above concerns me because of the part about:
> > > if (n == 0) {
> > > ...
> > > n = 1;
> > > }
> > >
> > > That ain't right: If I told vsnprintf the buffer size was zero, it
> > > should treat it as such. If I meant "one", I would have said "one."
> > >
> > > On the other hand, the vsnprintf.3 man page does explicitly state
> > > that "the output is always null-terminated," which would preclude
> > > passing a zero-length buffer, which is exactly what libarchive is
> > > doing in this situation. It is bogus, but at least it's documented
> > > bogosity. ;-)
>
> The vsnprintf() function cannot pass a zero-length buffer to __vfprintf()
> because the __vfprintf() function is expected return the number of bytes it
> would need to do the real printing. It's not illegal to pass a zero-length
> bugger to vsnprintf(); at least it's not specifically prohibited by the
> manpage. The following program *DOES* pass zero as the length of the
> buffer to vsnprintf() and a NULL pointer as the buffer address but doesn't
> fault on an i386 machine:
>
> 1 #include <stdarg.h>
> 2 #include <stdio.h>
> 3
> 4 size_t koko(const char *_fmt, ...);
> 5
> 6 int
> 7 main(void)
> 8 {
> 9 size_t foo;
> 10
> 11 foo = koko("%ld", 5);
> 12 printf("rc = %lu\n", (unsigned long)foo);
> 13 return 0;
> 14 }
> 15
> 16 size_t
> 17 koko(const char *fmt, ...)
> 18 {
> 19 size_t rc;
> 20 va_list ap;
> 21
> 22 va_start(ap, fmt);
> 23 rc = vsnprintf(NULL, 0, fmt, ap);
> 24 va_end(ap);
> 25 return rc;
> 26 }
Yes, I tried this myself and it works just fine. I think I've found
part of the problem, though:
Breakpoint 1, __archive_string_vsprintf (as=0x520240, fmt=0x4161d9
"Failed to open '%s'", ap=0x7fffffffe5a0) at
/usr/src/lib/libarchive/archive_string_sprintf.c:56
(gdb) print *ap
$24 = {gp_offset = 24, fp_offset = 48, overflow_arg_area =
0x7fffffffe6a0, reg_save_area = 0x7fffffffe5c0}
(gdb) n
(gdb) print *ap
$25 = {gp_offset = 32, fp_offset = 48, overflow_arg_area =
0x7fffffffe6a0, reg_save_area = 0x7fffffffe5c0}
After first call to vsnprintf the va_list has been messed up. If I
manually set it back to 24 then I get the appropriate message and no
core dump. So vsnprintf caused ap to be overwritten/incremented from
the args (i.e. the %s) in the format string. I'd expect it would
increment farther depending on the number of args in the format.
Sean
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1093225693.43845.7.camel>