sandbox: Refine the hardware video decoding policy.

This CL makes the hardware video decoding sandbox policy different for
the 3 mutually exclusive cases we need to handle (explained in
go/oop-vd-sandbox):

- Hardware decoding using VA-API on Intel.
- Hardware decoding using VA-API on AMD.
- Hardware decoding using V4L2.

I've tried to make the sandbox policy tight in the sense that commenting
a given permission would cause the representative
video.PlaybackPerf.h264_1080p_30fps_hw_oopvd test to fail on at least
one device. Nonetheless, this is a starting point: I know for a fact
that parts of the policy could be tightened even more (for example, by
restricting the arguments of the ioctl() system call). go/oop-vd-sandbox
discusses why this is enough for now.

Bug: b:210759684
Test: video.PlaybackPerf.h264_1080p_30fps_hw_oopvd on volteer
Test: video.PlaybackPerf.h264_1080p_30fps_hw_oopvd on zork
Test: video.PlaybackPerf.h264_1080p_30fps_hw_oopvd on trogdor
Test: video.PlaybackPerf.h264_1080p_30fps_hw_oopvd on kevin
Test: video.PlaybackPerf.h264_1080p_30fps_hw_oopvd on asurada
Test: video.PlaybackPerf.h264_1080p_30fps_hw_oopvd on elm
Change-Id: I04d52517b00c63c12b2053acf2e23cb8adb5ad21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3997337
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Auto-Submit: Andres Calderon Jaramillo <andrescj@chromium.org>
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1075074}

sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc

@@ -194,6 +194,21 @@ bool SyscallSets::IsFileSystem(int sysno) {
   }
 }
 
+bool SyscallSets::IsTruncate(int sysno) {
+  switch (sysno) {
+    case __NR_ftruncate:
+    case __NR_truncate:
+#if defined(__i386__) || defined(__arm__) || \
+    (defined(ARCH_CPU_MIPS_FAMILY) && defined(ARCH_CPU_32_BITS))
+    case __NR_ftruncate64:
+    case __NR_truncate64:
+#endif
+      return true;
+    default:
+      return false;
+  }
+}
+
 bool SyscallSets::IsAllowedFileSystemAccessViaFd(int sysno) {
   switch (sysno) {
     case __NR_fstat:

sandbox/linux/seccomp-bpf-helpers/syscall_sets.h

@@ -29,6 +29,7 @@ class SANDBOX_EXPORT SyscallSets {
   // a new file descriptor or otherwise perform an operation directly
   // via a path.
   static bool IsFileSystem(int sysno);
+  static bool IsTruncate(int sysno);
   static bool IsAllowedFileSystemAccessViaFd(int sysno);
   static bool IsDeniedFileSystemAccessViaFd(int sysno);
   static bool IsGetSimpleId(int sysno);

sandbox/policy/linux/bpf_hardware_video_decoding_policy_linux.cc

@@ -4,17 +4,21 @@
 
 #include "sandbox/policy/linux/bpf_hardware_video_decoding_policy_linux.h"
 
-#include "base/command_line.h"
+#include <linux/kcmp.h>
+
 #include "media/gpu/buildflags.h"
+#include "sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h"
+#include "sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.h"
 #include "sandbox/linux/seccomp-bpf-helpers/syscall_sets.h"
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 #include "sandbox/linux/system_headers/linux_syscalls.h"
-#include "sandbox/policy/linux/bpf_cros_amd_gpu_policy_linux.h"
-#include "sandbox/policy/linux/bpf_cros_arm_gpu_policy_linux.h"
-#include "sandbox/policy/linux/bpf_gpu_policy_linux.h"
 #include "sandbox/policy/linux/sandbox_linux.h"
-#include "sandbox/policy/switches.h"
 
+using sandbox::bpf_dsl::AllOf;
+using sandbox::bpf_dsl::Allow;
+using sandbox::bpf_dsl::Arg;
+using sandbox::bpf_dsl::Error;
+using sandbox::bpf_dsl::If;
 using sandbox::bpf_dsl::ResultExpr;
 
 namespace sandbox::policy {
@@ -49,33 +53,93 @@ HardwareVideoDecodingProcessPolicy::ComputePolicyType(
 
 HardwareVideoDecodingProcessPolicy::HardwareVideoDecodingProcessPolicy(
     PolicyType policy_type)
-    : policy_type_(policy_type) {
+    : policy_type_(policy_type) {}
+
+ResultExpr HardwareVideoDecodingProcessPolicy::EvaluateSyscall(
+    int system_call_number) const {
   switch (policy_type_) {
     case PolicyType::kVaapiOnIntel:
-      gpu_process_policy_ = std::make_unique<GpuProcessPolicy>();
-      break;
+      return EvaluateSyscallForVaapiOnIntel(system_call_number);
     case PolicyType::kVaapiOnAMD:
-      gpu_process_policy_ = std::make_unique<CrosAmdGpuProcessPolicy>();
-      break;
+      return EvaluateSyscallForVaapiOnAMD(system_call_number);
     case PolicyType::kV4L2:
-      gpu_process_policy_ = std::make_unique<CrosArmGpuProcessPolicy>(
-          base::CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kGpuSandboxAllowSysVShm));
-      break;
+      return EvaluateSyscallForV4L2(system_call_number);
   }
 }
 
-HardwareVideoDecodingProcessPolicy::~HardwareVideoDecodingProcessPolicy() =
-    default;
+ResultExpr HardwareVideoDecodingProcessPolicy::EvaluateSyscallForVaapiOnIntel(
+    int system_call_number) const {
+  if (SyscallSets::IsTruncate(system_call_number)) {
+    // Explicitly disallow ftruncate()/truncate() to eliminate the possibility
+    // that a video decoder process can change the size of a file (including,
+    // e.g., a dma-buf).
+    return CrashSIGSYS();
+  }
 
-ResultExpr HardwareVideoDecodingProcessPolicy::EvaluateSyscall(
+  if (system_call_number == __NR_ioctl)
+    return Allow();
+
+  auto* sandbox_linux = SandboxLinux::GetInstance();
+  if (sandbox_linux->ShouldBrokerHandleSyscall(system_call_number))
+    return sandbox_linux->HandleViaBroker(system_call_number);
+
+  return BPFBasePolicy::EvaluateSyscall(system_call_number);
+}
+
+ResultExpr HardwareVideoDecodingProcessPolicy::EvaluateSyscallForVaapiOnAMD(
     int system_call_number) const {
-  switch (policy_type_) {
-    case PolicyType::kVaapiOnIntel:
-    case PolicyType::kVaapiOnAMD:
-    case PolicyType::kV4L2:
-      return gpu_process_policy_->EvaluateSyscall(system_call_number);
+  if (SyscallSets::IsTruncate(system_call_number)) {
+    // Explicitly disallow ftruncate()/truncate() to eliminate the possibility
+    // that a video decoder process can change the size of a file (including,
+    // e.g., a dma-buf).
+    return CrashSIGSYS();
+  }
+
+  switch (system_call_number) {
+    case __NR_getdents64:
+    case __NR_ioctl:
+    case __NR_sysinfo:
+    case __NR_sched_setscheduler:
+      return Allow();
+    case __NR_sched_setaffinity:
+      return RestrictSchedTarget(GetPolicyPid(), system_call_number);
+    case __NR_kcmp: {
+      const Arg<pid_t> pid1(0);
+      const Arg<pid_t> pid2(1);
+      const Arg<int> type(2);
+      const pid_t policy_pid = GetPolicyPid();
+      // Only allowed when comparing file handles for the calling thread.
+      return If(AllOf(pid1 == policy_pid, pid2 == policy_pid,
+                      type == KCMP_FILE),
+                Allow())
+          .Else(Error(EPERM));
+    }
   }
+
+  auto* sandbox_linux = SandboxLinux::GetInstance();
+  if (sandbox_linux->ShouldBrokerHandleSyscall(system_call_number))
+    return sandbox_linux->HandleViaBroker(system_call_number);
+
+  return BPFBasePolicy::EvaluateSyscall(system_call_number);
+}
+
+ResultExpr HardwareVideoDecodingProcessPolicy::EvaluateSyscallForV4L2(
+    int system_call_number) const {
+  if (SyscallSets::IsTruncate(system_call_number)) {
+    // Explicitly disallow ftruncate()/truncate() to eliminate the possibility
+    // that a video decoder process can change the size of a file (including,
+    // e.g., a dma-buf).
+    return CrashSIGSYS();
+  }
+
+  if (system_call_number == __NR_ioctl)
+    return Allow();
+
+  auto* sandbox_linux = SandboxLinux::GetInstance();
+  if (sandbox_linux->ShouldBrokerHandleSyscall(system_call_number))
+    return sandbox_linux->HandleViaBroker(system_call_number);
+
+  return BPFBasePolicy::EvaluateSyscall(system_call_number);
 }
 
 }  // namespace sandbox::policy

sandbox/policy/linux/bpf_hardware_video_decoding_policy_linux.h

@@ -11,8 +11,6 @@
 
 namespace sandbox::policy {
 
-class GpuProcessPolicy;
-
 // Policy used to sandbox utility processes that perform hardware video decoding
 // on behalf of untrusted clients (Chrome renderer processes or ARC++/ARCVM).
 //
@@ -36,14 +34,19 @@ class SANDBOX_POLICY_EXPORT HardwareVideoDecodingProcessPolicy
       const HardwareVideoDecodingProcessPolicy&) = delete;
   HardwareVideoDecodingProcessPolicy& operator=(
       const HardwareVideoDecodingProcessPolicy&) = delete;
-  ~HardwareVideoDecodingProcessPolicy() override;
+  ~HardwareVideoDecodingProcessPolicy() override = default;
 
   // sandbox::bpf_dsl::Policy implementation.
   bpf_dsl::ResultExpr EvaluateSyscall(int system_call_number) const override;
 
  private:
+  bpf_dsl::ResultExpr EvaluateSyscallForVaapiOnIntel(
+      int system_call_number) const;
+  bpf_dsl::ResultExpr EvaluateSyscallForVaapiOnAMD(
+      int system_call_number) const;
+  bpf_dsl::ResultExpr EvaluateSyscallForV4L2(int system_call_number) const;
+
   const PolicyType policy_type_;
-  std::unique_ptr<GpuProcessPolicy> gpu_process_policy_;
 };
 
 }  // namespace sandbox::policy