[patch] wordexp resource cleanup

[Peter Rosin]

On 2012-08-20 12:29, Peter Rosin wrote:
> On 2012-08-20 12:17, Peter Rosin wrote:
>> On 2012-08-20 00:00, Peter Rosin wrote:
>>> While looking up how wordexp worked I noticed that the resource handling
>>> in the implementation was "somewhat lacking". So here's a follow-up to
>>> take care of a few corner cases. (Not even compile-tested as I don't
>>> know how to set that up without reading up on the subject, but you
>>> wouldn't trust a newcomer anyway, right?)
>>>
>>> 	* libc/posix/wordexp.c (wordexp): Free all file descriptors
>>> 	and make sure the child is reaped in case of failure. Remove
>>> 	dead code while at it.
>>>
>>
>> Whoops! There were more bugs in that function, so here's an update. This
>> one has actually been tested and works AFAICT.
>>
>> 	* libc/posix/wordexp.c (wordexp): Allocate enough storage for
>> 	all returned words. Handle expanded words longer than 500 bytes.
>> 	Return WRDE_NOSPACE on resource allocation failure, and cleanup
>> 	leftover resources for that case.
> 
> Whoops, that's a misleading ChangeLog entry. The first part of it
> referred to fixing a bug that I had introduced myself when sanitizing
> the realloc calls. So, a better ChangeLog would be:
> 
> 	* libc/posix/wordexp.c (wordexp): Handle expanded words longer
> 	than 500 bytes.	Return WRDE_NOSPACE on resource allocation
> 	failure. Cleanup leftover resources when failing.

Whoops again, more bugs. The following program leaks like hell (with a
self-built Cygwin bash supporting --wordexp, pending an update from Eric):

---------------8<------------------------------
#include <wordexp.h>

int
main (void)
{
  wordexp_t we;

  we.we_offs = 3000;
  while (1) {
    wordexp ("/usr/bin/*", &we, WRDE_DOOFFS);
    wordfree (&we);
  }
  return 0;
}
---------------8<------------------------------

So, here's an update fixing that too. Reading opengroup docs on wordexp
(http://pubs.opengroup.org/onlinepubs/009695399/functions/wordexp.html)
suggests that it should be ok to rely on the offset count in the given
wordexp_t, and I find nothing on that page suggesting that it is not
ok to store the non-use of an offset in the we_offs member. So, that's
what I did.

Cheers,
Peter

	* libc/posix/wordexp.c (wordexp): Handle expanded words longer
	than 500 bytes.	Don't leak file streams. Help wordfree step past
	we_offs entries when freeing. Return WRDE_NOSPACE on resource
	allocation failure. Cleanup leftover resources when failing.
	* libc/posix/wordfree.c (wordfree): Step past we_offs words
	before starting to free.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: wordexp-3.patch
Type: text/x-patch
Size: 6380 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/newlib/attachments/20120822/e4e6ddb8/attachment.bin>