If the mmap'd cache file gets evicted and the device is locked, then WebContent won't be able to bring the file back in under class A, causing SIGBUS.
<rdar://problem/23676252>
Created attachment 301365 [details] patch Attaching patch by Antti.
Comment on attachment 301365 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=301365&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.cpp:149 > + os_log(OS_LOG_DEFAULT, "protectionClass=%d", attrBuffer.protectionClass); I'll upload a version without the stray os_log.
Comment on attachment 301365 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=301365&action=review r=me > Source/WebKit2/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=168241 Please include radar url too. > Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.cpp:134 > +bool isSafeToUseSharedMemory(const String& path) I would have called this simply "canUseSharedMemoryForPath()". > Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.cpp:149 > + os_log(OS_LOG_DEFAULT, "protectionClass=%d", attrBuffer.protectionClass); I don't think we should call os_log() directly. We have the RELEASE_LOG macro for this.
(In reply to comment #3) > Comment on attachment 301365 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=301365&action=review > > > Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.cpp:149 > > + os_log(OS_LOG_DEFAULT, "protectionClass=%d", attrBuffer.protectionClass); > > I'll upload a version without the stray os_log. You cannot review your own patch :P r=me though.
Created attachment 301372 [details] patch
Comment on attachment 301372 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=301372&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.cpp:156 > + RELEASE_LOG(Network, "Disallowing use of shared mapped memory due to container protection class %d", attrBuffer.protectionClass); %u ?
Comment on attachment 301372 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=301372&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.cpp:148 > + if (error) We may want to RELEASE_LOG_ERROR() in this case too.
Created attachment 301375 [details] patch
Comment on attachment 301375 [details] patch lgtm
Created attachment 301381 [details] patch Cache::store needs canUseSharedMemoryForBodyData() test too for the case where we switch an existing resource to use mapped file backing after it has been written to disk.
Comment on attachment 301381 [details] patch lgtm.
lgtm here too, I've been trying out the patch locally and it looks stable.
https://trac.webkit.org/changeset/212244