RE: mdemo ltdl failure

2007-03-16 Thread Dave Korn
On 16 March 2007 15:35, Charles Wilson wrote:

 Well, it's failing all the time for me, but I'm not sure it's a 
 segfault. What does Hangup mean, when reported by the shell after 
 executing the app: Good question, I don't know.

  It means SIGHUP.

 I've (almost) tracked down the error: it is caused by yet another bug in
 newlib's argz_insert() (or possibly realloc()! ), as called by

  This thread should probably be on the newlib list then.

 What's odd is that this bug in argz_insert() is very ticklish: it
 triggers on tests/mdemo/Makefile, but not when argz_insert is called
 with ./tests/mdemo/Makefile.

  Isn't that just exactly what you would expect, given that you're talking
about sorting things in ascii order?  The period collates very early in ascii
sort order, whereas a lower-case t comes much later; hence if you specify the
'.' you get the makefile at the start of the list instead of the end.

 I need to verify this using a debug-built cygwin kernel, but it looks
 like within newlib's argz_insert(), the call to realloc() is not
 operating correctly in this instance.

  Sounds like it should be quite easy to PPAST then.

Can't think of a witty .sigline today

Re: mdemo ltdl failure

2007-03-16 Thread Charles Wilson
Well, once I got the cygwin1.dbg stuff worked out, it was pretty easy to 
track down: it is a bug in newlib's argz_insert:

Charles Wilson wrote:

Here's the code from newlib's argz_insert:

_DEFUN (argz_insert, (argz, argz_len, before, entry),
   char **argz _AND
   size_t *argz_len _AND
   char *before _AND
   const char *entry)
  int len = 0;

  if (before == NULL)
return argz_add(argz, argz_len, entry);

  if (before  *argz || before = *argz + *argz_len)
return EINVAL;

Note that before is always either NULL or points to some location within 
the existing *argz buffer.

  while (before != *argz  before[-1])

Because *argz contains NULL-delimited strings one after the other, if 
the user calls this function with a before that points into the middle 
of one of those strings, the preceeding two lines just back up to the 
beginning of that string (or to the beginning of the current argz, 
whichever comes first).

  len = strlen(entry) + 1;

In the failing call, we actually do a realloc...

  if(!(*argz = (char *)realloc(*argz, *argz_len + len)))
return ENOMEM;

But if we realloc the *argz buffer, then a non-NULL 'before' pointer 
will be pointing into the old, freed, *argz buffer.  So the following is 
clearly wrong, because we are copying stuff _from_ the new *argz to a 
modified location in the old (shorter) *argz -- which will overrun the 
end of the old buffer by exactly strlen(entry), and clobber stuff. 
Depending on the actual allocated locations of malloced data, this could 
include (1) like some of the the memory held by entry, or (2) some of 
the memory held by the new *argz buffer.  In this case, it is (1).

  memmove(before + len, before, *argz + *argz_len - before);

Then, we copy this clobbered entry data into the front of (the freed, 
old *argz buffer)

  memcpy(before, entry, len);

  *argz_len += len;

meanwhile, the actual (newly realloc'ed) *argz buffer just contains 
whatever was in *argz prior to the call to this function.  Worse, with 
upward malloc movement, the too-large memove above might also have 
clobbered the first several bytes of the new *argz buffer, as well.

And that's what's happening in this case.

The eventual FREE(buf) error is because the first few bytes in the 
malloc-managed memory for buf (e.g. just below *buf) which contain 
malloc bookkeeping info, are also clobbered.

  return 0;

I'll whip up a patch and post it to the newlib list.