Don't start the SECCOMP sandbox early for Tegra124

Don't start the SECCOMP sandbox early for Tegra124

Let the sandbox start normally and don't start the sandbox early.
Tegra124 libraries no longer need preloading of libraries or a whitelist
policy.
Add flag --gpu-sandbox-start-after-initialization to prevent the sandbox starting early.

BUG=none
TEST=Boot browser and about:gpu shows sandboxed to be true.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260130

[davidung, 2014-03-19 01:10:44]

I've re-based the patch to ToT, but I've been having trouble uploading it.

I've file a bug here
https://code.google.com/p/chromium/issues/detail?id=352481

[piman, 2014-03-19 01:16:55]

https://codereview.chromium.org/179983006/diff/1/content/browser/gpu/gpu_proc...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/179983006/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.cc:1096: switches::kGpuSandboxStartLater,
Can you add this to chrome/browser/chromeos/login/chrome_restart_request.cc so
that it also applies to guest mode?

[davidung, 2014-03-19 01:21:56]

https://codereview.chromium.org/179983006/diff/1/content/browser/gpu/gpu_proc...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/179983006/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.cc:1096: switches::kGpuSandboxStartLater,
On 2014/03/19 01:16:55, piman wrote:
> Can you add this to chrome/browser/chromeos/login/chrome_restart_request.cc so
> that it also applies to guest mode?

yes, the new rebased patch adds this.
but in the last week I hasn't been able to upload the new patch.

[Jorge Lucangeli Obes, 2014-03-19 16:44:05]

Can you try uploading the new patch? There's been some small changes in this
area lately.

Thanks!

https://codereview.chromium.org/179983006/diff/1/content/public/common/conten...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/179983006/diff/1/content/public/common/conten...
content/public/common/content_switches.cc:674: const char
kGpuSandboxStartLater[]          = "gpu-sandbox-start-later";
We should call this one something like "gpu-sandbox-delay-init".

[davidung, 2014-03-19 22:42:52]

On 2014/03/19 16:44:05, Jorge Lucangeli Obes wrote:
> Can you try uploading the new patch? There's been some small changes in this
> area lately.
> 

no good.  still looks like I have no access to "telemetry"

Traceback (most recent call last):
  <module> at /home/davidu/depot_tools/git_cl.py:2544
    sys.exit(main(sys.argv[1:]))
  main at /home/davidu/depot_tools/git_cl.py:2530
    return dispatcher.execute(OptionParser(), argv)
  execute at /home/davidu/depot_tools/subcommand.py:245
    return command(parser, args[1:])
  CMDupload at /home/davidu/depot_tools/git_cl.py:1700
    change=change)
  RunHook at /home/davidu/depot_tools/git_cl.py:805
    rietveld_obj=self.RpcServer())
  DoPresubmitChecks at /home/davidu/depot_tools/presubmit_support.py:1364
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  ExecPresubmitScript at /home/davidu/depot_tools/presubmit_support.py:1281
    result = eval(function_name + '(*__args)', context)
  <module> at content/test/gpu/page_sets/<string>:1
    None
  CheckChangeOnUpload at content/test/gpu/page_sets/<string>:70
    None
  _SyncFilesToCloud at content/test/gpu/page_sets/<string>:28
    None
  List at tools/telemetry/telemetry/page/cloud_storage.py:125
    stdout = _RunCommand(['ls', query])
  _RunCommand at tools/telemetry/telemetry/page/cloud_storage.py:115
    raise PermissionError(gsutil_path)

Locals:
  args        : ['ls', 'gs://chrome-telemetry/']
  gsutil      : <subprocess.Popen object at 0x3f15be90>
  gsutil_path : '/home/davidu/depot_tools/third_party/gsutil/gsutil'
  stderr      : 'GSResponseError: status=403, code=AccessDenied,
reason=Forbidden.\n'
  stdout      : ''

[jln (very slow on Chromium), 2014-03-20 00:21:01]

Great news, thanks davidung.

https://chromiumcodereview.appspot.com/179983006/diff/1/content/public/common...
File content/public/common/content_switches.cc (right):

https://chromiumcodereview.appspot.com/179983006/diff/1/content/public/common...
content/public/common/content_switches.cc:674: const char
kGpuSandboxStartLater[]          = "gpu-sandbox-start-later";
On 2014/03/19 16:44:05, Jorge Lucangeli Obes wrote:
> We should call this one something like "gpu-sandbox-delay-init".

It would be nice to make it clear that the "normal", expected behavior is the
one that NVidia is now following. I would prefer if Mali was the one that
requires a special "early initialization" flag.

Since it's not a trivial change, maybe a name such as
"gpu-sandbox-start-after-initialization" or the one Jorge suggested is ok in the
meantime.

But it'd be nice to reverse the polarity and have Mali use
"--do-sandbox-early-init-with-horrible-hacks-please-dont-use-me" at some point.

[Jorge Lucangeli Obes, 2014-03-20 00:27:43]

On 2014/03/19 22:42:52, davidung wrote:
> On 2014/03/19 16:44:05, Jorge Lucangeli Obes wrote:
> > Can you try uploading the new patch? There's been some small changes in this
> > area lately.
> > 
> 
> no good.  still looks like I have no access to "telemetry"
> 
> Traceback (most recent call last):
>   <module> at /home/davidu/depot_tools/git_cl.py:2544
>     sys.exit(main(sys.argv[1:]))
>   main at /home/davidu/depot_tools/git_cl.py:2530
>     return dispatcher.execute(OptionParser(), argv)
>   execute at /home/davidu/depot_tools/subcommand.py:245
>     return command(parser, args[1:])
>   CMDupload at /home/davidu/depot_tools/git_cl.py:1700
>     change=change)
>   RunHook at /home/davidu/depot_tools/git_cl.py:805
>     rietveld_obj=self.RpcServer())
>   DoPresubmitChecks at /home/davidu/depot_tools/presubmit_support.py:1364
>     results += executer.ExecPresubmitScript(presubmit_script, filename)
>   ExecPresubmitScript at /home/davidu/depot_tools/presubmit_support.py:1281
>     result = eval(function_name + '(*__args)', context)
>   <module> at content/test/gpu/page_sets/<string>:1
>     None
>   CheckChangeOnUpload at content/test/gpu/page_sets/<string>:70
>     None
>   _SyncFilesToCloud at content/test/gpu/page_sets/<string>:28
>     None
>   List at tools/telemetry/telemetry/page/cloud_storage.py:125
>     stdout = _RunCommand(['ls', query])
>   _RunCommand at tools/telemetry/telemetry/page/cloud_storage.py:115
>     raise PermissionError(gsutil_path)
> 
> Locals:
>   args        : ['ls', 'gs://chrome-telemetry/']
>   gsutil      : <subprocess.Popen object at 0x3f15be90>
>   gsutil_path : '/home/davidu/depot_tools/third_party/gsutil/gsutil'
>   stderr      : 'GSResponseError: status=403, code=AccessDenied,
> reason=Forbidden.\n'
>   stdout      : ''

I've emailed chromium-dev@chromium.org to see if we can figure out what's going
on. Thanks for your patience!

[Jorge Lucangeli Obes, 2014-03-20 21:50:18]

On 2014/03/20 00:27:43, Jorge Lucangeli Obes wrote:
> On 2014/03/19 22:42:52, davidung wrote:
> > On 2014/03/19 16:44:05, Jorge Lucangeli Obes wrote:
> > > Can you try uploading the new patch? There's been some small changes in
this
> > > area lately.
> > > 
> > 
> > no good.  still looks like I have no access to "telemetry"
> > 
> > Traceback (most recent call last):
> >   <module> at /home/davidu/depot_tools/git_cl.py:2544
> >     sys.exit(main(sys.argv[1:]))
> >   main at /home/davidu/depot_tools/git_cl.py:2530
> >     return dispatcher.execute(OptionParser(), argv)
> >   execute at /home/davidu/depot_tools/subcommand.py:245
> >     return command(parser, args[1:])
> >   CMDupload at /home/davidu/depot_tools/git_cl.py:1700
> >     change=change)
> >   RunHook at /home/davidu/depot_tools/git_cl.py:805
> >     rietveld_obj=self.RpcServer())
> >   DoPresubmitChecks at /home/davidu/depot_tools/presubmit_support.py:1364
> >     results += executer.ExecPresubmitScript(presubmit_script, filename)
> >   ExecPresubmitScript at /home/davidu/depot_tools/presubmit_support.py:1281
> >     result = eval(function_name + '(*__args)', context)
> >   <module> at content/test/gpu/page_sets/<string>:1
> >     None
> >   CheckChangeOnUpload at content/test/gpu/page_sets/<string>:70
> >     None
> >   _SyncFilesToCloud at content/test/gpu/page_sets/<string>:28
> >     None
> >   List at tools/telemetry/telemetry/page/cloud_storage.py:125
> >     stdout = _RunCommand(['ls', query])
> >   _RunCommand at tools/telemetry/telemetry/page/cloud_storage.py:115
> >     raise PermissionError(gsutil_path)
> > 
> > Locals:
> >   args        : ['ls', 'gs://chrome-telemetry/']
> >   gsutil      : <subprocess.Popen object at 0x3f15be90>
> >   gsutil_path : '/home/davidu/depot_tools/third_party/gsutil/gsutil'
> >   stderr      : 'GSResponseError: status=403, code=AccessDenied,
> > reason=Forbidden.\n'
> >   stdout      : ''
> 
> I've emailed mailto:chromium-dev@chromium.org to see if we can figure out
what's going
> on. Thanks for your patience!

Can you run with -vvv to get some more debugging info? In the meantime, you can
also use --bypass-hooks.

[davidung, 2014-03-21 02:02:52]

> Can you run with -vvv to get some more debugging info? In the meantime, you
can
> also use --bypass-hooks.

after another rebase its working now.  I've uploaded patchset 2.

[davidung, 2014-03-21 20:29:16]

On 2014/03/21 02:02:52, davidung wrote:
> > Can you run with -vvv to get some more debugging info? In the meantime, you
> can
> > also use --bypass-hooks.
> 
> after another rebase its working now.  I've uploaded patchset 2.

"git cl upload" is working again, so I can't reproduce with -vvv to get more
info sorry.

[Jorge Lucangeli Obes, 2014-03-21 20:31:48]

On 2014/03/21 20:29:16, davidung wrote:
> On 2014/03/21 02:02:52, davidung wrote:
> > > Can you run with -vvv to get some more debugging info? In the meantime,
you
> > can
> > > also use --bypass-hooks.
> > 
> > after another rebase its working now.  I've uploaded patchset 2.
> 
> "git cl upload" is working again, so I can't reproduce with -vvv to get more
> info sorry.

No worries, as long as it's working for you then it's fine.

I'll test this quickly on Mali before stamping.

[piman, 2014-03-21 21:26:47]

OWNER LGTM once reviewers are happy.

[davidung, 2014-03-25 23:37:03]

any update with the Mali tests?

[Jorge Lucangeli Obes, 2014-03-25 23:38:31]

On 2014/03/25 23:37:03, davidung wrote:
> any update with the Mali tests?

Hi David,

They will be done tomorrow morning.

[Jorge Lucangeli Obes, 2014-03-26 17:15:11]

Tested Mali and everything works.

https://codereview.chromium.org/179983006/diff/20001/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (right):

https://codereview.chromium.org/179983006/diff/20001/content/gpu/gpu_main.cc#...
content/gpu/gpu_main.cc:214: switches::kGpuSandboxStartAfterInitialization)) {
I think the correct indentation here is four spaces after the '(' on the new
line. You can use 'clang-format' or 'git cl format' for this.

[davidung, 2014-03-26 22:19:14]

https://codereview.chromium.org/179983006/diff/20001/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (right):

https://codereview.chromium.org/179983006/diff/20001/content/gpu/gpu_main.cc#...
content/gpu/gpu_main.cc:214: switches::kGpuSandboxStartAfterInitialization)) {
On 2014/03/26 17:15:11, Jorge Lucangeli Obes wrote:
> I think the correct indentation here is four spaces after the '(' on the new
> line. You can use 'clang-format' or 'git cl format' for this.

ugh, it's 5 spaces after running 'clang-format'

[Jorge Lucangeli Obes, 2014-03-26 22:25:34]

On 2014/03/26 22:19:14, davidung wrote:
> https://codereview.chromium.org/179983006/diff/20001/content/gpu/gpu_main.cc
> File content/gpu/gpu_main.cc (right):
> 
>
https://codereview.chromium.org/179983006/diff/20001/content/gpu/gpu_main.cc#...
> content/gpu/gpu_main.cc:214: switches::kGpuSandboxStartAfterInitialization)) {
> On 2014/03/26 17:15:11, Jorge Lucangeli Obes wrote:
> > I think the correct indentation here is four spaces after the '(' on the new
> > line. You can use 'clang-format' or 'git cl format' for this.
> 
> ugh, it's 5 spaces after running 'clang-format'

No worries, whatever clang-format --style=chromium says, that's the correct
thing.

[davidung, 2014-03-27 00:44:10]

patchset 3 uploaded
can someone kick off the needed try-bots?

[jln (very slow on Chromium), 2014-03-27 00:56:18]

I don't think we should delay this CL more, so feel free to land when Jorge is
happy about it.

However, hopefully with the late initialization we don't need to do all the
unsafe complicated hacks that we do for Mali.
I hope that Nvidia ARM can use the normal GPU policy at this point.

So we should now have two policies:
- A main GPU policy (used for x86 + Nvidia ARM)
- An early GPU policy (used for Mali)

I created https://crbug.com/356959/ to track this.

https://chromiumcodereview.appspot.com/179983006/diff/60001/content/common/sa...
File content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc (right):

https://chromiumcodereview.appspot.com/179983006/diff/60001/content/common/sa...
content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc:80: static const
char kXAuthorityPath[] = "/home/chronos/.Xauthority";
I imagine we could have a new "sane" mode, where we don't do any of this now for
the "--gpu-sandbox-start-after-initialization" case.

https://chromiumcodereview.appspot.com/179983006/diff/60001/content/common/sa...
content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc:139: case
__NR_connect:
This stuff is hopefully not needed either with
--gpu-sandbox-start-after-initialization

[jln (very slow on Chromium), 2014-03-27 00:58:16]

On 2014/03/27 00:56:18, jln wrote:

> I created https://crbug.com/356959/ to track this.

Err, the correct link is https://crbug.com/356959

[Jorge Lucangeli Obes, 2014-03-27 01:11:06]

I think it's reasonable to land this CL to remove David's TODOs and then do the
cleanup that Julien mentioned. I'm happy to own that cleanup, I'll see if I can
get a board.

lgtm

[davidung, 2014-03-27 01:38:36]

The CQ bit was checked by davidu@nvidia.com

[davidung, 2014-03-27 01:38:37]

The CQ bit was unchecked by davidu@nvidia.com

[davidung, 2014-03-27 01:38:38]

The CQ bit was checked by davidu@nvidia.com

[davidung, 2014-03-27 01:38:41]

The CQ bit was unchecked by davidu@nvidia.com

[davidung, 2014-03-27 01:50:51]

The CQ bit was checked by davidu@nvidia.com

[davidung, 2014-03-27 01:50:55]

The CQ bit was unchecked by davidu@nvidia.com

[davidung, 2014-03-27 18:05:50]

The CQ bit was checked by davidu@nvidia.com

[commit-bot: I haz the power, 2014-03-27 18:09:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidu@nvidia.com/179983006/60001

[commit-bot: I haz the power, 2014-03-27 20:44:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 20:44:10]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[jln (very slow on Chromium), 2014-03-28 12:38:14]

The CQ bit was checked by jln@chromium.org

[commit-bot: I haz the power, 2014-03-28 12:38:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidu@nvidia.com/179983006/60001

[commit-bot: I haz the power, 2014-03-28 15:31:20]

Change committed as 260130