[Toybox] Sigh. Anybody spot the bug?

[Andre Renaud]

On 4 July 2013 07:00, Avery Pennarun <apenwarr at gmail.com> wrote:
> On Wed, Jul 3, 2013 at 2:52 AM, Rob Landley <rob at landley.net> wrote:
>> Tail has a double free somewhere. (Aboriginal's more/buildall.sh is
>> complaining, that uses toybox in host-tools.) Haven't had time to track it
>> down yet, wondering if anybody else could spot it.
>>
>> From the behavior it's looking like it's on file close...
>
> For what it's worth, running programs under valgrind tends to catch
> this sort of problem instantly.

Valgrind complains about using uninitialised values in llist_traverse,
called from try_lseek:
==15446== Conditional jump or move depends on uninitialised value(s)
==15446==    at 0x40AF4F: llist_traverse (llist.c:18)
==15446==    by 0x41BF75: try_lseek (tail.c:126)
==15446==    by 0x41C029: do_tail (tail.c:150)
==15446==    by 0x409C26: loopfiles_rw (lib.c:850)
==15446==    by 0x409C7F: loopfiles (lib.c:858)
==15446==    by 0x41C260: tail_main (tail.c:221)
==15446==    by 0x404945: toy_exec (main.c:104)
==15446==    by 0x404A2F: toybox_main (main.c:126)
==15446==    by 0x404945: toy_exec (main.c:104)
==15446==    by 0x404A2F: toybox_main (main.c:126)
==15446==    by 0x404BAA: main (main.c:163)

Having a look at the code, it would appear that tail.c/get_chunk
doesn't set line->next to anything meaningful, so if you're still
working on the first chunk, then it won't be set at all. Changing it
as follows seems to remove the warning.

diff -r 6a37f642b572 toys/posix/tail.c
--- a/toys/posix/tail.c Sat Jun 08 14:11:41 2013 -0500
+++ b/toys/posix/tail.c Thu Jul 04 08:37:31 2013 +1200
@@ -49,6 +49,7 @@

   line->data = ((char *)line) + sizeof(struct line_list);
   line->len = readall(fd, line->data, len);
+  line->next = NULL;

   if (line->len < 1) {
     free(line);

Not sure if this is what was causing Rob's crash though.

Regards,
Andre

 1372884021.0