[libFuzzer] Avoid undefined behavior. Properly discard output to stdout and stderr.

[mpividori]

Summary

fuzzer-fdmask.test was failing in Windows, when setting -close_fd_mask to a non-zero value.

I realized it was happening, because libFuzzer closes the file descriptor for stdout (1) or stderr (2) , but after that, it writes directly to stdout and stderr using the file streams stdout and stderr, which is undefined behavior. In Windows, in particular, this was making the test fail.

Instead of closing the file descriptors, I redirect the output to /dev/null on linux and nul on Windows.

[vitalybuka]

lib/Fuzzer/FuzzerIOPosix.cpp

80 ↗

(On Diff #84412)

Why not just use CloseFile on Posix if it works?

[mpividori]

Hi, it is undefined behavior. It could fail.
Also, if we close stdout and we open a new file in libFuzzer, we get the file descriptor 1, which could generate problem if some code assumes file descriptors refers to stdout and works directly writing to the file descriptor 1, but it will be writing to the opened file.

[mpividori]

For example, before this commit, if you compile this simple test function with libFuzzer and execute: ./my_fuzzer -close_fd_mask=1 , it will write to the testout file many times the message that was expected to be send to stdout.

#include <cstdint>
#include <fcntl.h>
#include <iostream>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>

static volatile int Sink;

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
  int fd = open("testout", O_CREAT | O_WRONLY | O_APPEND);
  if (fd == -1) return 1;
  std::cout << "SOME MSG TO STDOUT, NOT TO THE FILE" << std::endl;
  close(fd);
  return 0;
}

[zturner]

lib/Fuzzer/FuzzerIOPosix.cpp

80 ↗

(On Diff #84412)

I think it was just working on accident, seems like writing to a closed file descriptor is always wrong, even if you get lucky and it "works".

[kcc]

LGTM

llvm/trunk/lib/Fuzzer/FuzzerIO.h

 FILE* OpenFile(int Fd, const char *Mode);
 
 int CloseFile(int Fd);
 
 int DuplicateFile(int Fd);
 
 void RemoveFile(const std::string &Path);
 
+void DiscardOutput(int Fd);
+
 }  // namespace fuzzer
 
 #endif  // LLVM_FUZZER_IO_H

llvm/trunk/lib/Fuzzer/FuzzerIO.cpp

 void DupAndCloseStderr() {
   int OutputFd = DuplicateFile(2);
   if (OutputFd > 0) {
     FILE *NewOutputFile = OpenFile(OutputFd, "w");
     if (NewOutputFile) {
       OutputFile = NewOutputFile;
       if (EF->__sanitizer_set_report_fd)
         EF->__sanitizer_set_report_fd(reinterpret_cast<void *>(OutputFd));
-      CloseFile(2);
+      DiscardOutput(2);
     }
   }
 }
 
 void CloseStdout() {
-  CloseFile(1);
+  DiscardOutput(1);
 }
 
 void Printf(const char *Fmt, ...) {
   va_list ap;
   va_start(ap, Fmt);
   vfprintf(OutputFile, Fmt, ap);
   va_end(ap);
   fflush(OutputFile);
 }
 
 }  // namespace fuzzer

llvm/trunk/lib/Fuzzer/FuzzerIOPosix.cpp

 int DuplicateFile(int Fd) {
   return dup(Fd);
 }
 
 void RemoveFile(const std::string &Path) {
   unlink(Path.c_str());
 }
 
+void DiscardOutput(int Fd) {
+  FILE* Temp = fopen("/dev/null", "w");
+  if (!Temp)
+    return;
+  dup2(fileno(Temp), Fd);
+  fclose(Temp);
+}
+
 std::string DirName(const std::string &FileName) {
   char *Tmp = new char[FileName.size() + 1];
   memcpy(Tmp, FileName.c_str(), FileName.size() + 1);
   std::string Res = dirname(Tmp);
   delete [] Tmp;
   return Res;
 }
 

llvm/trunk/lib/Fuzzer/FuzzerIOWindows.cpp

 int DuplicateFile(int Fd) {
   return _dup(Fd);
 }
 
 void RemoveFile(const std::string &Path) {
   _unlink(Path.c_str());
 }
 
+void DiscardOutput(int Fd) {
+  FILE* Temp = fopen("nul", "w");
+  if (!Temp)
+    return;
+  _dup2(_fileno(Temp), Fd);
+  fclose(Temp);
+}
+
 static bool IsSeparator(char C) {
   return C == '\\' || C == '/';
 }
 
 // Parse disk designators, like "C:\". If Relative == true, also accepts: "C:".
 // Returns number of characters considered if successful.
 static size_t ParseDrive(const std::string &FileName, const size_t Offset,
                          bool Relative = true) {