Bug 1434711 - WebGL causes a crash with the AMDGPU-PRO video driver. r=jld
authorGian-Carlo Pascutto <gcp@mozilla.com>
Thu, 29 Mar 2018 14:04:46 +0200
changeset 411591 6b6efca52e56ef7a616a480168c066802e9d75c7
parent 411590 1e930b5aad582ca9eb0d5edfd60efba4b8f35396
child 411592 549f12a1212d033d34f72c25a5ae0d214dfa1730
push id62121
push usergpascutto@mozilla.com
push dateWed, 04 Apr 2018 08:55:26 +0000
treeherderautoland@6b6efca52e56 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjld
bugs1434711
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1434711 - WebGL causes a crash with the AMDGPU-PRO video driver. r=jld Factor out the ATI-based driver detection code and use this to set specific permissions needed by this driver. In passing, unnest some of the SandboxBroker fallback paths, and make it properly report the operation in all error paths. MozReview-Commit-ID: FrRpicj5NF
security/sandbox/linux/SandboxBrokerClient.cpp
security/sandbox/linux/broker/SandboxBroker.cpp
security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
security/sandbox/linux/launch/SandboxLaunch.cpp
security/sandbox/linux/launch/SandboxLaunch.h
--- a/security/sandbox/linux/SandboxBrokerClient.cpp
+++ b/security/sandbox/linux/SandboxBrokerClient.cpp
@@ -139,18 +139,19 @@ SandboxBrokerClient::DoCall(const Reques
     }
     return resp.mError;
   }
   if (SandboxInfo::Get().Test(SandboxInfo::kVerbose)) {
     // Keep in mind that "rejected" files can include ones that don't
     // actually exist, if it's something that's optional or part of a
     // search path (e.g., shared libraries).  In those cases, this
     // error message is expected.
-    SANDBOX_LOG_ERROR("Failed errno %d op %d flags 0%o path %s",
-                      resp.mError, aReq->mOp, aReq->mFlags, path);
+    SANDBOX_LOG_ERROR("Failed errno %d op %s flags 0%o path %s",
+                      resp.mError, OperationDescription[aReq->mOp],
+                      aReq->mFlags, path);
   }
   if (openedFd >= 0) {
     close(openedFd);
   }
   return resp.mError;
 }
 
 int
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -649,17 +649,18 @@ SandboxBroker::SymlinkPermissions(const 
   // Resolve relative paths, propagate permissions and
   // fail if a symlink is in a writable path. The output is in perms.
   char* result = SandboxBroker::SymlinkPath(mPolicy.get(), pathBufSymlink, NULL, &perms);
   if (result != NULL) {
     free(result);
     // We finished the translation, so we have a usable return in "perms".
     return perms;
   } else {
-    // Empty path means we got a writable dir in the chain.
+    // Empty path means we got a writable dir in the chain or tried
+    // to back out of a link target.
     return 0;
   }
 }
 
 void
 SandboxBroker::ThreadMain(void)
 {
   char threadName[16];
@@ -793,37 +794,37 @@ SandboxBroker::ThreadMain(void)
       pathLen = ConvertRelativePath(pathBuf, sizeof(pathBuf), pathLen);
       perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
 
       // We don't have permissions on the requested dir.
       if (!perms) {
         // Was it a tempdir that we can remap?
         pathLen = RemapTempDirs(pathBuf, sizeof(pathBuf), pathLen);
         perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
-        if (!perms) {
-          // Did we arrive from a symlink in a path that is not writable?
-          // Then try to figure out the original path and see if that is
-          // readable. Work on the original path, this reverses
-            // ConvertRelative above.
-          int symlinkPerms = SymlinkPermissions(recvBuf, first_len);
-          if (symlinkPerms > 0) {
-            perms = symlinkPerms;
-          }
-          if (!perms) {
-            // Now try the opposite case: translate symlinks to their
-            // actual destination file. Firefox always resolves symlinks,
-            // and in most cases we have whitelisted fixed paths that
-            // libraries will rely on and try to open. So this codepath
-            // is mostly useful for Mesa which had its kernel interface
-            // moved around.
-            pathLen = RealPath(pathBuf, sizeof(pathBuf), pathLen);
-            perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
-          }
+      }
+      if (!perms) {
+        // Did we arrive from a symlink in a path that is not writable?
+        // Then try to figure out the original path and see if that is
+        // readable. Work on the original path, this reverses
+        // ConvertRelative above.
+        int symlinkPerms = SymlinkPermissions(recvBuf, first_len);
+        if (symlinkPerms > 0) {
+          perms = symlinkPerms;
         }
       }
+      if (!perms) {
+        // Now try the opposite case: translate symlinks to their
+        // actual destination file. Firefox always resolves symlinks,
+        // and in most cases we have whitelisted fixed paths that
+        // libraries will rely on and try to open. So this codepath
+        // is mostly useful for Mesa which had its kernel interface
+        // moved around.
+        pathLen = RealPath(pathBuf, sizeof(pathBuf), pathLen);
+        perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
+      }
 
       // Same for the second path.
       pathLen2 = strnlen(pathBuf2, kMaxPathLen);
       if (pathLen2 > 0) {
         // Force 0 termination.
         pathBuf2[pathLen2] = '\0';
         pathLen2 = ConvertRelativePath(pathBuf2, sizeof(pathBuf2), pathLen2);
         int perms2 = mPolicy->Lookup(nsDependentCString(pathBuf2, pathLen2));
@@ -1050,18 +1051,19 @@ SandboxBroker::AuditPermissive(int aOp, 
 
   struct stat statBuf;
 
   if (lstat(aPath, &statBuf) == 0) {
     // Path exists, set errno to 0 to indicate "success".
     errno = 0;
   }
 
-  SANDBOX_LOG_ERROR("SandboxBroker: would have denied op=%d rflags=%o perms=%d path=%s for pid=%d" \
-                    " permissive=1 error=\"%s\"", aOp, aFlags, aPerms,
+  SANDBOX_LOG_ERROR("SandboxBroker: would have denied op=%s rflags=%o perms=%d path=%s for pid=%d" \
+                    " permissive=1 error=\"%s\"", OperationDescription[aOp],
+                    aFlags, aPerms,
                     aPath, mChildPid, strerror(errno));
 }
 
 void
 SandboxBroker::AuditDenial(int aOp, int aFlags, int aPerms, const char* aPath)
 {
   if (SandboxInfo::Get().Test(SandboxInfo::kVerbose)) {
     SANDBOX_LOG_ERROR("SandboxBroker: denied op=%s rflags=%o perms=%d path=%s for pid=%d",
--- a/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
+++ b/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
@@ -9,16 +9,17 @@
 #include "SandboxLogging.h"
 
 #include "mozilla/Array.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/SandboxSettings.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/UniquePtrExtensions.h"
+#include "mozilla/SandboxLaunch.h"
 #include "mozilla/dom/ContentChild.h"
 #include "nsPrintfCString.h"
 #include "nsString.h"
 #include "nsThreadUtils.h"
 #include "nsXULAppAPI.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsAppDirectoryServiceDefs.h"
 #include "SpecialSystemDirectory.h"
@@ -46,16 +47,17 @@
 namespace mozilla {
 
 #if defined(MOZ_CONTENT_SANDBOX)
 namespace {
 static const int rdonly = SandboxBroker::MAY_READ;
 static const int wronly = SandboxBroker::MAY_WRITE;
 static const int rdwr = rdonly | wronly;
 static const int rdwrcr = rdwr | SandboxBroker::MAY_CREATE;
+static const int access = SandboxBroker::MAY_ACCESS;
 }
 #endif
 
 static void
 AddMesaSysfsPaths(SandboxBroker::Policy* aPolicy)
 {
   // Bug 1384178: Mesa driver loader
   aPolicy->AddPrefix(rdonly, "/sys/dev/char/226:");
@@ -523,16 +525,25 @@ SandboxBrokerPolicyFactory::GetContentPo
 
   if (allowPulse) {
     // PulseAudio also needs access to read the $XAUTHORITY file (see
     // bug 1384986 comment #1), but that's already allowed for hybrid
     // GPU drivers (see above).
     policy->AddPath(rdonly, "/var/lib/dbus/machine-id");
   }
 
+  // Bug 1434711 - AMDGPU-PRO crashes if it can't read it's marketing ids
+  // and various other things
+  if (HasAtiDrivers()) {
+    policy->AddDir(rdonly, "/opt/amdgpu/share");
+    policy->AddPath(rdonly, "/sys/module/amdgpu");
+    // AMDGPU-PRO's MESA version likes to readlink a lot of things here
+    policy->AddDir(access, "/sys");
+  }
+
   // Return the common policy.
   policy->FixRecursivePermissions();
   return policy;
 }
 
 void
 SandboxBrokerPolicyFactory::AddDynamicPathList(SandboxBroker::Policy *policy,
                                                const char* aPathListPref,
--- a/security/sandbox/linux/launch/SandboxLaunch.cpp
+++ b/security/sandbox/linux/launch/SandboxLaunch.cpp
@@ -114,16 +114,41 @@ IsDisplayLocal()
   }
 #endif
 
   // Assume that other backends (e.g., Wayland) will not use the
   // network namespace.
   return true;
 }
 
+bool HasAtiDrivers()
+{
+  nsCOMPtr<nsIGfxInfo> gfxInfo = services::GetGfxInfo();
+  nsAutoString vendorID;
+  static const Array<nsresult (nsIGfxInfo::*)(nsAString&), 2> kMethods = {
+    &nsIGfxInfo::GetAdapterVendorID,
+    &nsIGfxInfo::GetAdapterVendorID2,
+  };
+  for (const auto method : kMethods) {
+    if (NS_SUCCEEDED((gfxInfo->*method)(vendorID))) {
+      // This test is based on telemetry data.  The proprietary ATI
+      // drivers seem to use this vendor string, including for some
+      // newer devices that have AMD branding in the device name, such
+      // as those using AMDGPU-PRO drivers.
+      // The open-source drivers integrated into Mesa appear to use
+      // the vendor ID "X.Org" instead.
+      if (vendorID.EqualsLiteral("ATI Technologies Inc.")) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 // Content processes may need direct access to SysV IPC in certain
 // uncommon use cases.
 static bool
 ContentNeedsSysVIPC()
 {
   // The ALSA dmix plugin uses SysV semaphores and shared memory to
   // coordinate software mixing.
 #ifdef MOZ_ALSA
@@ -133,34 +158,20 @@ ContentNeedsSysVIPC()
 #endif
 
   // Bug 1438391: VirtualGL uses SysV shm for images and configuration.
   if (PR_GetEnv("VGL_ISACTIVE") != nullptr) {
     return true;
   }
 
   // The fglrx (ATI Catalyst) GPU drivers use SysV IPC.
-  nsCOMPtr<nsIGfxInfo> gfxInfo = services::GetGfxInfo();
-  nsAutoString vendorID;
-  static const Array<nsresult (nsIGfxInfo::*)(nsAString&), 2> kMethods = {
-    &nsIGfxInfo::GetAdapterVendorID,
-    &nsIGfxInfo::GetAdapterVendorID2,
-  };
-  for (const auto method : kMethods) {
-    if (NS_SUCCEEDED((gfxInfo->*method)(vendorID))) {
-      // This test is based on telemetry data.  The proprietary ATI
-      // drivers seem to use this vendor string, including for some
-      // newer devices that have AMD branding in the device name.
-      // The open-source drivers integrated into Mesa appear to use
-      // the vendor ID "X.Org" instead.
-      if (vendorID.EqualsLiteral("ATI Technologies Inc.")) {
-        return true;
-      }
-    }
+  if (HasAtiDrivers()) {
+    return true;
   }
+
   return false;
 }
 
 static void
 PreloadSandboxLib(base::environment_map* aEnv)
 {
   // Preload libmozsandbox.so so that sandbox-related interpositions
   // can be defined there instead of in the executable.
--- a/security/sandbox/linux/launch/SandboxLaunch.h
+++ b/security/sandbox/linux/launch/SandboxLaunch.h
@@ -12,12 +12,13 @@
 
 namespace mozilla {
 
 // Called in the parent process to set up launch-time aspects of
 // sandboxing.  If aType is GeckoProcessType_Content, this must be
 // called on the main thread in order to access prefs.
 void SandboxLaunchPrepare(GeckoProcessType aType,
                           base::LaunchOptions* aOptions);
+bool HasAtiDrivers();
 
 } // namespace mozilla
 
 #endif // mozilla_SandboxLaunch_h