Attachment 368502 Details for Bug 197390 – Patch

[patch] Patch

bug-197390-20190429152922.patch (text/plain), 13.18 KB, created by Sihui Liu on 2019-04-29 15:29:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-04-29 15:29:23 PDT

Size: 13.18 KB

patch

obsolete

>Subversion Revision: 244735
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 475cdd28b74cf6c19251851e75de9c279aebd116..bc2ac66c35c8e84171a70f6c193f7717c069576c 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-04-29  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Add assertion to check whether shm files have maximum FileProtection of CompleteUnlessOpen
>+        https://bugs.webkit.org/show_bug.cgi?id=197390
>+        <rdar://problem/42685773>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/FileSystem.cpp:
>+        (WTF::FileSystemImpl::isSafeToUseMemoryMapForPath):
>+        (WTF::FileSystemImpl::makeSafeToUseMemoryMapForPath):
>+        * wtf/FileSystem.h:
>+        * wtf/cocoa/FileSystemCocoa.mm:
>+        (WTF::FileSystemImpl::isSafeToUseMemoryMapForPath):
>+        (WTF::FileSystemImpl::makeSafeToUseMemoryMapForPath):
>+
> 2019-04-26  Don Olmstead  <don.olmstead@sony.com>
> 
>         Add WTF::findIgnoringASCIICaseWithoutLength to replace strcasestr
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d3c94590602595c42527d99f8caad2d74a933749..7869364ab69a81acbc016383004563148db41b11 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-04-29  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Add assertion to check whether shm files have maximum FileProtection of CompleteUnlessOpen
>+        https://bugs.webkit.org/show_bug.cgi?id=197390
>+        <rdar://problem/42685773>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We have seen crashes about accessing database files after device is locked. We are suspecting this is because 
>+        shm files have wrong data protection class, but shm files should not have Complete class protection when it 
>+        is created. It is likely the protection class is changed later. Add an assertion to verify our guess. If the 
>+        crash signature changes after this patch, we probably need to change database implementation. If it is not, we
>+        have other problem than data protection.
>+
>+        * platform/sql/SQLiteDatabase.cpp:
>+        (WebCore::SQLiteDatabase::open):
>+
> 2019-04-25  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         REGRESSION(r244635): [GTK] Wrong background color used in non-dark mode
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 00b570606c3997e50d17e73e5972ca4bb24b902e..cca4da4a07d443af995410c457e1938549adac3e 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-04-29  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Add assertion to check whether shm files have maximum FileProtection of CompleteUnlessOpen
>+        https://bugs.webkit.org/show_bug.cgi?id=197390
>+        <rdar://problem/42685773>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Move data protection check to WebCore so it can be applied to database files.
>+
>+        * NetworkProcess/cache/NetworkCacheBlobStorage.cpp:
>+        (WebKit::NetworkCache::BlobStorage::add):
>+        * NetworkProcess/cache/NetworkCacheFileSystem.cpp:
>+        (WebKit::NetworkCache::makeSafeToUseMemoryMapForPath): Deleted.
>+        * NetworkProcess/cache/NetworkCacheFileSystem.h:
>+        * NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm: Removed.
>+        * SourcesCocoa.txt:
>+        * UIProcess/API/APIContentRuleListStore.cpp:
>+        (API::openAndMapOrCopyContentRuleList):
>+        (API::compiledToFile):
>+
> 2019-04-29  Alex Christensen  <achristensen@webkit.org>
> 
>         Build fix.
>diff --git a/Source/WTF/wtf/FileSystem.cpp b/Source/WTF/wtf/FileSystem.cpp
>index 06dec7d293767ba542841799a9fbbfa1c43f5b67..ba49b1e77a7a41749ab40235304e9f5bf4daffa8 100644
>--- a/Source/WTF/wtf/FileSystem.cpp
>+++ b/Source/WTF/wtf/FileSystem.cpp
>@@ -363,5 +363,16 @@ bool fileIsDirectory(const String& path, ShouldFollowSymbolicLinks shouldFollowS
>     return metadata.value().type == FileMetadata::Type::Directory;
> }
> 
>+#if !PLATFORM(IOS_FAMILY)
>+bool isSafeToUseMemoryMapForPath(const String&)
>+{
>+    return true;
>+}
>+
>+void makeSafeToUseMemoryMapForPath(const String&)
>+{
>+}
>+#endif
>+
> } // namespace FileSystemImpl
> } // namespace WTF
>diff --git a/Source/WTF/wtf/FileSystem.h b/Source/WTF/wtf/FileSystem.h
>index 903962ad46d27bb783222d391c01bc871d3f113e..0ac8961fd84b6f777250daaf8b6ad4ba7f064cc3 100644
>--- a/Source/WTF/wtf/FileSystem.h
>+++ b/Source/WTF/wtf/FileSystem.h
>@@ -187,6 +187,9 @@ WTF_EXPORT_PRIVATE bool deleteNonEmptyDirectory(const String&);
> 
> WTF_EXPORT_PRIVATE String realPath(const String&);
> 
>+WTF_EXPORT_PRIVATE bool isSafeToUseMemoryMapForPath(const String&);
>+WTF_EXPORT_PRIVATE void makeSafeToUseMemoryMapForPath(const String&);
>+
> class MappedFileData {
> public:
>     MappedFileData() { }
>diff --git a/Source/WTF/wtf/cocoa/FileSystemCocoa.mm b/Source/WTF/wtf/cocoa/FileSystemCocoa.mm
>index e122200692b99a41d8ece9b6e6c2e8f112058c65..257bca587e55bdef4a8020f8f5766642a2161ba2 100644
>--- a/Source/WTF/wtf/cocoa/FileSystemCocoa.mm
>+++ b/Source/WTF/wtf/cocoa/FileSystemCocoa.mm
>@@ -139,5 +139,33 @@ bool deleteNonEmptyDirectory(const String& path)
>     return [[NSFileManager defaultManager] removeItemAtPath:path error:nil];
> }
> 
>+#if PLATFORM(IOS_FAMILY)
>+bool isSafeToUseMemoryMapForPath(const String& path)
>+{
>+    NSError *error = nil;
>+    NSDictionary<NSFileAttributeKey, id> *attributes = [[NSFileManager defaultManager] attributesOfItemAtPath:path error:&error];
>+    if (error) {
>+        LOG_ERROR("Unable to get path protection class");
>+        return false;
>+    }
>+    if ([[attributes objectForKey:NSFileProtectionKey] isEqualToString:NSFileProtectionComplete]) {
>+        LOG_ERROR("Path protection class is NSFileProtectionComplete, so it is not safe to use memory map");
>+        return false;
>+    }
>+    return true;
>+}
>+
>+void makeSafeToUseMemoryMapForPath(const String& path)
>+{
>+    if (isSafeToUseMemoryMapForPath(path))
>+        return;
>+    
>+    NSError *error = nil;
>+    BOOL success = [[NSFileManager defaultManager] setAttributes:@{ NSFileProtectionKey: NSFileProtectionCompleteUnlessOpen } ofItemAtPath:path error:&error];
>+    ASSERT(!error);
>+    ASSERT_UNUSED(success, success);
>+}
>+#endif
>+
> } // namespace FileSystemImpl
> } // namespace WTF
>diff --git a/Source/WebCore/platform/sql/SQLiteDatabase.cpp b/Source/WebCore/platform/sql/SQLiteDatabase.cpp
>index 3ff57ed2014ddb11fe178abe462a46fb8c1ca714..579646aa3ac52e5b178093b6081389227e950edc 100644
>--- a/Source/WebCore/platform/sql/SQLiteDatabase.cpp
>+++ b/Source/WebCore/platform/sql/SQLiteDatabase.cpp
>@@ -149,6 +149,10 @@ bool SQLiteDatabase::open(const String& filename, OpenMode openMode)
>     if (openMode != OpenMode::ReadOnly)
>         useWALJournalMode();
> 
>+    String shmFileName = makeString(filename, "-shm"_s);
>+    if (FileSystem::fileExists(shmFileName))
>+        RELEASE_ASSERT(FileSystem::isSafeToUseMemoryMapForPath(shmFileName));
>+
>     return isOpen();
> }
> 
>diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp b/Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp
>index bf9ff23daaac083a8eb7caace7e841a38635dc62..2d4d33b940d654d9e60e80d9f4f3a5fad75714dc 100644
>--- a/Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp
>+++ b/Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp
>@@ -94,7 +94,7 @@ BlobStorage::Blob BlobStorage::add(const String& path, const Data& data)
>         return { data, hash };
> 
>     String blobPathString = blobPathForHash(hash);
>-    makeSafeToUseMemoryMapForPath(blobPathString);
>+    FileSystem::makeSafeToUseMemoryMapForPath(blobPathString);
>     
>     auto blobPath = FileSystem::fileSystemRepresentation(blobPathString);
>     auto linkPath = FileSystem::fileSystemRepresentation(path);
>diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp b/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
>index 1cd67ac670844e3ab072fa38603a9f881ad47c81..75a26654ec06afa52bdb12ab26320c7e63b2561b 100644
>--- a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
>+++ b/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
>@@ -145,11 +145,5 @@ void updateFileModificationTimeIfNeeded(const String& path)
> #endif
> }
> 
>-#if !PLATFORM(IOS_FAMILY)
>-void makeSafeToUseMemoryMapForPath(const String&)
>-{
>-}
>-#endif
>-
> }
> }
>diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h b/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h
>index 4470923f7ed47dcd051e1960527e472805925470..7a5f8cc4fe38a9561ce1c8b61c10c7479ca8742f 100644
>--- a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h
>+++ b/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h
>@@ -42,7 +42,5 @@ struct FileTimes {
> FileTimes fileTimes(const String& path);
> void updateFileModificationTimeIfNeeded(const String& path);
> 
>-void makeSafeToUseMemoryMapForPath(const String&);
>-
> }
> }
>diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm b/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm
>deleted file mode 100644
>index 812196d9f5228e42ed0de18e70a95d1fe15249e3..0000000000000000000000000000000000000000
>--- a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm
>+++ /dev/null
>@@ -1,64 +0,0 @@
>-/*
>- * Copyright (C) 2019 Apple Inc. All rights reserved.
>- *
>- * Redistribution and use in source and binary forms, with or without
>- * modification, are permitted provided that the following conditions
>- * are met:
>- * 1. Redistributions of source code must retain the above copyright
>- *    notice, this list of conditions and the following disclaimer.
>- * 2. Redistributions in binary form must reproduce the above copyright
>- *    notice, this list of conditions and the following disclaimer in the
>- *    documentation and/or other materials provided with the distribution.
>- *
>- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
>- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
>- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>- * THE POSSIBILITY OF SUCH DAMAGE.
>- */
>-
>-#include "config.h"
>-#include "NetworkCacheFileSystem.h"
>-
>-#include "Logging.h"
>-#include <wtf/Assertions.h>
>-
>-namespace WebKit {
>-namespace NetworkCache {
>-
>-#if PLATFORM(IOS_FAMILY)
>-static bool isSafeToUseMemoryMapForPath(const String& path)
>-{
>-    NSError *error = nil;
>-    NSDictionary<NSFileAttributeKey, id> *attributes = [[NSFileManager defaultManager] attributesOfItemAtPath:path error:&error];
>-    if (error) {
>-        RELEASE_LOG_ERROR(Network, "Unable to get cache directory protection class, disabling use of shared mapped memory");
>-        return false;
>-    }
>-    if ([[attributes objectForKey:NSFileProtectionKey] isEqualToString:NSFileProtectionComplete]) {
>-        RELEASE_LOG(Network, "Disallowing use of shared mapped memory due to container protection class NSFileProtectionComplete");
>-        return false;
>-    }
>-    return true;
>-}
>-
>-void makeSafeToUseMemoryMapForPath(const String& path)
>-{
>-    if (isSafeToUseMemoryMapForPath(path))
>-        return;
>-
>-    NSError *error = nil;
>-    BOOL success = [[NSFileManager defaultManager] setAttributes:@{ NSFileProtectionKey: NSFileProtectionCompleteUnlessOpen } ofItemAtPath:path error:&error];
>-    ASSERT(!error);
>-    ASSERT_UNUSED(success, success);
>-}
>-#endif // PLATFORM(IOS_FAMILY)
>-
>-}
>-}
>diff --git a/Source/WebKit/SourcesCocoa.txt b/Source/WebKit/SourcesCocoa.txt
>index b404e3abeb4136681468fae814c1b4208ee162df..fd38da331b24e0f57b119892b455173b26d4d5a0 100644
>--- a/Source/WebKit/SourcesCocoa.txt
>+++ b/Source/WebKit/SourcesCocoa.txt
>@@ -22,7 +22,6 @@
> // THE POSSIBILITY OF SUCH DAMAGE.
> 
> NetworkProcess/cache/NetworkCacheDataCocoa.mm
>-NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm
> NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm
> 
> NetworkProcess/cocoa/NetworkActivityTrackerCocoa.mm
>diff --git a/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp b/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp
>index 8249aa13b74f8b876a26fc1acd138728745de880..030657c0fae09c612f3274ad8c661f27175bd895 100644
>--- a/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp
>+++ b/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp
>@@ -208,7 +208,7 @@ struct MappedData {
> 
> static Optional<MappedData> openAndMapOrCopyContentRuleList(const WTF::String& path)
> {
>-    WebKit::NetworkCache::makeSafeToUseMemoryMapForPath(path);
>+    FileSystem::makeSafeToUseMemoryMapForPath(path);
>     WebKit::NetworkCache::Data fileData = mapFile(fileSystemRepresentation(path).data());
>     if (fileData.isNull())
>         return WTF::nullopt;
>@@ -386,7 +386,7 @@ static Expected<MappedData, std::error_code> compiledToFile(WTF::String&& json,
>         return makeUnexpected(ContentRuleListStore::Error::CompileFailed);
>     }
>     
>-    makeSafeToUseMemoryMapForPath(finalFilePath);
>+    FileSystem::makeSafeToUseMemoryMapForPath(finalFilePath);
>     
>     return {{ WTFMove(metaData), WTFMove(mappedData) }};
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 197390: 368502