[Toybox] Release prep.

[Andreas Gampe]

Argh. I should have given you the full failure message (or be more
explicit in the suggested solution) - sorry, my mistake. ASAN
complains about a heap buffer overflow, not a null pointer access
(which should always kill top, not just under ASAN). Your fix was:

  struct carveup *otb = old.tb ? *old.tb : 0, *ntb = new.tb ? *new.tb : 0;

That only works if the tb value started out as null (as the loop
itself only increments the pointers). That doesn't seem to be the
failure case that ASAN is complaining about.


I think this would be a better solution:

  struct carveup *otb = old.count ? *old.tb : 0, *ntb = new.count ? *new.tb : 0;


Best,
 Andreas


On Thu, Feb 8, 2018 at 7:06 AM, Rob Landley <rob at landley.net> wrote:
> On 02/07/2018 10:41 AM, Andreas Gampe wrote:
>> Stack Trace:
>>   RELADDR           FUNCTION         FILE:LINE
>>      7e253  top_common+6387  external/toybox/toys/posix/ps.c:1420
>>      7c413  top_main+555     external/toybox/toys/posix/ps.c:1666
>>      1f7db  toy_exec+311     external/toybox/main.c:169
>>      1ef77  toybox_main+91   external/toybox/main.c:182
>>      1fa27  main+431         external/toybox/main.c:240
>>      ac8a3  __libc_init+91   bionic/libc/bionic/libc_init_dynamic.cpp:129
>>
>> ps.c:1419f
>>
>>     while (old.count || new.count) {
>>       struct carveup *otb = *old.tb, *ntb = *new.tb;
>>
>> You probably should not proactively dereference both old.tb and
>> new.tb. One might be invalid if not both old.count and new.count > 0.
>
> Oops, yes.
>
> Quick and dirty patch pushed, I need to properly re-read this chunk when
> I get more time and verify the logic.
>
> Rob