From 08dddedfd37e16625a16eb23d8823ad604cd872a Mon Sep 17 00:00:00 2001 From: "adamk@chromium.org" Date: Fri, 15 Aug 2014 15:47:51 +0000 Subject: Revert of Fix Mac sandbox meta data access (reland) (patchset #2 of https://codereview.chromium.org/469383002/) Reason for revert: Caused GPU crashes on Mac10.6 Blink layout tests running with component=shared_library. Original issue's description: > Fix Mac sandbox meta data access (reland) > > We currently allow all metadata access due to > https://codereview.chromium.org/10539009/ made the for loop comparison > in Sandbox::AllowMetadataForPath() always false, when we actually only > want to allow access to the path and all its parent path until root. > > Turn the for loop to a do/while loop instead as it's a better fit, also > add a test case for Sandbox::AllowMetadataForPath(). > > We also need file read meta data access to all the .dylibs we linked to. > > It should only affect component builds on OS X 10.6 and utility process > as no other process is using this mechanism. > > BUG=403801 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289738 TBR=avi@chromium.org,jeremy@chromium.org,thakis@chromium.org,jiangj@opera.com NOTREECHECKS=true NOTRY=true BUG=403801 Review URL: https://codereview.chromium.org/470693004 Cr-Commit-Position: refs/heads/master@{#289877} git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289877 0039d316-1c4b-4281-b951-d872f2087c98 --- content/common/sandbox_mac.h | 1 - content/common/sandbox_mac.mm | 26 ++++-------------------- content/common/sandbox_mac_diraccess_unittest.mm | 13 +----------- 3 files changed, 5 insertions(+), 35 deletions(-) diff --git a/content/common/sandbox_mac.h b/content/common/sandbox_mac.h index f978f02..557b4fb7 100644 --- a/content/common/sandbox_mac.h +++ b/content/common/sandbox_mac.h @@ -160,7 +160,6 @@ class CONTENT_EXPORT Sandbox { FRIEND_TEST_ALL_PREFIXES(MacDirAccessSandboxTest, StringEscape); FRIEND_TEST_ALL_PREFIXES(MacDirAccessSandboxTest, RegexEscape); FRIEND_TEST_ALL_PREFIXES(MacDirAccessSandboxTest, SandboxAccess); - FRIEND_TEST_ALL_PREFIXES(MacDirAccessSandboxTest, AllowMetadataForPath); DISALLOW_IMPLICIT_CONSTRUCTORS(Sandbox); }; diff --git a/content/common/sandbox_mac.mm b/content/common/sandbox_mac.mm index 8b4dfe3..c7c1265 100644 --- a/content/common/sandbox_mac.mm +++ b/content/common/sandbox_mac.mm @@ -114,14 +114,12 @@ NSString* Sandbox::AllowMetadataForPath(const base::FilePath& allowed_path) { // Collect a list of all parent directories. base::FilePath last_path = allowed_path; std::vector subpaths; - - base::FilePath path = allowed_path; - do { + for (base::FilePath path = allowed_path; + path.value() != last_path.value(); + path = path.DirName()) { subpaths.push_back(path); - last_path = path; - path = path.DirName(); - } while (path.value() != last_path.value()); + } // Iterate through all parents and allow stat() on them explicitly. NSString* sandbox_command = @"(allow file-read-metadata "; @@ -572,22 +570,6 @@ bool Sandbox::EnableSandbox(int sandbox_type, [base::mac::MainBundle() executablePath]); NSString* sandbox_command = AllowMetadataForPath( GetCanonicalSandboxPath(bundle_executable)); - - // In addition to the workaround above, for OS X <= 10.6 we also need to - // allow reading file metadata for all the dylibs under the same directory - // containing the Chrome bundle. It requires to go 5 levels up from main - // bundle path because the main bundle here is the Helper.app bundle. - base::FilePath product_path = base::mac::MainBundlePath() - .DirName() - .DirName() - .DirName() - .DirName() - .DirName(); - sandbox_command = [sandbox_command - stringByAppendingFormat: - @"(allow file-read-metadata (regex #\"^%@/.*\\.dylib$\"))", - base::mac::FilePathToNSString(product_path)]; - substitutions["COMPONENT_BUILD_WORKAROUND"] = SandboxSubstring(base::SysNSStringToUTF8(sandbox_command)); } diff --git a/content/common/sandbox_mac_diraccess_unittest.mm b/content/common/sandbox_mac_diraccess_unittest.mm index 7fb7d45..06a5442 100644 --- a/content/common/sandbox_mac_diraccess_unittest.mm +++ b/content/common/sandbox_mac_diraccess_unittest.mm @@ -127,6 +127,7 @@ TEST_F(MacDirAccessSandboxTest, RegexEscape) { std::string out; EXPECT_TRUE(Sandbox::QuoteStringForRegex(in_utf8, &out)); EXPECT_EQ(expected, out); + } } @@ -177,18 +178,6 @@ TEST_F(MacDirAccessSandboxTest, SandboxAccess) { } } -TEST_F(MacDirAccessSandboxTest, AllowMetadataForPath) { - { - std::string expected( - "(allow file-read-metadata (literal \"/\")(literal \"/System\")" - "(literal \"/System/Library\")" - "(literal \"/System/Library/Frameworks\"))"); - NSString* sandbox_command = Sandbox::AllowMetadataForPath( - base::FilePath("/System/Library/Frameworks")); - EXPECT_EQ(base::SysNSStringToUTF8(sandbox_command), expected); - } -} - MULTIPROCESS_TEST_MAIN(mac_sandbox_path_access) { char *sandbox_allowed_dir = getenv(kSandboxAccessPathKey); if (!sandbox_allowed_dir) -- cgit v1.1