[Toybox] Unfixed bug in tail toy

[Rob Landley]

On 11/19/14 13:40, luckboy at vp.pl wrote:
> I see that bug in tail toy still is unfixed. Can you apply my fix of
> this bug?
> 
> Patch:
> https://github.com/luckboy/toyroot/blob/master/patch/toybox-0.5.0-tail-n-c.patch

A) I'm working on it as we speak, the problem is it's not the right fix.

B) That's the original patch addressing multiple issues. I applied part
of it a while back, every time I look at that I have to work out which
parts are the remaining ones at issue.

> 
> This bug is is segmentation fault for 'tail -c 10' that read big data
> from stdin.
> You can check occurrence of this bug by invoke commands:
> 
> dd if=/dev/urandom of=test.bin bs=1024 count=8
> hexdump test.bin | ./toybox tail -c 10

I asked for a reliable way to reproduce the issue and you told me to
read from /dev/urandom. When I substituted /dev/zero I didn't see it.
With the bytes I got from /dev/urandom your fix (applying the whole
patch to commit 1535 instead of just the previous half) changes the
segfault into:

$ hexdump test.bin | ./tail -c 10
tail: xwrite: Bad address

Sorry this took so long. I sat down to try to understand the "original
length" logic a half-dozen times, and couldn't, before finally
acknowledging the problem wasn't me. (I also had "./tail -c +10"
confused with "head -c 10", and thought this codepath was handling both
cases, which it clearly wasn't.)

Anyway, now that I _have_ finally wrapped my head back around this code,
the fix is apparently just:

--- a/toys/posix/tail.c	Tue Nov 18 04:25:27 2014 -0600
+++ b/toys/posix/tail.c	Wed Nov 19 14:18:22 2014 -0600
@@ -164,6 +164,7 @@
           }
           list->data += bytes;
           list->len -= bytes;
+          bytes = 0;
         }
       } else {
         int len = new->len, count;

I.E. once we've read through the initial negative TT.bytes backlog, we
discard the extra data, meaning we adjust the remaining amount each time
so the overflow is zero bytes. We were doing the adjustment right, but
not zeroing out the overflow counter after we did so.

Does that look right to you?

Rob

P.S. I did release notes for the existing commits yesterday, but my
remaining todo list includes items like this one and the sha1sum
bigendian thing that really need to be fixed before release, so I'm
working on them now.

 1416428976.0