chroot to /proc instead of /tmp. This gets rid of a lot of unnecessary

chroot to /proc instead of /tmp. This gets rid of a lot of unnecessary
complexity and fixes a race condition.

(Original idea from Markus)

The chroot helper will chroot to /proc/self/fdinfo (or /proc/self/fd). This is
pretty safe because access to this directory is protected by the ptrace() check
in the kernel and the helper is privileged.

Moreover, as soon as the helper _exit() and becomes a zombie, the directory
will be empty. Zygote should wait() for us to make everything deterministric.

We also export SBX_HELPER_PID so that Zygote can specifically wait for the
helper.

[Markus (顧孟勤), 2011-03-24 23:26:58]

LGTM

http://codereview.chromium.org/6731031/diff/1/sandbox/linux/suid/sandbox.c
File sandbox/linux/suid/sandbox.c (right):

http://codereview.chromium.org/6731031/diff/1/sandbox/linux/suid/sandbox.c#ne...
sandbox/linux/suid/sandbox.c:70: #define SAFE_DIR2 "/proc/self/fd"
Do we ever need to worry about nesting of setuid sandboxes?

If CLONE_NEWPID is in effect, /proc/self doesn't actually point to us :-(

We could add a sanity check, if we are really worried about this.

[jln (very slow on Chromium), 2011-03-24 23:33:48]

On Thu, Mar 24, 2011 at 4:26 PM,  <markus@chromium.org> wrote:
> LGTM
>
>
> http://codereview.chromium.org/6731031/diff/1/sandbox/linux/suid/sandbox.c
> File sandbox/linux/suid/sandbox.c (right):
>
>
http://codereview.chromium.org/6731031/diff/1/sandbox/linux/suid/sandbox.c#ne...
> sandbox/linux/suid/sandbox.c:70: #define SAFE_DIR2 "/proc/self/fd"
> Do we ever need to worry about nesting of setuid sandboxes?
>
> If CLONE_NEWPID is in effect, /proc/self doesn't actually point to us
> :-(
>
> We could add a sanity check, if we are really worried about this.

I would be pretty worried about it. Hopefully the ptrace() check would
hold (in both fdinfo and fd cases), but we would lood the property of
an empty directory as soon as the helper becomes a zombie (or dies for
real).

But I'll be surprised if it's the case. In any of the namespaces where
we exist, /proc/self should work. This could be a kernel bug in your
specific kernel, but I don't recall anything like that. I've performed
quite extensive testing of this with the standalone version of the
setuid sandbox and I didn't notice anything.

What kernel do you use ?

Julien

[agl, 2011-03-25 14:05:21]

LGTM

http://codereview.chromium.org/6731031/diff/1/sandbox/linux/suid/sandbox.c
File sandbox/linux/suid/sandbox.c (right):

http://codereview.chromium.org/6731031/diff/1/sandbox/linux/suid/sandbox.c#ne...
sandbox/linux/suid/sandbox.c:79: char  *safedir = NULL;
double space.