commit ab5b3bd919db1c43d70e10b0adf042db0e7b017a
author Kelvin Zhang <zhangkelvin@google.com> Mon Jan 22 15:01:02 2024 -0800
committer Kelvin Zhang <zhangkelvin@google.com> Mon Jan 22 16:59:22 2024 -0800
tree 158f5f4eba68adeaa20d94d9626b0ff92114be9e
parent 1fa3355b80d8d421a1d37433d05958698ea3b222

Disable fdsan after clone()

libbrillo closes all file descriptors after calling clone() to prepare
for exec(). FDSAN complains because child process is trying to close
file descriptors already owned by unique_fd. This is a false positive,
as child process will immediately call execv(), making all unique_fd
obsolete.

Test: th
Bug: 321373933
Change-Id: Ib5331dbca68cfa98134ccf3d761eaa21d0ffc4e6

Android.bp

diff --git a/Android.bp b/Android.bp
index 8a10dc8..b999c68 100644
--- a/Android.bp
+++ b/Android.bp
@@ -171,7 +171,9 @@
     static_libs: [
         "libmodpb64",
     ],
-    header_libs: ["libgtest_prod_headers"],
+    header_libs: [
+        "libgtest_prod_headers",
+    ],
     cflags: libbrillo_CFLAGS,
     export_include_dirs: ["."],
 

brillo/process.cc

diff --git a/brillo/process.cc b/brillo/process.cc
index 5623db8..1fb33bf 100644
--- a/brillo/process.cc
+++ b/brillo/process.cc
@@ -4,6 +4,10 @@
 
 #include "brillo/process.h"
 
+#ifdef __BIONIC__
+#include <android/fdsan.h>
+#endif
+
 #include <fcntl.h>
 #include <signal.h>
 #include <stdint.h>
@@ -37,11 +41,9 @@
   return true;
 }
 
-Process::Process() {
-}
+Process::Process() {}
 
-Process::~Process() {
-}
+Process::~Process() {}
 
 bool Process::ProcessExists(pid_t pid) {
   return base::DirectoryExists(
@@ -55,8 +57,7 @@
       pre_exec_(base::Bind(&ReturnTrue)),
       search_path_(false),
       inherit_parent_signal_mask_(false),
-      close_unused_file_descriptors_(false) {
-}
+      close_unused_file_descriptors_(false) {}
 
 ProcessImpl::~ProcessImpl() {
   Reset(0);
@@ -167,8 +168,7 @@
 
 bool ProcessImpl::IsFileDescriptorInPipeMap(int fd) const {
   for (const auto& pipe : pipe_map_) {
-    if (fd == pipe.second.parent_fd_ ||
-        fd == pipe.second.child_fd_ ||
+    if (fd == pipe.second.parent_fd_ || fd == pipe.second.child_fd_ ||
         fd == pipe.first) {
       return true;
     }
@@ -259,6 +259,11 @@
 }
 
 void ProcessImpl::ExecChildProcess(char** argv) {
+#ifdef __BIONIC__
+  // Disable fdsan and fdtrack post-fork, so we don't falsely trigger on
+  // processes that fork, close all of their fds, and then exec.
+  android_fdsan_set_error_level(ANDROID_FDSAN_ERROR_LEVEL_DISABLED);
+#endif
   // Executing inside the child process.
   // Close unused file descriptors.
   if (close_unused_file_descriptors_) {
@@ -361,8 +366,8 @@
   // kill the process that has just exited.
   UpdatePid(0);
   if (!WIFEXITED(status)) {
-    DCHECK(WIFSIGNALED(status)) << old_pid
-                                << " neither exited, nor died on a signal?";
+    DCHECK(WIFSIGNALED(status))
+        << old_pid << " neither exited, nor died on a signal?";
     LOG(ERROR) << "Process " << old_pid
                << " did not exit normally: " << WTERMSIG(status);
     return -1;