Replace vfork() with fork()

[eugenis]

ASan has stack false positives with vfork(). After vfork(), child process runs in the context and on the stack of parent process and poisons it. After that, execve() returns control to the parent (kind of like longjmp()) with leftover poisoning in newly-unallocated stack space.

See https://android-review.googlesource.com/#/c/platform/libcore/+/641439/ for an example.

AFAIK, it is always safe to replace vfork() with fork(). Do that in an interceptor.

[eugenis]

Fixed by https://reviews.llvm.org/D44587, r327752.

[eugenis]

I've reverted the fix for performance concerns. Apparently, fork() is not nearly as fast as vfork().

Now, the next plan is to intercept vfork() and unpoison the unallocated part of the stack in the parent process after the child is done. But there is a catch - vfork() can not be intercepted. Not in C, in any case. That is because the spec says the child is not allowed to return from a function that called vfork(), and interceptor itself would be such a function. Returning from the caller of vfork messes up parent's stack - effectively, the caller returns twice from its point of view.

We might be able to implement an interceptor in assembly, either by
(a) copying part of the parent's stack, and running the child there
or by
(b) "read-only returning" on the existing stack - basically, the interceptor needs a special epilogue that does the run thing when executed twice. @pcc

[morehouse]

@eugenis: Status?

[philippv]

Now, the next plan is to intercept vfork() and unpoison the unallocated part of the stack in the parent process after the child is done. But there is a catch - vfork() can not be intercepted.

Would it work to intercept execve / _exit instead, and unpoison stack from there?

[dvyukov]

_exit discards address space and execve completely replaces address space, so doing anything there looks pointless (can't have any effect on anything).

[philippv]

But after vfork and until execve child process is operating in address space of the parent, intercepting execve would allow us to inject desired epilogue to do necessary cleanup of leftover poisoning before replacing the address space?

[philippv]

Also why cannot we unpoison now-unallocated part of the parent process stack from vfork interceptor itself immediately after control is transferred back to parent process?

[dvyukov]

But after vfork and until execve child process is operating in address space of the parent, intercepting execve would allow us to inject desired epilogue to do necessary cleanup of leftover poisoning before replacing the address space?

Ah, I see what you mean. My bad. @eugenis what do you think.

[eugenis]

If we intercept execve, we'll need to intercept _exit, and some other functions as well. The full list is not clear to me. Intercepting vfork will work, but is extremely platform-dependent and must be done in assembly. Note that a function that calls vfork can not return in the child process, or it would corrupt the stack frame in the parent.

…

On Wed, Oct 10, 2018 at 8:11 AM Dmitry Vyukov ***@***.***> wrote: But after vfork and until execve child process is operating in address space of the parent, intercepting execve would allow us to inject desired epilogue to do necessary cleanup of leftover poisoning before replacing the address space? Ah, I see what you mean. My bad. @eugenis <https://github.com/eugenis> what do you think. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#925 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAZuSvt3QUuXuZ7u-tzSTj0MA9rd2RyCks5ujg4VgaJpZM4SuYXN> .

[philippv]

Yeah, agree that intercepting vfork itself is going to be platform-dependent. But injecting epilogue through intercepting only _exit and execve should be enough to cover POSIX-compliant usage of vfork (though POSIX just under-specifies behavior of any function calls other than _exit and exec function family [frontends for execve], so there is some gray area)?

[eugenis]

Yes, it should work in practice. We'd need to intercept a ton of exec* functions, but it still feels better than a platform specific vfork interceptor.

[jmgao]

Intercepting execve doesn't seem like it's the right way to go, since it can fail. This code doesn't seem completely insane to me, for example:

pid_t pid = vfork();
if (pid == 0) {
    execl("/bin/foo", "foo", nullptr);
    execl("/usr/bin/foo", "foo", nullptr);
    execl("/usr/local/bin/foo", "foo", nullptr);
    _exit(1);
}

[eugenis]

Good point. Sounds like we will have to bite the bullet and wrap vfork in assembly. We can not do the system call directly, because libc function may need to maintain internal state - ex. in bionic it clears the cached pid. To wrap the libc function, we'd need a place to store the return address, and we can not use stack for that. Libc has it easy - the kernel preserves more registers than userspace ABI function does. Bionic stores the return address in RDI (x86_64) and LR (arm64). Our interceptor has the same ABI as the libc function, so there are no free registers. We could use a thread local slot, or just grab a lock and use a global variable. Does anyone have a better idea? Note that in a multithreaded program two threads can call vfork simultaneously. That would be completely crazy, of course, but the man page states that only the calling thread is frozen while vfork child runs. Nested vforks are not allowed. A lock can be used to serialize all vfork calls. Child threads will not touch the lock (i.e. will run with lock held).

…

On Mon, Dec 3, 2018 at 1:40 PM Josh Gao ***@***.***> wrote: Intercepting execve doesn't seem like it's the right way to go, since it can fail. This code doesn't seem completely insane to me, for example: pid_t pid = vfork();if (pid == 0) { execl("/bin/foo", "foo", nullptr); execl("/usr/bin/foo", "foo", nullptr); execl("/usr/local/bin/foo", "foo", nullptr); _exit(1); } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#925 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAZuSpGuak1LHsJUs-Q5iYR8oRaL-5rEks5u1Zo2gaJpZM4SuYXN> .

[jmgao]

Do you actually need to stash the return address? Can't you just tail call to your unpoisoning function? e.g. something like:

vfork:
  bl __real_vfork
  cmp r0, #0
  bxgt __vfork_unpoison_stack
  bx lr

pid_t __vfork_unpoison_stack(pid_t rc) {
   // ...
   return rc;
}

[eugenis]

On Mon, Dec 3, 2018 at 3:00 PM Josh Gao ***@***.***> wrote: Do you actually need to stash the return address? Can't you just tail call to your unpoisoning function? e.g. something like: vfork: bl __real_vfork

Did not you just destroy the return address in LR?

…

cmp r0, #0 bxgt __vfork_unpoison_stack bx lr pid_t __vfork_unpoison_stack(pid_t rc) { // ... return rc; } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[jmgao]

Did not you just destroy the return address in LR?

Oops, right, I'm dumb. Sticking it in TLS seems like the best option to me.

Note that in a multithreaded program two threads can call vfork
simultaneously. That would be completely crazy, of course, but the man page
states that only the calling thread is frozen while vfork child runs.

bionic uses vfork in posix_spawn if you either explicitly ask for vfork, or if you're doing something trivial that boils down to if (fork() == 0) execve(...), so multithreaded vfork is probably more common than you expect.

[eugenis]

I've uploaded a first revision of asm interceptor for vfork using thread-local storage for return address here:
https://reviews.llvm.org/D58313

[eugenis]

r355030 intercepts vfork in hwasan & asan on x86, x86_64, arm, aarch64.