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 <r...@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
_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
