After collecting several weeks of telemetry, we can now switch from allowing (with reports) to denying access (with reports).
<rdar://problem/61403216>
Created attachment 395717 [details] Patch
Tested on macOS and iOS.
Comment on attachment 395717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395717&action=review > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:619 > + (iokit-registry-entry-class "IOFramebufferSharedUserClient") This seems to be allowed earlier in the sandbox, is that expected? > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:727 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED > 101500 > + (with report) > +#endif Would it make sense to also have telemetry-backtrace here?
Comment on attachment 395717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395717&action=review R=me. > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:603 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED < 101500 Should this be '< 101600'? > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:612 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500 Should this be '>= 101600'?
Comment on attachment 395717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395717&action=review I think I'll be more conservative and only impact 10.16, since we might want to back-port some of this to 10.15 and I don't want to introduce any issues. > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:123 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED < 101600 This should have been < 101500 >> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:603 >> +#if __MAC_OS_X_VERSION_MIN_REQUIRED < 101500 > > Should this be '< 101600'? I don't think so. We did not see these being used on 10.15, but could not collect telemetry on older systems. So this leaves older systems alone, and blocks on the systems we could measure. >> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:619 >> + (iokit-registry-entry-class "IOFramebufferSharedUserClient") > > This seems to be allowed earlier in the sandbox, is that expected? Ah! My earlier condition was wrong. I'll fix that. >> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:727 >> +#endif > > Would it make sense to also have telemetry-backtrace here? Sure -- I'll change that.
Created attachment 396113 [details] Patch for landing
Committed r259891: <https://trac.webkit.org/changeset/259891> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396113 [details].
Reverted r259891 for reason: Causes significant iOS MotionMark regression Committed r260120: <https://trac.webkit.org/changeset/260120>
MotionMark on iPad Pro (original sandbox): 894.52 +/- 9.44% MotionMark on iPad Pro (revised sandbox): 885.32 +- 10.50%
Created attachment 396579 [details] Patch
Comment on attachment 396579 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396579&action=review Great! R=me, given the MotionMark regression is fixed :) > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:222 > "AGXCommandQueue" > "AGXDevice" > - "AGXSharedUserClient" > + "AGXSharedUserClient")) Do we need all of these?
Let's use this radar for the macOS bits. We'll do a separate one for the iOS changes.
Created attachment 396683 [details] Patch
Comment on attachment 396683 [details] Patch R=me.
Committed r260221: <https://trac.webkit.org/changeset/260221> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396683 [details].