[libc] unpoison memory returned by pipe syscall

[michaelrj-google]

The memory sanitizer doesn't recognize the results of the pipe syscall
as being initialized. This patch manually unpoisons that memory.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

The memory sanitizer doesn't recognize the results of the pipe syscall
as being initialized. This patch manually unpoisons that memory.

---

Full diff: https://github.com/llvm/llvm-project/pull/88942.diff

2 Files Affected:

• (modified) libc/src/unistd/linux/pipe.cpp (+2)
• (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+6-3)

diff --git a/libc/src/unistd/linux/pipe.cpp b/libc/src/unistd/linux/pipe.cpp
index b4e8b9b7d9c85e..8cfb8d1d5c2c13 100644
--- a/libc/src/unistd/linux/pipe.cpp
+++ b/libc/src/unistd/linux/pipe.cpp
@@ -10,6 +10,7 @@
 
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
+#include "src/__support/macros/sanitizer.h" // for MSAN_UNPOISON
 #include "src/errno/libc_errno.h"
 #include <sys/syscall.h> // For syscall numbers.
 
@@ -23,6 +24,7 @@ LLVM_LIBC_FUNCTION(int, pipe, (int pipefd[2])) {
   int ret = LIBC_NAMESPACE::syscall_impl<int>(
       SYS_pipe2, reinterpret_cast<long>(pipefd), 0);
 #endif
+  MSAN_UNPOISON(pipefd, sizeof(int) * 2);
   if (ret < 0) {
     libc_errno = -ret;
     return -1;
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index fb37f113b310a7..8caf8acdb7da08 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -786,7 +786,7 @@ libc_support_library(
         ":errno",
         ":hdr_fenv_macros",
         ":hdr_math_macros",
-	":types_fenv_t"
+        ":types_fenv_t",
     ],
 )
 
@@ -1041,6 +1041,7 @@ libc_support_library(
     deps = [
         ":__support_common",
         ":__support_cpp_bit",
+        ":__support_macros_sanitizer",
     ],
 )
 
@@ -1262,7 +1263,7 @@ libc_function(
     deps = [
         ":__support_common",
         ":__support_fputil_fenv_impl",
-	":types_fexcept_t"
+        ":types_fexcept_t",
     ],
 )
 
@@ -1273,7 +1274,7 @@ libc_function(
     deps = [
         ":__support_common",
         ":__support_fputil_fenv_impl",
-	":types_fexcept_t",
+        ":types_fexcept_t",
     ],
 )
 
@@ -2987,6 +2988,7 @@ libc_function(
     hdrs = ["src/unistd/pipe.h"],
     deps = [
         ":__support_common",
+        ":__support_macros_sanitizer",
         ":__support_osutil_syscall",
         ":errno",
     ],
@@ -3014,6 +3016,7 @@ libc_function(
     }),
     deps = [
         ":__support_common",
+        ":__support_macros_sanitizer",
         ":__support_osutil_syscall",
         ":errno",
     ],

[nickdesaulniers]

+2 but cc @ramosian-glider since these changes are annoying to keep having to make. Are we "holding it wrong?" For instance, our syscall wrapper here treats all arguments as inputs.

https://github.com/llvm/llvm-project/blob/main/libc/src/__support/OSUtil/linux/x86_64/syscall.h#L36-L40

But I suspect if we said the pipefd param was an ouput with =D (rather than an input with D), perhaps msan would know that the kernel is expected to overwrite the output and DTRT?

[michaelrj-google]

Landing for now to unblock downstream. We can update this with a cleaner design later.