Linux sandbox: Restrict sched_* and ioprio_* calls in the cros arm GPU policy.

Linux sandbox: Restrict sched_* and ioprio_* calls in the cros arm GPU policy.

BUG=413855
Committed: https://crrev.com/a401ea2a1d6bc90d2b3d7eab1fd80e8d45e83dea
Cr-Commit-Position: refs/heads/master@{#300046}

[jln (very slow on Chromium), 2014-10-09 00:31:23]

jln@chromium.org changed reviewers:
+ jln@chromium.org

[jln (very slow on Chromium), 2014-10-09 00:31:24]

lgtm but proceed with caution. Send a heads' up to graphics-dev@ as mentioned
before.

When you tested, did you check in about:gpu that the sandbox was enabled? (I'm
pretty sure we have a bunch of checks to make sure that's the case nowadays, but
I don't recall precisely, it has changed many times).

https://codereview.chromium.org/640123002/diff/1/sandbox/linux/seccomp-bpf-he...
File sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc
(right):

https://codereview.chromium.org/640123002/diff/1/sandbox/linux/seccomp-bpf-he...
sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc:256: If(who
== 0 || who == target_pid, Allow()).Else(Error(EPERM)))
No case spotted of who == gettid() ?

That's good news. Did you get any sense of what wraps these calls in practice?
Could they be directly used by a GPU driver?
If we don't feel confident it can't happen, I wonder if we shouldn't have a
SIGSYS again here to support this case.

[rickyz (no longer on Chrome), 2014-10-09 22:58:21]

On 2014/10/09 00:31:24, jln (away til Oct 14th) wrote:
> lgtm but proceed with caution. Send a heads' up to graphics-dev@ as mentioned
> before.
> 
> When you tested, did you check in about:gpu that the sandbox was enabled? (I'm
> pretty sure we have a bunch of checks to make sure that's the case nowadays,
but
> I don't recall precisely, it has changed many times).
> 
>
https://codereview.chromium.org/640123002/diff/1/sandbox/linux/seccomp-bpf-he...
> File sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc
> (right):
> 
>
https://codereview.chromium.org/640123002/diff/1/sandbox/linux/seccomp-bpf-he...
> sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc:256:
If(who
> == 0 || who == target_pid, Allow()).Else(Error(EPERM)))
> No case spotted of who == gettid() ?
> 
> That's good news. Did you get any sense of what wraps these calls in practice?
> Could they be directly used by a GPU driver?
> If we don't feel confident it can't happen, I wonder if we shouldn't have a
> SIGSYS again here to support this case.

Sorry for the slow response - ran into some issues with my build setup. I did
verify that the sandbox was enabled, but I had not investigated how these calls
were used until now, and from what I can tell, the mali driver does not use any
pthread functions that should trigger these syscalls. I also dug through the
history to find out when these syscalls were whitelisted. It looks like they
were whitelisted in:

https://chromiumcodereview.appspot.com/13974008/

and the comments seem to indicate that most of them weren't actually strictly
necessary. Since the necessary ones were are already allowed in the GPU policy
that this inherits from (and based on some manual testing), I think it should be
safe to just pass through to the base GPU policy for the advanced scheduler
syscalls.

[rickyz (no longer on Chrome), 2014-10-17 01:50:52]

On 2014/10/09 22:58:21, rickyz wrote:
> On 2014/10/09 00:31:24, jln (away til Oct 14th) wrote:
> > lgtm but proceed with caution. Send a heads' up to graphics-dev@ as
mentioned
> > before.
> > 
> > When you tested, did you check in about:gpu that the sandbox was enabled?
(I'm
> > pretty sure we have a bunch of checks to make sure that's the case nowadays,
> but
> > I don't recall precisely, it has changed many times).
> > 
> >
>
https://codereview.chromium.org/640123002/diff/1/sandbox/linux/seccomp-bpf-he...
> > File sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc
> > (right):
> > 
> >
>
https://codereview.chromium.org/640123002/diff/1/sandbox/linux/seccomp-bpf-he...
> > sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc:256:
> If(who
> > == 0 || who == target_pid, Allow()).Else(Error(EPERM)))
> > No case spotted of who == gettid() ?
> > 
> > That's good news. Did you get any sense of what wraps these calls in
practice?
> > Could they be directly used by a GPU driver?
> > If we don't feel confident it can't happen, I wonder if we shouldn't have a
> > SIGSYS again here to support this case.
> 
> Sorry for the slow response - ran into some issues with my build setup. I did
> verify that the sandbox was enabled, but I had not investigated how these
calls
> were used until now, and from what I can tell, the mali driver does not use
any
> pthread functions that should trigger these syscalls. I also dug through the
> history to find out when these syscalls were whitelisted. It looks like they
> were whitelisted in:
> 
> https://chromiumcodereview.appspot.com/13974008/
> 
> and the comments seem to indicate that most of them weren't actually strictly
> necessary. Since the necessary ones were are already allowed in the GPU policy
> that this inherits from (and based on some manual testing), I think it should
be
> safe to just pass through to the base GPU policy for the advanced scheduler
> syscalls.

Going to try pushing this and see what breaks :-). I ran the tests suggested by
graphics-dev on a daisy - all but one tests passed - I had a failure on
PlaybackPerf.h264 with "ERROR: Can not use hardware decoding." I think this is
unrelated to sandboxing, as none of the tests triggered any sandbox violations.

[rickyz (no longer on Chrome), 2014-10-17 01:50:58]

The CQ bit was checked by rickyz@chromium.org

[commit-bot: I haz the power, 2014-10-17 01:51:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/640123002/20001

[rickyz (no longer on Chrome), 2014-10-17 01:52:35]

On 2014/10/17 01:50:52, rickyz wrote:
> On 2014/10/09 22:58:21, rickyz wrote:
> > On 2014/10/09 00:31:24, jln (away til Oct 14th) wrote:
> > > lgtm but proceed with caution. Send a heads' up to graphics-dev@ as
> mentioned
> > > before.
> > > 
> > > When you tested, did you check in about:gpu that the sandbox was enabled?
> (I'm
> > > pretty sure we have a bunch of checks to make sure that's the case
nowadays,
> > but
> > > I don't recall precisely, it has changed many times).
> > > 
> > >
> >
>
https://codereview.chromium.org/640123002/diff/1/sandbox/linux/seccomp-bpf-he...
> > > File sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/640123002/diff/1/sandbox/linux/seccomp-bpf-he...
> > > sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc:256:
> > If(who
> > > == 0 || who == target_pid, Allow()).Else(Error(EPERM)))
> > > No case spotted of who == gettid() ?
> > > 
> > > That's good news. Did you get any sense of what wraps these calls in
> practice?
> > > Could they be directly used by a GPU driver?
> > > If we don't feel confident it can't happen, I wonder if we shouldn't have
a
> > > SIGSYS again here to support this case.
> > 
> > Sorry for the slow response - ran into some issues with my build setup. I
did
> > verify that the sandbox was enabled, but I had not investigated how these
> calls
> > were used until now, and from what I can tell, the mali driver does not use
> any
> > pthread functions that should trigger these syscalls. I also dug through the
> > history to find out when these syscalls were whitelisted. It looks like they
> > were whitelisted in:
> > 
> > https://chromiumcodereview.appspot.com/13974008/
> > 
> > and the comments seem to indicate that most of them weren't actually
strictly
> > necessary. Since the necessary ones were are already allowed in the GPU
policy
> > that this inherits from (and based on some manual testing), I think it
should
> be
> > safe to just pass through to the base GPU policy for the advanced scheduler
> > syscalls.
> 
> Going to try pushing this and see what breaks :-). I ran the tests suggested
by
> graphics-dev on a daisy - all but one tests passed - I had a failure on
> PlaybackPerf.h264 with "ERROR: Can not use hardware decoding." I think this is
> unrelated to sandboxing, as none of the tests triggered any sandbox
violations.

Forgot to mention, the graphics-dev thread was
https://groups.google.com/a/chromium.org/forum/#!topic/graphics-dev/NmVaIxWj1gY,
and the tests were video_PlaybackPerf and video_WebRtcPerf.

[commit-bot: I haz the power, 2014-10-17 02:33:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-17 02:34:06]

Patchset 2 (id:??) landed as
https://crrev.com/a401ea2a1d6bc90d2b3d7eab1fd80e8d45e83dea
Cr-Commit-Position: refs/heads/master@{#300046}