Factor common syscall from GPU and Flash policies

Factor common syscall from GPU and Flash policies

In the Linux BPF sandbox, we factor the common syscalls between the
Flash and GPU policies into one common function.

This change introduces no semantic change in the policies.

BUG=
NOTRY=true


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150389

[jln (very slow on Chromium), 2012-08-07 05:18:41]

This change doesn't change the policies at all: the goal is to make the next set
of changes easier to review.

It allows us to highlight the differences between the Flash and GPU process
policies.

Moreover, system calls are now sorted by alphabetical order.

[Chris Evans, 2012-08-07 06:43:45]

https://chromiumcodereview.appspot.com/10837135/diff/2001/content/common/sand...
File content/common/sandbox_seccomp_bpf_linux.cc (right):

https://chromiumcodereview.appspot.com/10837135/diff/2001/content/common/sand...
content/common/sandbox_seccomp_bpf_linux.cc:244: case __NR_getegid:
Should we start preparing for the renderer policy by creating a "harmless" class
of syscall where we're 100% sure a syscall is super safe? These simple get*id
calls could go in to it.

https://chromiumcodereview.appspot.com/10837135/diff/2001/content/common/sand...
content/common/sandbox_seccomp_bpf_linux.cc:251: case __NR_mmap:
Unrelated to this CL I know, but maybe a TODO here to restrict mmap() flags? I
don't think Ubuntu ever patched the MAP_GROWSDOWN thing. Also, fcntl(), prctl(),
others...?

https://chromiumcodereview.appspot.com/10837135/diff/2001/content/common/sand...
content/common/sandbox_seccomp_bpf_linux.cc:334: case __NR_times:
Should we just fold times() into IsGettimeSyscall()? Every baby step towards PSS
seems useful :)

[jln (very slow on Chromium), 2012-08-07 08:02:41]

Let's hold this off until the next CL?

The next CL will make all existing x86_64 system calls explicit, still without
changing the semantics (many are commented out).

The next step will be to fully rationalize the policies, but I haven't finished
that yet.

https://chromiumcodereview.appspot.com/10837135/diff/2001/content/common/sand...
File content/common/sandbox_seccomp_bpf_linux.cc (right):

https://chromiumcodereview.appspot.com/10837135/diff/2001/content/common/sand...
content/common/sandbox_seccomp_bpf_linux.cc:244: case __NR_getegid:
On 2012/08/07 06:43:45, Chris Evans wrote:
> Should we start preparing for the renderer policy by creating a "harmless"
class
> of syscall where we're 100% sure a syscall is super safe? These simple get*id
> calls could go in to it.

Done in the following CL. I went through every x86_64 system call and classified
it. The next CL will have us have every system call in the file and make a
conscious decision about it.

https://chromiumcodereview.appspot.com/10837135/diff/2001/content/common/sand...
content/common/sandbox_seccomp_bpf_linux.cc:251: case __NR_mmap:
On 2012/08/07 06:43:45, Chris Evans wrote:
> Unrelated to this CL I know, but maybe a TODO here to restrict mmap() flags? I
> don't think Ubuntu ever patched the MAP_GROWSDOWN thing. Also, fcntl(),
prctl(),
> others...?

Yes, good point. There are quite a few where we'll want to restrict arguments.

https://chromiumcodereview.appspot.com/10837135/diff/2001/content/common/sand...
content/common/sandbox_seccomp_bpf_linux.cc:334: case __NR_times:
On 2012/08/07 06:43:45, Chris Evans wrote:
> Should we just fold times() into IsGettimeSyscall()? Every baby step towards
PSS
> seems useful :)

It is actually in my "global process environment" set, along with:
__NR_getrlimit
__NR_getrusage
__NR_times
__NR_personality  // can change its personality as well.
__NR_setrlimit
__NR_acct  // privileged
__NR_prlimit64  // like setrlimit / getrlimit

I will send that CL tomorrow, let's have that discussion then ? There is
definitely plenty to discuss, I'm not super happy with my current sets, but it's
a start.

[Jorge Lucangeli Obes, 2012-08-07 18:31:49]

With the current syscall number evaluation code, the fact that the case:
statements in the switch are in alphabetical order has nothing to do with the
final order of evaluation of the syscall right? And, since we're compressing
ranges, we mostly don't care about evaluation order.

[cevans, 2012-08-07 18:34:54]

LGTM, with nit: I still think NR_times logically goes with the AllowGetTime
concept. I know that may be rewritten with PSS, but no harm in having every
step along the way be clean too, etc.

On Tue, Aug 7, 2012 at 11:31 AM, <jorgelo@chromium.org> wrote:

> With the current syscall number evaluation code, the fact that the case:
> statements in the switch are in alphabetical order has nothing to do with
> the
> final order of evaluation of the syscall right? And, since we're
> compressing
> ranges, we mostly don't care about evaluation order.
>
>
https://chromiumcodereview.**appspot.com/10837135/<https://chromiumcodereview...
>

[jln (very slow on Chromium), 2012-08-07 18:49:50]

On 2012/08/07 18:34:54, cevans wrote:
> LGTM, with nit: I still think NR_times logically goes with the AllowGetTime
> concept. I know that may be rewritten with PSS, but no harm in having every
> step along the way be clean too, etc.

I don't think "times" belongs to AlloGetTime. As I said earlier, it's differs
significantly from those. It belongs with getrusage etc (see my earlier e-mail).

Julien

[jln (very slow on Chromium), 2012-08-07 18:50:30]

On 2012/08/07 18:31:49, Jorge Lucangeli Obes wrote:
> With the current syscall number evaluation code, the fact that the case:
> statements in the switch are in alphabetical order has nothing to do with the
> final order of evaluation of the syscall right? And, since we're compressing
> ranges, we mostly don't care about evaluation order.

Correct, that's why I sorted them now.

[Jorge Lucangeli Obes, 2012-08-07 19:19:24]

On 2012/08/07 18:50:30, Julien Tinnes wrote:
> On 2012/08/07 18:31:49, Jorge Lucangeli Obes wrote:
> > With the current syscall number evaluation code, the fact that the case:
> > statements in the switch are in alphabetical order has nothing to do with
the
> > final order of evaluation of the syscall right? And, since we're compressing
> > ranges, we mostly don't care about evaluation order.
> 
> Correct, that's why I sorted them now.

lgtm

[cevans, 2012-08-07 19:21:25]

On Tue, Aug 7, 2012 at 11:49 AM, <jln@chromium.org> wrote:

> On 2012/08/07 18:34:54, cevans wrote:
>
>> LGTM, with nit: I still think NR_times logically goes with the
>> AllowGetTime
>> concept. I know that may be rewritten with PSS, but no harm in having
>> every
>> step along the way be clean too, etc.
>>
>
> I don't think "times" belongs to AlloGetTime. As I said earlier, it's
> differs
> significantly from those. It belongs with getrusage etc (see my earlier
> e-mail).
>

LGTM


>
> Julien
>
>
https://chromiumcodereview.**appspot.com/10837135/<https://chromiumcodereview...
>

[commit-bot: I haz the power, 2012-08-07 19:23:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jln@chromium.org/10837135/2001

[commit-bot: I haz the power, 2012-08-07 19:23:11]

Change committed as 150389