Commit 23ba2c71 authored by Jed Davis's avatar Jed Davis
Browse files

Bug 1698778 - Loosen the RDD sandbox policy to support VA-API (including Intel VCS2). r=gcp 

Changes:

1. For the `intel` drivers [on newer hardware][VCS2], access to SysV IPC
   is granted.  There is a slight restriction: `semget` and `shmget` are
   restricted to the fixed `key_t` value used by the driver; however,
   the other calls take shm/sem identifiers, which are dynamically
   assigned and globally scoped, so an attacker could still access
   other resources.  This is considered a reasonable tradeoff for not
   needing to allow this (or, eventually, any GPU access) in the content
   process, which is much easier for malicious content to attack than
   RDD.

2. Access to devices in `/dev/dri` and the `DRM_IOCTL_*` ioctls (type `'d'`).

3. Read access to the parts of sysfs used by Mesa to do device detection;
   again, given the choice we'd rather allow this in RDD than content.

4. Read access to directories containing libraries, for plugin loading.

5. Allowing `kcmp` in the special case of comparing the process's
   own fds, for `amdgpu` (already allowed for content).

6. The `eventfd2` syscall, which we use in connection with dma-buf.

[VCS2]: https://github.com/intel/media-driver/blob/77b3b2a6c366/media_driver/linux/common/os/mos_os_specific.c#L1508-L1512

Differential Revision: https://phabricator.services.mozilla.com/D131680
parent 15878724

security/sandbox/linux/SandboxFilter.cpp

+50 −1
Original line number Diff line number Diff line
@@ -1750,11 +1750,60 @@ class RDDSandboxPolicy final : public SandboxPolicyCommon { 
      : SandboxPolicyCommon(aBroker, ShmemUsage::MAY_CREATE,

                            AllowUnsafeSocketPair::NO) {}



#ifndef ANDROID

  Maybe<ResultExpr> EvaluateIpcCall(int aCall, int aArgShift) const override {

    // The Intel media driver uses SysV IPC (semaphores and shared

    // memory) on newer hardware models; it always uses this fixed

    // key, so we can restrict semget and shmget.  Unfortunately, the

    // calls that operate on these resources take "identifiers", which

    // are unpredictable (by us) but guessable (by an adversary).

    static constexpr key_t kIntelKey = 'D' << 24 | 'V' << 8 | 'X' << 0;



    switch (aCall) {

      case SEMGET:

      case SHMGET: {

        Arg<key_t> key(0 + aArgShift);

        return Some(If(key == kIntelKey, Allow()).Else(InvalidSyscall()));

      }



      case SEMCTL:

      case SEMOP:

      case SEMTIMEDOP:

      case SHMCTL:

      case SHMAT:

      case SHMDT:

        return Some(Allow());



      default:

        return SandboxPolicyCommon::EvaluateIpcCall(aCall, aArgShift);

    }

  }

#endif



  ResultExpr EvaluateSyscall(int sysno) const override {

    switch (sysno) {

      case __NR_getrusage:

        return Allow();



      case __NR_ioctl: {

        Arg<unsigned long> request(1);

        auto shifted_type = request & kIoctlTypeMask;

        static constexpr unsigned long kDrmType =

            static_cast<unsigned long>('d') << _IOC_TYPESHIFT;



        // Allow DRI for VA-API

        return If(shifted_type == kDrmType, Allow())

            .Else(SandboxPolicyCommon::EvaluateSyscall(sysno));

      }



        // Mesa/amdgpu

      case __NR_kcmp:

        return KcmpPolicyForMesa();



        // We use this in our DMABuf support code.

      case __NR_eventfd2:

        return Allow();



        // Pass through the common policy.

      default:

        return SandboxPolicyCommon::EvaluateSyscall(sysno);

security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp

+8 −0
Original line number Diff line number Diff line
@@ -809,6 +809,14 @@ SandboxBrokerPolicyFactory::GetRDDPolicy(int aPid) { 
    }

  }



  // VA-API needs DRI and GPU detection

  policy->AddDir(rdwr, "/dev/dri");

  AddMesaSysfsPaths(policy.get());



  // FFmpeg and GPU drivers may need general-case library loading

  AddLdconfigPaths(policy.get());

  AddLdLibraryEnvPaths(policy.get());



  if (policy->IsEmpty()) {

    policy = nullptr;

  }

security/sandbox/linux/launch/SandboxLaunch.cpp

+2 −1
Original line number Diff line number Diff line
@@ -326,7 +326,8 @@ void SandboxLaunchPrepare(GeckoProcessType aType, 
    case GeckoProcessType_RDD:

      if (level >= 1) {

        canChroot = true;

        flags |= CLONE_NEWNET | CLONE_NEWIPC;

        // Can't use CLONE_NEWIPC because of intel-media-driver.

        flags |= CLONE_NEWNET;

      }

      break;

    case GeckoProcessType_Content: