1695866 - Warning: 'kIOSurfaceIsGlobal' is deprecated: first deprecated in macOS 10.11 - Global surfaces are insecure [-Wdeprecated-declarations]

Status: NEW

(Core :: Widget: Cocoa, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1] (away from Feb 12th - Feb 22nd)]

When compiling Firefox I can see the following deprecation and insecure compiler output:

15:51.05 /Users/henrik/code/gecko/dom/media/platforms/apple/AppleVTDecoder.cpp:624:34: warning: 'kIOSurfaceIsGlobal' is deprecated: first deprecated in macOS 10.11 - Global surfaces are insecure [-Wdeprecated-declarations]
15:51.05   const void* IOSurfaceKeys[] = {kIOSurfaceIsGlobal};
15:51.05                                  ^
15:51.05 /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/IOSurface.framework/Headers/IOSurfaceRef.h:101:26: note: 'kIOSurfaceIsGlobal' has been explicitly marked deprecated here
15:51.05 extern const CFStringRef kIOSurfaceIsGlobal                                 API_DEPRECATED("Global surfaces are insecure",macos(10.6,10.11), ios(11.0,11.0), watchos(4.0,4.0), tvos(11.0,11.0));

A query on searchfox reveales usages in DOM and GFX:
https://searchfox.org/mozilla-central/search?q=kIOSurfaceIsGlobal&path=

Markus, is that something we should be worried about, or what does insecure actually mean in this context?

Comment 1

[Markus Stange [:mstange]]

I think "insecure" means that other processes that are running on the system can read and write to our surfaces, so the can see and change the pixels that we put on the screen. So fixing this is an opportunity to strengthen our sandbox.
Furthermore, there is a non-zero chance that macOS might eventually remove support for global IOSurfaces entirely.

It might not be very easy to fix; I think we'll need to send mach_ports for the surfaces, and I'm not sure how straightforward that is with our IPC mechanism.

Comment 2

[Intermittent Failures Robot]

1 failures in 3289 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1

Platform and build breakdown:

• osx-cross: 1
  • debug: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1695866&startday=2022-04-25&endday=2022-05-01&tree=all

Comment 3

[Markus Stange [:mstange]]

We should try to change the places which send an integer to send a mozilla::UniqueMachSendRight(IOSurfaceCreateMachPort(surface)) instead.

Comment 4

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 1: Change SurfaceDescriptorMacIOSurface to use IOSurfacePort.

This fails to compile because it causes SurfaceDescriptor to implcitly
delete its copy constructor, which is required by type declarations like
Maybe<SurfaceDescriptor>. In theory, Maybe should work correctly with
move types, but that's not happening here. Example of compile errors:

error: call to implicitly-deleted copy constructor of 'NonConstT' (aka 'mozilla::layers::SurfaceDescriptor')
note: copy constructor is implicitly deleted because 'SurfaceDescriptor' has a user-declared move constructor
MOZ_IMPLICIT SurfaceDescriptor(SurfaceDescriptor&& aOther);

Comment 5

[Brad Werth [:bradwerth]]

Attachment: Bug 1695866 Part 2: Make Maybe conditionally compile some methods for copyable types.

Comment 6

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 3: Unify the IPC ParamTraits::Write methods for const ref and rvalue params.

Comment 7

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866: Claude 2d changes.

Comment 8

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866: Claude layers changes.

Comment 9

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 7: Update GL and layers OpenGL classes for move-only SurfaceDescriptors.

Comment 10

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 8: Update canvas classes for move-only SurfaceDescriptor.

Comment 11

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 9: Update SVG classes for move-only SurfaceDescriptor.

Comment 12

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 11: Update VR classes for move-only SurfaceDescriptor.

Comment 13

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 10: Update webgpu classes for move-only SurfaceDescriptor.

Comment 14

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 4: Split IOSurfacePort.h into a cpp definition.

This simplifies some compilation challenges but is otherwise a
non-functional change. It keeps some Apple headers out of IOSurfacePort.h.

Comment 15

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 2: Update media actors for move-only SurfaceDescriptor.

Comment 16

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 6: Update layers classes for move-only SurfaceDescriptor.

This is most of the Texture and TextureHost classes that use a
SurfaceDescriptor, as well as some of the gfx Bridge actors.

Comment 17

[Brad Werth [:bradwerth]]

The patches are still a bit of a mess. I'm going to split out the patches into individual blocker Bugs to make each piece easier to reason about -- and to review.

Comment 18

[Phabricator Automation]

Comment on attachment 9544092 [details]
WIP: Bug 1695866 Part 4: Split IOSurfacePort.h into a cpp definition.

Revision D282739 was moved to bug 2016533. Setting attachment 9544092 [details] to obsolete.

Comment 19

[Phabricator Automation]

Comment on attachment 9542452 [details]
WIP: Bug 1695866 Part 3: Unify the IPC ParamTraits::Write methods for const ref and rvalue params.

Revision D281835 was moved to bug 2016562. Setting attachment 9542452 [details] to obsolete.

Comment 20

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 3: Update layers texture client for move-only SurfaceDescriptors.

Comment 21

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 4: Update layers ipc actors for move-only SurfaceDescriptors.

Comment 22

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 5: Update layers composite texture hosts for move-only SurfaceDescriptors.

Comment 23

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1695866 Part 6: Update layers for move-only SurfaceDescriptors.