Looks like this happens when /proc/$PID/cmdline is empty, which is true for "kthreadd" on android. Numbered strings below are ptb->str + ptb->offset[i]. For some reason ptb->slot[SLOT_argv0len] == 8 when cmdline is empty, but I don't see where it's coming from. And ptb->str + ptb->offset[4] + ptb->slot[SLOT_argv0len] overlaps with buf; and there are no "/" characters in cmdline to limit the memcpy range. Looks like a bug?
0: 1: SyS_epoll_wait 2: 3: 4: /init --second-stage buf: 0x55928d7961 ptb->str = 0x55928d7935, ptb->offset[4] = 23, s = 0x55928d794c s: /init --second-stage, i: 5, len: 3051 skip 1 buf 0x55928d7961, s 0x55928d794d, len 4 USER PID PPID VSZ RSS WCHAN PC S NAME root 1 0 10628 1316 SyS_epoll_ 4d805c S init 0: 1: kthreadd 2: 3: 4: buf: 0x55928d794b ptb->str = 0x55928d7935, ptb->offset[4] = 21, s = 0x55928d794a s: , i: 8, len: 3073 buf 0x55928d794b, s 0x55928d794a, len 8 ================================================================= ==23291==ERROR: AddressSanitizer: memcpy-param-overlap: memory ranges [0x0055928d794b,0x0055928d7953) and [0x0055928d794a, 0x0055928d7952) overlap On Fri, Oct 7, 2016 at 12:00 PM, Rob Landley <r...@landley.net> wrote: > On 10/06/2016 04:15 PM, Evgenii Stepanov wrote: >> Hi Rob, >> >> thanks for the explanation. This is definitely not a false positive - >> the report even contains dst and src ranges for the memcpy() call, and >> they indeed overlap. Should be possible to reproduce w/o ASan by >> checking the addresses in the code. > > Right before the memcpy/memmove I added: > > printf("buf=%p s=%p len=%ld\n", buf, s, (long)len); > if (s+len >= buf) printf("BANANA!\n"); > > And did > > ./ps -aO NAME > > And I don't see an instance of overlap? > > (I shouldn't have to test the other way because buf should always be > len?) > >> I've added some debug printfs to this code, and got this: > > I'm guessing on line 753? > >> j = 5 >> ptb 0x0 tb 0x5564dfb750 buf 0x5564dfb961 > > This doesn't have s and buf, it has tb and buf. Still, it lets us know > you're looking at the non-thread case during the PS_NAME copy. Line 758 > reassigns ptb so you'd have to have your printf before that. > > Then we do a slightly funky basename(): > > i = ptb->slot[SLOT_argv0len]; > s = ptb->str+ptb->offset[4]; > while (-1!=(k = stridx(s, '/')) && k<i) { > s += k+1; > i -= k+1; > } > > Given that offset[4] should be a properly null terminated string, that > while() loop shouldn't advance past the end of it, and _should_ decrease > i by the same amount it increased s. Since "buf" should point to the > next byte after that string... how do they wind up overlapping? > > (Is ptb->slot[SLOT_argv0len] winding up bigger than > strlen(ptb->str+ptb->offset[4]) somehow? I'm not seeing it in my tests > here, is it a 32-bit thing? Bionic? Android kernel?) > > >> USER PID PPID VSZ RSS WCHAN PC S NAME >> root 1 0 10628 1272 SyS_epoll_ 4d805c S init >> j = 5 >> ptb 0x0 tb 0x5564dfb750 buf 0x5564dfb94b > > Basically the same data as above. We're not copying from buf to tb, > we're copying len bytes from s to buf. This doesn't tell me what the bad > memcpy's inputs are. > >> The change (the one that requires a google login) simply replaces >> memcpy with memmove, which sounds like a bad idea at this point. > > Well... maybe? I still don't know what's going on. > > Rob _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net