1150765 - Content process sandboxing prevents hardware rendering for OpenGL on Mac with E10S

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Kelsey Gilbert [:jgilbert]]

Our content process sandboxing prevents us from getting a hardware-backed OpenGL context. OSX has a software renderer, so we are accidentally using that on e10s right now.

This means Mac's WebGL on E10S is not rendered by the GPU. (same for other GL users, such as SkiaGL, Flash, etc.)

With sandboxing disabled via setting the pref "security.sandbox.content.level" to 0, we get hardware rendering back.


Gecko will spew the GL renderer info for each created context when run with the envvar MOZ_GL_SPEW=1.

With e10s:
OpenGL version detected: 210
OpenGL vendor: Apple Inc.
OpenGL renderer: Apple Software Renderer
OpenGL vendor ('Apple Inc.') unrecognized

Without e10s:
OpenGL version detected: 210
OpenGL vendor: ATI Technologies Inc.
OpenGL renderer: AMD Radeon HD 6630M OpenGL Engine
OpenGL vendor ('ATI Technologies Inc.') recognized as: ATI

Comment 1

[André Reinald]

(In reply to Jeff Gilbert [:jgilbert] from comment #0)
> Our content process sandboxing prevents us from getting a hardware-backed
> OpenGL context. OSX has a software renderer, so we are accidentally using
> that on e10s right now.

Jeff, what you tell sounds familiar to me. Please provide the relevant portion of /var/log/system.log when trying to create the OpenGL context in e10s mode and with sandbox turned on (security.sandbox.content.level to 1).

Comment 2

[Kelsey Gilbert [:jgilbert]]

(In reply to André Reinald from comment #1)
> (In reply to Jeff Gilbert [:jgilbert] from comment #0)
> > Our content process sandboxing prevents us from getting a hardware-backed
> > OpenGL context. OSX has a software renderer, so we are accidentally using
> > that on e10s right now.
> 
> Jeff, what you tell sounds familiar to me. Please provide the relevant
> portion of /var/log/system.log when trying to create the OpenGL context in
> e10s mode and with sandbox turned on (security.sandbox.content.level to 1).

I think this is the relevant line?

Apr  3 13:01:09 jgilberts-Mac-mini.local sandboxd[257] ([82440]): plugin-container(82440) deny iokit-open AMDRadeonX3000_AMDAccelDevice

I'll attach the full log excerpt.

Comment 3

[Kelsey Gilbert [:jgilbert]]

Attachment: system.log snippet

Comment 4

[André Reinald]

Attachment: bugzilla-1150765-1.patch

Jeff, can you please confirm the above patch solves your issue?

Comment 5

[André Reinald]

(In reply to Jeff Gilbert [:jgilbert] from comment #2)
> I think this is the relevant line?
> 
> Apr  3 13:01:09 jgilberts-Mac-mini.local sandboxd[257] ([82440]):
> plugin-container(82440) deny iokit-open AMDRadeonX3000_AMDAccelDevice

I also think so. I could not reproduce with the patch applied.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

Related to bug 1151492?

Comment 7

[Kelsey Gilbert [:jgilbert]]

(In reply to André Reinald from comment #4)
> Created attachment 8588367 [details] [diff] [review]
> bugzilla-1150765-1.patch
> 
> Jeff, can you please confirm the above patch solves your issue?

It does not. With the patch, I get:

> Apr  6 13:59:10 jgilberts-Mac-mini kernel[0]: Sandbox: plugin-container(84957) deny iokit-open AMDRadeonX3000_AMDAccelSharedUserClient

I poked around the internet fruitlessly trying to find docs on sandboxing rules, but I found this:
http://opensource.apple.com/source/WebKit2/WebKit2-7537.71/Resources/PlugInSandboxProfiles/com.apple.WebKit.plugin-common.sb?txt

By process of elimination, only one line seems necessary:

>            (iokit-connection "IOAccelerator")

Comment 8

[Kelsey Gilbert [:jgilbert]]

Attachment: 0005-Allow-OpenGL-in-sandbox.patch

This works for me.

Comment 9

[André Reinald]

(In reply to Jeff Gilbert [:jgilbert] from comment #7)
> By process of elimination, only one line seems necessary:
> >            (iokit-connection "IOAccelerator")

Jeff, unfortunately, if you skimmed through bug 1083344, you'll realize that for lack of documentation from Apple, getting correct rules has been a purely empirical process.

There were 2 approaches:
1. either add "allow" rules until to suppress all "denied" in the log (most of the ruleset),
2. take whole chapters from existing rules files (we did that for printing stuff).

I clearly prefer the 2nd, and your finding (iokit-connection "IOAccelerator") falls in that one.
Though I'd prefer we take the whole block after the ";; Graphics..." from com.apple.WebKit.plugin-common.sb

I'll write the patch in the afternoon or tomorrow.

Comment 10

[Kelsey Gilbert [:jgilbert]]

For reference, I did manage to find a reference to IOAccelerator on [1], in the Graphics section.

[1]: https://developer.apple.com/library/mac/documentation/DeviceDrivers/Conceptual/IOKitFundamentals/Families_Ref/Families_Ref.html#//apple_ref/doc/uid/TP0000021-BABIGAIA

Comment 11

[André Reinald]

Jeff, what system version are you running on?

I run 10.9.5 and (allow iokit-open (iokit-connection "IOAccelerator")) is rejected -> the content process crashes, and I have the rules printed on stdout, which means "syntax error somewhere".

Other issue: I have nvidia graphics, which may behave differently.

Comment 12

[André Reinald]

Attachment: bugzilla-1150765-2.patch

I had to comment the (iokit-connection ...) on 10.9.5, or else the content process crashes.

Comment 13

[André Reinald]

Also noteworthy, even with the sandbox disabled, I still get:
OpenGL version detected: 210
OpenGL vendor: Intel Inc.
OpenGL renderer: Intel HD Graphics 4000 OpenGL Engine
OpenGL vendor ('Intel Inc.') recognized as: Intel

Though my MacBook Pro also has an nvidia card built in.

Comment 14

[André Reinald]

(In reply to André Reinald from comment #12)
> Created attachment 8590017 [details] [diff] [review]
> bugzilla-1150765-2.patch
> 
> I had to comment the (iokit-connection ...) on 10.9.5, or else the content
> process crashes.

Inexact: the first opened window (at launch) works fine. But closing and reopening results in a crash with a "rules dump" on stdout.

Comment 15

[André Reinald]

(In reply to Jeff Gilbert [:jgilbert] from comment #10)
> For reference, I did manage to find a reference to IOAccelerator on [1], in
> the Graphics section.
> 
> [1]:
> https://developer.apple.com/library/mac/documentation/DeviceDrivers/
> Conceptual/IOKitFundamentals/Families_Ref/Families_Ref.html#//apple_ref/doc/
> uid/TP0000021-BABIGAIA

Though the document makes no mention of sandboxing. The name match may be a coincidence.

Comment 16

[Kelsey Gilbert [:jgilbert]]

(In reply to André Reinald from comment #15)
> (In reply to Jeff Gilbert [:jgilbert] from comment #10)
> > For reference, I did manage to find a reference to IOAccelerator on [1], in
> > the Graphics section.
> > 
> > [1]:
> > https://developer.apple.com/library/mac/documentation/DeviceDrivers/
> > Conceptual/IOKitFundamentals/Families_Ref/Families_Ref.html#//apple_ref/doc/
> > uid/TP0000021-BABIGAIA
> 
> Though the document makes no mention of sandboxing. The name match may be a
> coincidence.

It specifically is about iokit, which is the same thing we restrict with sandboxing. Exact match may be coincidental, but I don't think it's a conceptual coincidence.

Comment 17

[André Reinald]

Attachment: bugzilla-1150765-3.patch

Jeff, can you please check if this patch fixes the issue on your machine?

Comment 18

[André Reinald]

I'm also curious why, even when the sandbox is disabled, on my machine, the Intel graphics card is used. Shouldn't the nvidia card be used instead?

Comment 19

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8590027 [details] [diff] [review]
bugzilla-1150765-3.patch

Review of attachment 8590027 [details] [diff] [review]:
-----------------------------------------------------------------

Works for me!

Comment 20

[André Reinald]

Comment on attachment 8590027 [details] [diff] [review]
bugzilla-1150765-3.patch

Steven, I changed the rules of the content process sandbox related to accelerated graphics. Adding some "allow", and grouping them under an explicit comment. It fixes Jeff's issues, and will improve readability.

Comment 21

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8590027 [details] [diff] [review]
bugzilla-1150765-3.patch

This looks fine to me.

At some point we should look more closely at these sandbox exceptions, and see if there's a reasonable way to avoid them -- for example by proxying the initialization of OpenGL hardware acceleration through the chrome process.  But I expect that will be a lot of work.

Comment 22

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #6)

> Related to bug 1151492?

Probably not.

Comment 23

[André Reinald]

(In reply to Steven Michaud from comment #21)
> At some point we should look more closely at these sandbox exceptions, and
> see if there's a reasonable way to avoid them -- for example by proxying the
> initialization of OpenGL hardware acceleration through the chrome process. 
> But I expect that will be a lot of work.

I'm not sure it would make sense to move those away from the content process, as it really makes sense to have this API used from there.

Thanks for the review, asking for check-in!

Comment 24

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #18)

> I'm also curious why, even when the sandbox is disabled, on my
> machine, the Intel graphics card is used. Shouldn't the nvidia card
> be used instead?

It's the OS that chooses which card gets used, and as best I can tell
the decision is pretty arbitrary.  But there's a nice utility called
gfxCardStatus (https://gfx.io/) that you can use to force the OS to
decide one way or the other.

Comment 25

[Carsten Book [:Tomcat]]

(In reply to André Reinald from comment #23)
> (In reply to Steven Michaud from comment #21)
> > At some point we should look more closely at these sandbox exceptions, and
> > see if there's a reasonable way to avoid them -- for example by proxying the
> > initialization of OpenGL hardware acceleration through the chrome process. 
> > But I expect that will be a lot of work.
> 
> I'm not sure it would make sense to move those away from the content
> process, as it really makes sense to have this API used from there.
> 
> Thanks for the review, asking for check-in!

can we get a try run for this change ? thanks!

Comment 26

[André Reinald]

In progress here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d9d7e1bee33

Comment 27

[André Reinald]

Mistake, tests were run on 10.6 where sandbox is disabled. New try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f5dcd056920

Comment 28

[André Reinald]

Tests all green.

Comment 29

[Carsten Book [:Tomcat]]

Hi, this failed to apply to mozilla-inbound:

patching file security/sandbox/mac/Sandbox.mm
Hunk #3 FAILED at 280
1 out of 3 hunks FAILED -- saving rejects to file security/sandbox/mac/Sandbox.mm.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh bugzilla-1150765-3.patch

could you take a look, thanks!

Comment 30

[André Reinald]

Attachment: bugzilla-1150765-4.patch

Minor changes were made in the same file (getting rid of XUL dependency).
Rebased the previous patch to current m-c.
Carrying r+.

Comment 31

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a82f563aec94

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a82f563aec94