RESOLVED FIXED 215869
[macOS] Stop logging sandbox violations of com.apple.CoreDisplay.Notification
https://bugs.webkit.org/show_bug.cgi?id=215869
Summary [macOS] Stop logging sandbox violations of com.apple.CoreDisplay.Notification
Per Arne Vollan
Reported 2020-08-26 15:13:56 PDT
On macOS, stop logging mach-lookup sandbox violations of com.apple.CoreDisplay.Notification for performance reasons.
Attachments
Patch (1.42 KB, patch)
2020-08-26 15:16 PDT, Per Arne Vollan
no flags
Patch (1.48 KB, patch)
2020-08-26 16:29 PDT, Per Arne Vollan
no flags
Per Arne Vollan
Comment 1 2020-08-26 15:16:55 PDT
Per Arne Vollan
Comment 2 2020-08-26 15:17:37 PDT
Per Arne Vollan
Comment 3 2020-08-26 16:29:10 PDT
Darin Adler
Comment 4 2020-08-26 16:33:37 PDT
Comment on attachment 407353 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407353&action=review > Source/WebKit/ChangeLog:8 > + On macOS, stop logging mach-lookup sandbox violations of com.apple.CoreDisplay.Notification for performance reasons. This makes it sound like it’s something already denied, and this patch simply changes it to no-log. > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:771 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000 > +(deny mach-lookup (with no-log) > + (global-name "com.apple.CoreDisplay.Notification)) > +#endif But this is a new deny rule, not changing an existing one to no-log mode. What am I missing?
Per Arne Vollan
Comment 5 2020-08-26 16:57:19 PDT
(In reply to Darin Adler from comment #4) > Comment on attachment 407353 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407353&action=review > > > Source/WebKit/ChangeLog:8 > > + On macOS, stop logging mach-lookup sandbox violations of com.apple.CoreDisplay.Notification for performance reasons. > > This makes it sound like it’s something already denied, and this patch > simply changes it to no-log. > Yes, that is correct. > > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:771 > > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000 > > +(deny mach-lookup (with no-log) > > + (global-name "com.apple.CoreDisplay.Notification)) > > +#endif > > But this is a new deny rule, not changing an existing one to no-log mode. > > What am I missing? Yes, you are right. There is no previous rule for this service, so the default behavior is in effect, where the service is being denied, but with logging enabled. This change is only changing the logging behavior. Thanks for reviewing!
Darin Adler
Comment 6 2020-08-26 17:05:22 PDT
Comment on attachment 407353 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407353&action=review >>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:771 >>> +#endif >> >> But this is a new deny rule, not changing an existing one to no-log mode. >> >> What am I missing? > > Yes, you are right. There is no previous rule for this service, so the default behavior is in effect, where the service is being denied, but with logging enabled. > > This change is only changing the logging behavior. > > Thanks for reviewing! Oh, didn’t realize that "deny" was default for everything. I guess that should have been obvious.
Per Arne Vollan
Comment 7 2020-08-26 17:07:58 PDT
(In reply to Darin Adler from comment #6) > Comment on attachment 407353 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407353&action=review > > >>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:771 > >>> +#endif > >> > >> But this is a new deny rule, not changing an existing one to no-log mode. > >> > >> What am I missing? > > > > Yes, you are right. There is no previous rule for this service, so the default behavior is in effect, where the service is being denied, but with logging enabled. > > > > This change is only changing the logging behavior. > > > > Thanks for reviewing! > > Oh, didn’t realize that "deny" was default for everything. I guess that > should have been obvious. Actually, in some cases, "allow" is the default, but not in this case :) Thanks for reviewing!
EWS
Comment 8 2020-08-26 18:58:37 PDT
Committed r266216: <https://trac.webkit.org/changeset/266216> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407353 [details].
Chris Dumez
Comment 9 2020-08-27 09:53:00 PDT
Comment on attachment 407353 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407353&action=review > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:770 > + (global-name "com.apple.CoreDisplay.Notification)) Missing double quote here :( https://trac.webkit.org/changeset/266238/webkit
Note You need to log in before you can comment on or make changes to this bug.