Linux GPU sandbox: only allocate broker policy in the broker.

The GPU broker policy was allocated in the main GPU process and then used in
the broker process. We switch the logic so that the broker policy is only ever
allocated in the broker process itself.

Besides fixing a small memory leak (in the GPU process), this makes sure that a
policy is only ever used in the process that allocated it. This will allow to
bind policies with properties such as "which processes does this policy allow
to send signal to".

BUG=367986
R=jorgelo@chromium.org

Review URL: https://codereview.chromium.org/251183004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266726 0039d316-1c4b-4281-b951-d872f2087c98

content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc

@@ -94,13 +94,16 @@ void AddArmGpuWhitelist(std::vector<std::string>* read_whitelist,
 
 class CrosArmGpuBrokerProcessPolicy : public CrosArmGpuProcessPolicy {
  public:
-  CrosArmGpuBrokerProcessPolicy() : CrosArmGpuProcessPolicy(false) {}
+  static sandbox::SandboxBPFPolicy* Create() {
+    return new CrosArmGpuBrokerProcessPolicy();
+  }
   virtual ~CrosArmGpuBrokerProcessPolicy() {}
 
   virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox_compiler,
                                     int system_call_number) const OVERRIDE;
 
  private:
+  CrosArmGpuBrokerProcessPolicy() : CrosArmGpuProcessPolicy(false) {}
   DISALLOW_COPY_AND_ASSIGN(CrosArmGpuBrokerProcessPolicy);
 };
 
@@ -169,12 +172,9 @@ bool CrosArmGpuProcessPolicy::PreSandboxHook() {
   // Add ARM-specific files to whitelist in the broker.
 
   AddArmGpuWhitelist(&read_whitelist_extra, &write_whitelist_extra);
-  InitGpuBrokerProcess(
-      base::Bind(&SandboxSeccompBPF::StartSandboxWithExternalPolicy,
-                 base::Passed(scoped_ptr<sandbox::SandboxBPFPolicy>(
-                     new CrosArmGpuBrokerProcessPolicy))),
-      read_whitelist_extra,
-      write_whitelist_extra);
+  InitGpuBrokerProcess(CrosArmGpuBrokerProcessPolicy::Create,
+                       read_whitelist_extra,
+                       write_whitelist_extra);
 
   const int dlopen_flag = RTLD_NOW | RTLD_GLOBAL | RTLD_NODELETE;
 

content/common/sandbox_linux/bpf_gpu_policy_linux.cc

@@ -106,13 +106,16 @@ intptr_t GpuSIGSYS_Handler(const struct arch_seccomp_data& args,
 
 class GpuBrokerProcessPolicy : public GpuProcessPolicy {
  public:
-  GpuBrokerProcessPolicy() {}
+  static sandbox::SandboxBPFPolicy* Create() {
+    return new GpuBrokerProcessPolicy();
+  }
   virtual ~GpuBrokerProcessPolicy() {}
 
   virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox_compiler,
                                     int system_call_number) const OVERRIDE;
 
  private:
+  GpuBrokerProcessPolicy() {}
   DISALLOW_COPY_AND_ASSIGN(GpuBrokerProcessPolicy);
 };
 
@@ -146,9 +149,11 @@ void UpdateProcessTypeToGpuBroker() {
 }
 
 bool UpdateProcessTypeAndEnableSandbox(
-    const base::Callback<bool(void)>& broker_sandboxer_callback) {
+    sandbox::SandboxBPFPolicy* (*broker_sandboxer_allocator)(void)) {
+  DCHECK(broker_sandboxer_allocator);
   UpdateProcessTypeToGpuBroker();
-  return broker_sandboxer_callback.Run();
+  return SandboxSeccompBPF::StartSandboxWithExternalPolicy(
+      make_scoped_ptr(broker_sandboxer_allocator()));
 }
 
 }  // namespace
@@ -198,9 +203,7 @@ bool GpuProcessPolicy::PreSandboxHook() {
   DCHECK(!broker_process());
   // Create a new broker process.
   InitGpuBrokerProcess(
-      base::Bind(&SandboxSeccompBPF::StartSandboxWithExternalPolicy,
-                 base::Passed(scoped_ptr<sandbox::SandboxBPFPolicy>(
-                     new GpuBrokerProcessPolicy))),
+      GpuBrokerProcessPolicy::Create,
       std::vector<std::string>(),  // No extra files in whitelist.
       std::vector<std::string>());
 
@@ -226,7 +229,7 @@ bool GpuProcessPolicy::PreSandboxHook() {
 }
 
 void GpuProcessPolicy::InitGpuBrokerProcess(
-    const base::Callback<bool(void)>& broker_sandboxer_callback,
+    sandbox::SandboxBPFPolicy* (*broker_sandboxer_allocator)(void),
     const std::vector<std::string>& read_whitelist_extra,
     const std::vector<std::string>& write_whitelist_extra) {
   static const char kDriRcPath[] = "/etc/drirc";
@@ -256,7 +259,7 @@ void GpuProcessPolicy::InitGpuBrokerProcess(
   // The initialization callback will perform generic initialization and then
   // call broker_sandboxer_callback.
   CHECK(broker_process_->Init(base::Bind(&UpdateProcessTypeAndEnableSandbox,
-                                         broker_sandboxer_callback)));
+                                         broker_sandboxer_allocator)));
 }
 
 }  // namespace content

content/common/sandbox_linux/bpf_gpu_policy_linux.h

@@ -29,13 +29,13 @@ class GpuProcessPolicy : public SandboxBPFBasePolicy {
 
  protected:
   // Start a broker process to handle open() inside the sandbox.
-  // |broker_sandboxer_callback| is a callback that will enable a suitable
-  // sandbox for the broker process itself.
+  // |broker_sandboxer_allocator| is a function pointer which can allocate a
+  // suitable sandbox policy for the broker process itself.
   // |read_whitelist_extra| and |write_whitelist_extra| are lists of file
   // names that should be whitelisted by the broker process, in addition to
   // the basic ones.
   void InitGpuBrokerProcess(
-      const base::Callback<bool(void)>& broker_sandboxer_callback,
+      sandbox::SandboxBPFPolicy* (*broker_sandboxer_allocator)(void),
       const std::vector<std::string>& read_whitelist_extra,
       const std::vector<std::string>& write_whitelist_extra);