On Sat, Mar 11, 2017 at 3:48 AM, Rob Landley <r...@landley.net> wrote:
> Back in Austin, recovering from jetlag...
>
> On 03/09/2017 12:31 PM, enh wrote:
>> i could have sworn i mentioned this already, but i can't find any
>> proof that i did.
>>
>> i haven't seen this crash personally, but automated testing has seen
>> it a few times now:
>>
>>
>> pid: 25863, tid: 25863, name: ps >>> ps <<<
>> signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x11
>
> Huh, that's not good.
>
>> x0 0000000000000001 x1 000000557cc90468 x2 0000007fd2a2b1c8
>
> Guessing from register size/count this is arm64?
>
>> Stack Trace:
>> RELADDR FUNCTION FILE:LINE
>> 00000000000353bc get_threads+164
>
> So +164 would be about... No idea, but presumably not right at the start
> of the function? Which rules out new being funky...
>
>> external/toybox/toys/posix/ps.c:905 (discriminator 1)
>> 000000000000dabc dirtree_handle_callback+36
>> external/toybox/lib/dirtree.c:112
>> 000000000000dbc8 dirtree_recurse+128
>> external/toybox/lib/dirtree.c:156
>> 000000000000db14 dirtree_handle_callback+124
>> external/toybox/lib/dirtree.c:115
>> 00000000000347a0 ps_main+952
>> external/toybox/toys/posix/ps.c:1235
>> 0000000000013814 toy_exec+92
>> external/toybox/main.c:166 (discriminator 1)
>> 00000000000133d8 toybox_main+48
>> external/toybox/main.c:179 (discriminator 1)
>> 00000000000138dc main+124 external/toybox/main.c:237
>>
>> given the 0x11, i'm assuming this is actually 0x10 (aka 16 aka
>> sizeof(void*)/2 aka struct dirtree::child) off DIRTREE_ABORTVAL.
>
> Ok, I'm wrong, sounds like arm64 takes 164 bytes to clear its throat,
> because that sounds like:
>
> static int get_threads(struct dirtree *new)
> {
> struct dirtree *dt;
> struct carveup *tb;
> unsigned pid, kcount;
>
> if (!new->parent) return get_ps(new);
> pid = atol(new->name);
>
> should be faulting on the very first line (where we have a read
> dereferencing a null-ish pointer).
from the line number addr2line gave above, it's the line marked with !
below. i've included the block between too, since that modifies
new->child.
// Recurse down into tasks, retaining thread groups.
// Disable show_process at least until we can calculate tcount
kcount = TT.kcount;
sprintf(toybuf, "/proc/%u/task", pid);
new->child = dirtree_flagread(toybuf, DIRTREE_SHUTUP|DIRTREE_PROC, get_ps);
TT.threadparent = 0;
kcount = TT.kcount-kcount+1;
tb = (void *)new->extra;
tb->slot[SLOT_tcount] = kcount;
// Fill out tid and thread count for each entry in group
! if (new->child) for (dt = new->child->child; dt; dt = dt->next) {
so i assume it's that dirtree_flagread that's failing, returning
DIRTREE_ABORTVAL, and since the code that follows just assumes it's
either valid or null, we die on the line marked ! (as claimed by the
stack trace).
i don't know is why why dirtree_flagread is failing, though, and can't
reproduce it.
>> but i still don't see how we end up with DIRTREE_ABORTVAL here, so i'm
>> not sure what the right fix is.
>
> ABORTVAL is what you return when you want the recursion to stop, it
> means you found what you're looking for. Why it would be passed _into_
> the callback is a question: that shouldn't happen.
>
>> any ideas? (i'm assuming it's fallout from DIRTREE_SHUTUP, but haven't
>> worked out how so yet.)
>
> It sounds like /proc isn't mounted? Comment in lib/ditree.c right above
> dirtree_flagread() says:
>
> // Returns DIRTREE_ABORTVAL if path didn't exist (use DIRTREE_SHUTUP
> // to handle error message yourself).
>
> Which is because handle_callback() returns DIRTREE_ABORTVAL if it's
> passed a null pointer for new. And our various calls to
> dirtree_flagread() in ps.c seem to assume NULL as the error return,
> which isn't right.
>
> Except... the only use of get_threads() in ps is line 1237 where we feed
> it as an argument to dirtree_flagread() and that _should_ be getting
> this right. The callback shouldn't get called back with ABORTVAL in the
> first place? It's filtered out in dirtree.c line 157, which is the only
> caller of handle_callback in the file other than dirtree_flagread()...
>
> I need to look at this when I'm more awake. And yeah, that got fiddled
> with in DIRTREE_SHUTUP. Hmmm, how would I reproduce this... I just tried
> a chroot with /proc not mounted (and again with no /proc directory) and
> ps just gave me the header line with no contents, no segfault...
>
> Rob
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
