commit 1fa1185070afe03ec6eef810b896fb497ad9a5d0
author Ricky Liang <jcliang@chromium.org> Fri Aug 05 19:58:54 2022 +0800
committer Chromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Aug 12 01:15:32 2022 +0000
tree 439e1cb2f46e0ca2c05e28a1d5c885e290bd216c
parent d324ee71be7ee7bcc05f00f57ff7b13d488ff220

camera: dup buffer FDs for Mojo transport

Mojo closes the FDs in a mojo callback, so we need to dup the FDs if
we want to keep the original FDs open.

While we're at it, copy the checks of close() return value from
base::ScopedFD to camera_buffer_handle so that we can catch
double-close bugs more easily in the future.

BUG=b:241459028
TEST=Run cros_camera_connector_test on redrix

Change-Id: I9f97d647cd776dd757ba726b7ac5ae13f2198b25
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3811049
Reviewed-by: Shik Chen <shik@chromium.org>
Tested-by: Ricky Liang <jcliang@chromium.org>
Commit-Queue: Ricky Liang <jcliang@chromium.org>
(cherry picked from commit 02005b48b1b6b6d6c426893eab618cf8d843beb0)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3821240

camera/common/camera_buffer_handle.h

diff --git a/camera/common/camera_buffer_handle.h b/camera/common/camera_buffer_handle.h
index 12e1806..9327d0e 100644
--- a/camera/common/camera_buffer_handle.h
+++ b/camera/common/camera_buffer_handle.h
@@ -66,9 +66,16 @@
 
   ~camera_buffer_handle() {
     for (size_t i = 0; i < kMaxPlanes; ++i) {
-      if (fds[i] != -1) {
-        close(fds[i]);
+      if (fds[i] == -1) {
+        continue;
       }
+      // See the comments in base/files/scoped_file.cc in libchrome for why we
+      // need to crash here when close fails.
+      int ret = IGNORE_EINTR(close(fds[i]));
+      if (ret != 0 && errno != EBADF) {
+        ret = 0;
+      }
+      PCHECK(0 == ret);
     }
   }
 

camera/hal_adapter/camera_device_adapter.cc

diff --git a/camera/hal_adapter/camera_device_adapter.cc b/camera/hal_adapter/camera_device_adapter.cc
index b755853..8769d4d 100644
--- a/camera/hal_adapter/camera_device_adapter.cc
+++ b/camera/hal_adapter/camera_device_adapter.cc
@@ -859,8 +859,10 @@
       auto num_planes = CameraBufferManager::GetNumPlanes(buffer_handle);
       mojo_buffer_handle->sizes = std::vector<uint32_t>();
       for (size_t plane = 0; plane < num_planes; plane++) {
+        auto dup_fd = DupWithCloExec(buffer_handle->data[plane]);
+        CHECK(dup_fd.is_valid());
         mojo_buffer_handle->fds.push_back(
-            mojo::WrapPlatformFile(base::ScopedFD(buffer_handle->data[plane])));
+            mojo::WrapPlatformFile(std::move(dup_fd)));
         mojo_buffer_handle->strides.push_back(
             CameraBufferManager::GetPlaneStride(buffer_handle, plane));
         mojo_buffer_handle->offsets.push_back(

camera/include/cros-camera/common.h

diff --git a/camera/include/cros-camera/common.h b/camera/include/cros-camera/common.h
index ad22391..26f8726 100644
--- a/camera/include/cros-camera/common.h
+++ b/camera/include/cros-camera/common.h
@@ -80,7 +80,7 @@
   if (fd < 0) {
     return base::ScopedFD();
   }
-  return base::ScopedFD(HANDLE_EINTR(fcntl(fd, F_DUPFD_CLOEXEC)));
+  return base::ScopedFD(HANDLE_EINTR(fcntl(fd, F_DUPFD_CLOEXEC, 0)));
 }
 
 #endif  // CAMERA_INCLUDE_CROS_CAMERA_COMMON_H_