arc: Create ARC server socket in Chrome

Create ARC server socket in chrome instead of in session_manager.

BUG=b:119447298
TEST=Ensure container starts up

Change-Id: I6207f16fd6cfaa5a9eface6cf1ae6806c053157b
Reviewed-on: https://chromium-review.googlesource.com/c/1343046
Commit-Queue: Christopher Morin <cmtm@google.com>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610198}

chromeos/dbus/fake_session_manager_client.cc

@@ -531,7 +531,7 @@ void FakeSessionManagerClient::StartArcMiniContainer(
 
 void FakeSessionManagerClient::UpgradeArcContainer(
     const login_manager::UpgradeArcContainerRequest& request,
-    UpgradeArcContainerCallback success_callback,
+    base::OnceClosure success_callback,
     UpgradeErrorCallback error_callback) {
   last_upgrade_arc_request_ = request;
 
@@ -549,7 +549,8 @@ void FakeSessionManagerClient::UpgradeArcContainer(
     PostReply(FROM_HERE, std::move(error_callback), true);
     return;
   }
-  PostReply(FROM_HERE, std::move(success_callback), base::ScopedFD());
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                std::move(success_callback));
 }
 
 void FakeSessionManagerClient::StopArcInstance(

chromeos/dbus/fake_session_manager_client.h

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_forward.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
@@ -100,7 +101,7 @@ class FakeSessionManagerClient : public SessionManagerClient {
       StartArcMiniContainerCallback callback) override;
   void UpgradeArcContainer(
       const login_manager::UpgradeArcContainerRequest& request,
-      UpgradeArcContainerCallback success_callback,
+      base::OnceClosure success_callback,
       UpgradeErrorCallback error_callback) override;
   void StopArcInstance(VoidDBusMethodCallback callback) override;
   void SetArcCpuRestriction(

chromeos/dbus/session_manager_client.cc

@@ -412,7 +412,7 @@ class SessionManagerClientImpl : public SessionManagerClient {
 
   void UpgradeArcContainer(
       const login_manager::UpgradeArcContainerRequest& request,
-      UpgradeArcContainerCallback success_callback,
+      base::OnceClosure success_callback,
       UpgradeErrorCallback error_callback) override {
     DCHECK(!success_callback.is_null());
     DCHECK(!error_callback.is_null());
@@ -811,7 +811,7 @@ class SessionManagerClientImpl : public SessionManagerClient {
     std::move(callback).Run(std::move(container_instance_id));
   }
 
-  void OnUpgradeArcContainer(UpgradeArcContainerCallback success_callback,
+  void OnUpgradeArcContainer(base::OnceClosure success_callback,
                              UpgradeErrorCallback error_callback,
                              dbus::Response* response,
                              dbus::ErrorResponse* error) {
@@ -823,15 +823,7 @@ class SessionManagerClientImpl : public SessionManagerClient {
                             login_manager::dbus_error::kLowFreeDisk);
       return;
     }
-
-    dbus::MessageReader reader(response);
-    base::ScopedFD server_socket;
-    if (!reader.PopFileDescriptor(&server_socket)) {
-      LOG(ERROR) << "Invalid response: " << response->ToString();
-      std::move(error_callback).Run(false);
-      return;
-    }
-    std::move(success_callback).Run(std::move(server_socket));
+    std::move(success_callback).Run();
   }
 
   dbus::ObjectProxy* session_manager_proxy_ = nullptr;

chromeos/dbus/session_manager_client.h

@@ -10,7 +10,6 @@
 #include <vector>
 
 #include "base/callback.h"
-#include "base/files/scoped_file.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/time/time.h"
@@ -326,17 +325,14 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
       StartArcMiniContainerCallback callback) = 0;
 
   // UpgradeArcContainer upgrades a mini-container to a full ARC container. In
-  // case of success, success_callback is called. |server_socket| should be
-  // accept(2)ed to connect to the ArcBridgeService Mojo channel.  In case of
-  // error, error_callback will be called with a |low_free_disk_space| signaling
+  // case of success, success_callback is called. In case of error,
+  // error_callback will be called with a |low_free_disk_space| signaling
   // whether the failure was due to low free disk space.
-  using UpgradeArcContainerCallback =
-      base::OnceCallback<void(base::ScopedFD server_socket)>;
   using UpgradeErrorCallback =
       base::OnceCallback<void(bool low_free_disk_space)>;
   virtual void UpgradeArcContainer(
       const login_manager::UpgradeArcContainerRequest& request,
-      UpgradeArcContainerCallback success_callback,
+      base::OnceClosure success_callback,
       UpgradeErrorCallback error_callback) = 0;
 
   // Asynchronously stops the ARC instance.  Upon completion, invokes

components/arc/arc_session_impl.cc

@@ -5,6 +5,7 @@
 #include "components/arc/arc_session_impl.h"
 
 #include <fcntl.h>
+#include <grp.h>
 #include <poll.h>
 #include <unistd.h>
 
@@ -13,6 +14,7 @@
 
 #include "ash/public/cpp/default_scale_factor_retriever.h"
 #include "base/command_line.h"
+#include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/rand_util.h"
@@ -39,6 +41,9 @@ namespace arc {
 
 namespace {
 
+constexpr char kArcBridgeSocketPath[] = "/run/chrome/arc_bridge.sock";
+constexpr char kArcBridgeSocketGroup[] = "arc-bridge";
+
 chromeos::SessionManagerClient* GetSessionManagerClient() {
   // If the DBusThreadManager or the SessionManagerClient aren't available,
   // there isn't much we can do. This should only happen when running tests.
@@ -136,11 +141,17 @@ class ArcSessionDelegateImpl : public ArcSessionImpl::Delegate {
   ~ArcSessionDelegateImpl() override = default;
 
   // ArcSessionImpl::Delegate override.
+  void CreateSocket(CreateSocketCallback callback) override;
+
   base::ScopedFD ConnectMojo(base::ScopedFD socket_fd,
                              ConnectMojoCallback callback) override;
   void GetLcdDensity(GetLcdDensityCallback callback) override;
 
  private:
+  // Synchronously create a UNIX domain socket. This is designed to run on a
+  // blocking thread. Unlinks any existing files at socket address.
+  static base::ScopedFD CreateSocketInternal();
+
   // Synchronously accepts a connection on |server_endpoint| and then processes
   // the connected socket's file descriptor. This is designed to run on a
   // blocking thread.
@@ -172,6 +183,13 @@ ArcSessionDelegateImpl::ArcSessionDelegateImpl(
       default_scale_factor_retriever_(retriever),
       weak_factory_(this) {}
 
+void ArcSessionDelegateImpl::CreateSocket(CreateSocketCallback callback) {
+  base::PostTaskWithTraitsAndReplyWithResult(
+      FROM_HERE, {base::MayBlock()},
+      base::BindOnce(&ArcSessionDelegateImpl::CreateSocketInternal),
+      std::move(callback));
+}
+
 base::ScopedFD ArcSessionDelegateImpl::ConnectMojo(
     base::ScopedFD socket_fd,
     ConnectMojoCallback callback) {
@@ -205,6 +223,51 @@ void ArcSessionDelegateImpl::GetLcdDensity(GetLcdDensityCallback callback) {
       std::move(callback)));
 }
 
+// static
+base::ScopedFD ArcSessionDelegateImpl::CreateSocketInternal() {
+  auto endpoint = mojo::NamedPlatformChannel({kArcBridgeSocketPath});
+  // TODO(cmtm): use NamedPlatformChannel to bootstrap mojo connection after
+  // libchrome uprev in android.
+  base::ScopedFD socket_fd =
+      endpoint.TakeServerEndpoint().TakePlatformHandle().TakeFD();
+  if (!socket_fd.is_valid()) {
+    LOG(ERROR) << "Socket creation failed";
+    return socket_fd;
+  }
+
+  // Change permissions on the socket.
+  struct group arc_bridge_group;
+  struct group* arc_bridge_group_res = nullptr;
+  int ret = 0;
+  char buf[10000];
+  do {
+    ret = getgrnam_r(kArcBridgeSocketGroup, &arc_bridge_group, buf, sizeof(buf),
+                     &arc_bridge_group_res);
+  } while (ret == EINTR);
+  if (ret != 0) {
+    LOG(ERROR) << "getgrnam_r: " << strerror_r(ret, buf, sizeof(buf));
+    return base::ScopedFD();
+  }
+
+  if (!arc_bridge_group_res) {
+    LOG(ERROR) << "Group '" << kArcBridgeSocketGroup << "' not found";
+    return base::ScopedFD();
+  }
+
+  if (chown(kArcBridgeSocketPath, -1, arc_bridge_group.gr_gid) < 0) {
+    PLOG(ERROR) << "chown failed";
+    return base::ScopedFD();
+  }
+
+  if (!base::SetPosixFilePermissions(base::FilePath(kArcBridgeSocketPath),
+                                     0660)) {
+    PLOG(ERROR) << "Could not set permissions: " << kArcBridgeSocketPath;
+    return base::ScopedFD();
+  }
+
+  return socket_fd;
+}
+
 // static
 mojo::ScopedMessagePipeHandle ArcSessionDelegateImpl::ConnectMojoInternal(
     base::ScopedFD socket_fd,
@@ -391,6 +454,28 @@ void ArcSessionImpl::DoUpgrade() {
   VLOG(2) << "Upgrading an existing ARC mini instance";
   state_ = State::STARTING_FULL_INSTANCE;
 
+  delegate_->CreateSocket(base::BindOnce(&ArcSessionImpl::OnSocketCreated,
+                                         weak_factory_.GetWeakPtr()));
+}
+
+void ArcSessionImpl::OnSocketCreated(base::ScopedFD socket_fd) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_EQ(state_, State::STARTING_FULL_INSTANCE);
+
+  if (stop_requested_) {
+    // The ARC instance has started to run. Request to stop.
+    VLOG(1) << "Stop() called while creating socket";
+    StopArcInstance();
+    return;
+  }
+
+  if (!socket_fd.is_valid()) {
+    LOG(ERROR) << "ARC: Error creating socket";
+    OnStopped(ArcStopReason::GENERIC_BOOT_FAILURE);
+    return;
+  }
+
+  VLOG(2) << "Socket is created. Starting ARC container";
   login_manager::UpgradeArcContainerRequest request;
   user_manager::UserManager* user_manager = user_manager::UserManager::Get();
   DCHECK(user_manager->GetPrimaryUser());
@@ -437,10 +522,13 @@ void ArcSessionImpl::DoUpgrade() {
         upgrade_params_.demo_session_apps_path.value());
   }
 
+  request.set_create_socket_in_chrome(true);
+
   chromeos::SessionManagerClient* client = GetSessionManagerClient();
   client->UpgradeArcContainer(
       request,
-      base::BindOnce(&ArcSessionImpl::OnUpgraded, weak_factory_.GetWeakPtr()),
+      base::BindOnce(&ArcSessionImpl::OnUpgraded, weak_factory_.GetWeakPtr(),
+                     std::move(socket_fd)),
       base::BindOnce(&ArcSessionImpl::OnUpgradeError,
                      weak_factory_.GetWeakPtr()));
 }

components/arc/arc_session_impl.h

@@ -136,9 +136,13 @@ class ArcSessionImpl : public ArcSession,
     // Used for ConnectMojo completion callback.
     using ConnectMojoCallback =
         base::OnceCallback<void(std::unique_ptr<mojom::ArcBridgeHost>)>;
+    using CreateSocketCallback = base::OnceCallback<void(base::ScopedFD)>;
 
     virtual ~Delegate() = default;
 
+    // Creates arcbridge UNIX domain socket on a worker pool.
+    virtual void CreateSocket(CreateSocketCallback callback) = 0;
+
     // Connects ArcBridgeHost via |socket_fd|, and invokes |callback| with
     // connected ArcBridgeHost instance if succeeded (or nullptr if failed).
     // Returns a FD which cancels the current connection on close(2).
@@ -178,6 +182,9 @@ class ArcSessionImpl : public ArcSession,
   // Sends a D-Bus message to upgrade to a full instance.
   void DoUpgrade();
 
+  // Called when arcbridge socket is created.
+  void OnSocketCreated(base::ScopedFD fd);
+
   // D-Bus callback for UpgradeArcContainer(). |socket_fd| should be a socket
   // which should be accept(2)ed to connect ArcBridgeService Mojo channel.
   void OnUpgraded(base::ScopedFD socket_fd);

components/arc/arc_session_impl_unittest.cc

@@ -11,7 +11,6 @@
 
 #include "base/command_line.h"
 #include "base/location.h"
-#include "base/posix/eintr_wrapper.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -61,7 +60,15 @@ class FakeDelegate : public ArcSessionImpl::Delegate {
     PostCallback(std::move(pending_callback_));
   }
 
-  // ArcSessionImpl::Delegate override:
+  // ArcSessionImpl::Delegate overrides:
+  void CreateSocket(CreateSocketCallback callback) override {
+    // Open /dev/null as a dummy FD.
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(callback),
+                                  base::ScopedFD(open("/dev/null",
+                                                      O_RDONLY | O_CLOEXEC))));
+  }
+
   base::ScopedFD ConnectMojo(base::ScopedFD socket_fd,
                              ConnectMojoCallback callback) override {
     if (suspend_) {
@@ -72,7 +79,7 @@ class FakeDelegate : public ArcSessionImpl::Delegate {
     }
 
     // Open /dev/null as a dummy FD.
-    return base::ScopedFD(HANDLE_EINTR(open("/dev/null", O_RDONLY)));
+    return base::ScopedFD(open("/dev/null", O_RDONLY | O_CLOEXEC));
   }
 
   void GetLcdDensity(GetLcdDensityCallback callback) override {