Disable Chrome OS seccomp filter GPU sandbox.

Disable Chrome OS seccomp filter GPU sandbox.

The seccomp filter sandbox for the GPU process is causing crashes
on Chrome OS. Disable it until we develop a more robust
implementation.

BUG=127491
TEST=None


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

[Jorge Lucangeli Obes, 2012-05-10 03:05:33]

PTAL

Still need to add appropriate OWNERS as reviewers.

[jln (very slow on Chromium), 2012-05-10 03:16:39]

https://chromiumcodereview.appspot.com/10388063/diff/1/content/common/sandbox...
File content/common/sandbox_init_linux.cc (left):

https://chromiumcodereview.appspot.com/10388063/diff/1/content/common/sandbox...
content/common/sandbox_init_linux.cc:338: return;
What about making a EnableGpuSandbox switch and have it disabled as the default
?

If you don't want to do that now, wouldn't it be cleaner and easier for us to
patch this back-in for testing if you just commented out line 337 (with a
comment explaining that it's disabled for now) ?

I would leave the rest of the code as is.

https://chromiumcodereview.appspot.com/10388063/diff/1/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (left):

https://chromiumcodereview.appspot.com/10388063/diff/1/content/gpu/gpu_main.c...
content/gpu/gpu_main.cc:108: #endif
I would also leave that as is

[piman, 2012-05-10 03:18:21]

LGTM, thanks

[Jorge Lucangeli Obes, 2012-05-10 03:38:34]

https://chromiumcodereview.appspot.com/10388063/diff/1/content/common/sandbox...
File content/common/sandbox_init_linux.cc (left):

https://chromiumcodereview.appspot.com/10388063/diff/1/content/common/sandbox...
content/common/sandbox_init_linux.cc:338: return;
On 2012/05/10 03:16:39, Julien Tinnes wrote:
> What about making a EnableGpuSandbox switch and have it disabled as the
default
> ?
> 
> If you don't want to do that now, wouldn't it be cleaner and easier for us to
> patch this back-in for testing if you just commented out line 337 (with a
> comment explaining that it's disabled for now) ?
> 
> I would leave the rest of the code as is.

Agreed, having the change in only one place makes more sense.

https://chromiumcodereview.appspot.com/10388063/diff/1/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (left):

https://chromiumcodereview.appspot.com/10388063/diff/1/content/gpu/gpu_main.c...
content/gpu/gpu_main.cc:108: #endif
On 2012/05/10 03:16:39, Julien Tinnes wrote:
> I would also leave that as is

Done.

[commit-bot: I haz the power, 2012-05-10 05:08:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jorgelo@chromium.org/10388063/3001

[commit-bot: I haz the power, 2012-05-10 05:08:44]

Presubmit check for 10388063-3001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/common

Presubmit checks took 8.0s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[Jorge Lucangeli Obes, 2012-05-10 05:18:14]

jam@, cevans@, requesting OWNERS rubberstamp. Thanks!

[cevans, 2012-05-10 05:58:04]

Hang on, I'll weight in in a moment.

On Wed, May 9, 2012 at 10:18 PM, <jorgelo@chromium.org> wrote:

> jam@, cevans@, requesting OWNERS rubberstamp. Thanks!
>
>
https://chromiumcodereview.**appspot.com/10388063/<https://chromiumcodereview...
>

[cevans, 2012-05-10 07:06:28]

LGTM

Land whatever you need to to unblock the ChromeOS GPU guys.

IMHO, the better change would be to disable the call to
content::InitializeSandbox(). You could also change
if defined(OS_LINUX)
to add a !defined(OS_CHROMEOS) (or whatever you use) for now?

Or you could add --disable-gpu-sandbox to the ChromeOS flags?

etc :)

I'll be working tomorrow and will look into GPU vs. libnss initialization.

On Wed, May 9, 2012 at 10:58 PM, Chris Evans <cevans@google.com> wrote:

> Hang on, I'll weight in in a moment.
>
>
> On Wed, May 9, 2012 at 10:18 PM, <jorgelo@chromium.org> wrote:
>
>> jam@, cevans@, requesting OWNERS rubberstamp. Thanks!
>>
>>
https://chromiumcodereview.**appspot.com/10388063/<https://chromiumcodereview...
>>
>
>

[Jorge Lucangeli Obes, 2012-05-10 16:35:37]

On 2012/05/10 07:06:28, cevans wrote:
> LGTM
> 
> Land whatever you need to to unblock the ChromeOS GPU guys.
> 
> IMHO, the better change would be to disable the call to
> content::InitializeSandbox(). You could also change
> if defined(OS_LINUX)
> to add a !defined(OS_CHROMEOS) (or whatever you use) for now?
> 
> Or you could add --disable-gpu-sandbox to the ChromeOS flags?
> 
> etc :)
> 
> I'll be working tomorrow and will look into GPU vs. libnss initialization.
>

I'll land this and then talk to piman@ to see what's safe to re-enable.

We've already added --disable-seccomp-filter-sandbox to the Chrome OS session
manager but some devs launch Chrome directly on Chrome OS.

> On Wed, May 9, 2012 at 10:58 PM, Chris Evans <mailto:cevans@google.com> wrote:
> 
> > Hang on, I'll weight in in a moment.
> >
> >
> > On Wed, May 9, 2012 at 10:18 PM, <mailto:jorgelo@chromium.org> wrote:
> >
> >> jam@, cevans@, requesting OWNERS rubberstamp. Thanks!
> >>
> >>
>
https://chromiumcodereview.**appspot.com/10388063/%3Chttps://chromiumcoderevi...>
> >>
> >
> >

[commit-bot: I haz the power, 2012-05-10 16:35:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jorgelo@chromium.org/10388063/3001

[commit-bot: I haz the power, 2012-05-10 16:36:02]

Presubmit check for 10388063-3001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/common

Presubmit checks took 1.3s to calculate.

[Jorge Lucangeli Obes, 2012-05-10 16:37:27]

On 2012/05/10 16:36:02, I haz the power (commit-bot) wrote:
> Presubmit check for 10388063-3001 failed and returned exit status 1.
> 
> Running presubmit commit checks ...
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for files in these directories:
>     content/common
> 
> Presubmit checks took 1.3s to calculate.

Chris, can you LGTM from your @chromium address?

[Jorge Lucangeli Obes, 2012-05-10 17:20:00]

-jam since we're not touching code in content/ anymore
+kbr for OWNERS approval in gpu/

cevans, LGTM?

[piman, 2012-05-10 17:21:44]

Shouldn't it be disabled on linux too, because AFAIU it doesn't work yet?

[Jorge Lucangeli Obes, 2012-05-10 17:24:22]

On 2012/05/10 17:21:44, piman wrote:
> Shouldn't it be disabled on linux too, because AFAIU it doesn't work yet?

cevans repro'd on Linux and was planning on fixing today.

[Chris Evans, 2012-05-10 17:24:31]

On 2012/05/10 17:21:44, piman wrote:
> Shouldn't it be disabled on linux too, because AFAIU it doesn't work yet?

LGTM

It's been working on desktop Linux for a few dev channels (with the exception of
some Nvidia binary driver issues which are now fixed).

There's a recent regression with libnss initialization in the GPU process --
unrelated to the new sandbox code -- but I'll track it down and fix it today,
i.e. before the next dev channel release.

[Jorge Lucangeli Obes, 2012-05-10 18:45:35]

kbr, friendly ping for OWNERS LGTM =)

[apatrick_chromium, 2012-05-10 18:48:28]

content/gpu LGTM

[commit-bot: I haz the power, 2012-05-10 19:03:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jorgelo@chromium.org/10388063/5004

[commit-bot: I haz the power, 2012-05-10 19:17:14]

Try job failure for 10388063-5004 (retry) (retry) on android for steps "Compile,
build".
It's a second try, previously, steps "Compile, build" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...

[commit-bot: I haz the power, 2012-05-10 19:19:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jorgelo@chromium.org/10388063/5004

[commit-bot: I haz the power, 2012-05-10 20:55:30]

Change committed as 136384