[sanitizer_common] Implement popen, popenve, pclose interceptors

[mgorny]

Summary

Implement the interceptors for popen(), pclose() and popenve()
functions. The first two are POSIX, the third one is specific
to NetBSD. popen() spawns a process and creates a FILE object piping
data from/to that process. pclose() closes the pipe and waits for
the process to terminate appropriately.

For the purpose of popen(), a new COMMON_INTERCEPTOR_PIPE_OPEN macro is
added that is like COMMON_INTERCEPTOR_FILE_OPEN, except it is passed
the new FILE* and no path.

[krytarowski]

Looks good to me, but it would be nice to have opinion of @vitalybuka and/or @dvyukov (TSan).

[dvyukov]

This needs some tests, at least to exercise interceptors code once.

lib/esan/esan_interceptors.cpp

90 ↗

(On Diff #179709)

The idea behind defining a no-op version in sanitizer_common_interceptors.inc was exactly that tools that are not interested in it does not need to be changed. So please remove this.

lib/tsan/rtl/tsan_interceptors.cc

2260

An alternative would be to pass NULL to COMMON_INTERCEPTOR_FILE_OPEN and then do:

if (path)
  Acquire(thr, pc, File2addr(path))

Just to not multiply entities. But neither approach looks strictly better to me, so I am not too strong about it.

[krytarowski]

lib/esan/esan_interceptors.cpp

90 ↗

(On Diff #179709)

I would add an intermediate revision to drop redefinitions in this file.

lib/tsan/rtl/tsan_interceptors.cc

2260

Is File2addr() producing any useful result?

Reusing COMMON_INTERCEPTOR_FILE_OPEN looks fine.

[dvyukov]

lib/tsan/rtl/tsan_interceptors.cc

2260

Is File2addr() producing any useful result?

What exactly do you mean?
It produces a degenerate, but useful result. The original idea was that it can return different results for different files, depending on io_sync flag value. E.g. not synchronizing deletion of one file with open of another file. But this gets tricky with links, bind mounts, etc and was never implemented.
But at least if a file is not involved at all, then we need to skip the Acquire.

[mgorny]

The tests were submitted earlier as D56153. I'll implement the remaining suggestions.

[mgorny]

Updated to use COMMON_INTERCEPTOR_FILE_OPEN with nullptr argument.

[mgorny]

lib/esan/esan_interceptors.cpp

90 ↗

(On Diff #179709)

@krytarowski, apparently it's not that simple. I've tried removing all the empty definitions and now I get a lot of build errors ;-). It would probably be necessary to take them one by one to establish which one are unnecessary after all, and I don't think we have time for this right now.

lib/sanitizer_common/sanitizer_common_interceptors.inc

#define INIT_CDB \
   COMMON_INTERCEPT_FUNCTION(cdbw_put_data); \
   COMMON_INTERCEPT_FUNCTION(cdbw_put_key); \
   COMMON_INTERCEPT_FUNCTION(cdbw_output); \
   COMMON_INTERCEPT_FUNCTION(cdbw_close)
 #else
 #define INIT_CDB
 #endif
 
+#if SANITIZER_INTERCEPT_POPEN
+INTERCEPTOR(__sanitizer_FILE *, popen, const char *command, const char *type) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, popen, command, type);
+  if (command)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, command, REAL(strlen)(command) + 1);
+  if (type)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, type, REAL(strlen)(type) + 1);
+  __sanitizer_FILE *res = REAL(popen)(command, type);
+  COMMON_INTERCEPTOR_FILE_OPEN(ctx, res, nullptr);
+  if (res) unpoison_file(res);
+  return res;
+}
+#define INIT_POPEN COMMON_INTERCEPT_FUNCTION(popen)
+#else
+#define INIT_POPEN
+#endif
+
+#if SANITIZER_INTERCEPT_POPENVE
+INTERCEPTOR(__sanitizer_FILE *, popenve, const char *path,
+            char *const *argv, char *const *envp, const char *type) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, popenve, path, argv, envp, type);
+  if (path)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
+  if (argv) {
+    for (char *const *pa = argv; ; ++pa) {
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, pa, sizeof(char **));
+      if (!*pa)
+        break;
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, *pa, REAL(strlen)(*pa) + 1);
+    }
+  }
+  if (envp) {
+    for (char *const *pa = envp; ; ++pa) {
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, pa, sizeof(char **));
+      if (!*pa)
+        break;
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, *pa, REAL(strlen)(*pa) + 1);
+    }
+  }
+  if (type)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, type, REAL(strlen)(type) + 1);
+  __sanitizer_FILE *res = REAL(popenve)(path, argv, envp, type);
+  COMMON_INTERCEPTOR_FILE_OPEN(ctx, res, nullptr);
+  if (res) unpoison_file(res);
+  return res;
+}
+#define INIT_POPENVE COMMON_INTERCEPT_FUNCTION(popenve)
+#else
+#define INIT_POPENVE
+#endif
+
+#if SANITIZER_INTERCEPT_PCLOSE
+INTERCEPTOR(int, pclose, __sanitizer_FILE *fp) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, pclose, fp);
+  COMMON_INTERCEPTOR_FILE_CLOSE(ctx, fp);
+  const FileMetadata *m = GetInterceptorMetadata(fp);
+  int res = REAL(pclose)(fp);
+  if (m) {
+    COMMON_INTERCEPTOR_INITIALIZE_RANGE(*m->addr, *m->size);
+    DeleteInterceptorMetadata(fp);
+  }
+  return res;
+}
+#define INIT_PCLOSE COMMON_INTERCEPT_FUNCTION(pclose);
+#else
+#define INIT_PCLOSE
+#endif
+
 static void InitializeCommonInterceptors() {
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map =
       new ((void *)&metadata_mem) MetadataHashMap();  // NOLINT
 
   INIT_MMAP;
   INIT_MMAP64;
   INIT_TEXTDOMAIN;
static void InitializeCommonInterceptors() {
   INIT_MD4;
   INIT_RMD160;
   INIT_MD5;
   INIT_FSEEK;
   INIT_MD2;
   INIT_SHA2;
   INIT_VIS;
   INIT_CDB;
+  INIT_POPEN;
+  INIT_POPENVE;
+  INIT_PCLOSE;
 
   INIT___PRINTF_CHK;
 }

lib/sanitizer_common/sanitizer_platform_interceptors.h

 #define SANITIZER_INTERCEPT_RMD160 SI_NETBSD
 #define SANITIZER_INTERCEPT_MD5 SI_NETBSD
 #define SANITIZER_INTERCEPT_FSEEK (SI_NETBSD || SI_FREEBSD)
 #define SANITIZER_INTERCEPT_MD2 SI_NETBSD
 #define SANITIZER_INTERCEPT_SHA2 SI_NETBSD
 #define SANITIZER_INTERCEPT_CDB SI_NETBSD
 #define SANITIZER_INTERCEPT_VIS (SI_NETBSD || SI_FREEBSD)
 
+#define SANITIZER_INTERCEPT_POPEN SI_POSIX
+#define SANITIZER_INTERCEPT_POPENVE SI_NETBSD
+#define SANITIZER_INTERCEPT_PCLOSE SI_POSIX
+
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H

lib/tsan/rtl/tsan_interceptors.cc

 
 #define COMMON_INTERCEPTOR_ENTER_NOIGNORE(ctx, func, ...) \
   SCOPED_INTERCEPTOR_RAW(func, __VA_ARGS__);              \
   TsanInterceptorContext _ctx = {thr, caller_pc, pc};     \
   ctx = (void *)&_ctx;                                    \
   (void) ctx;
 
 #define COMMON_INTERCEPTOR_FILE_OPEN(ctx, file, path) \
-  Acquire(thr, pc, File2addr(path));                  \
+  if (path)                                           \
+    Acquire(thr, pc, File2addr(path));                \
   if (file) {                                         \
     int fd = fileno_unlocked(file);                   \
     if (fd >= 0) FdFileCreate(thr, pc, fd);           \
   }
 
 #define COMMON_INTERCEPTOR_FILE_CLOSE(ctx, file) \
   if (file) {                                    \
     int fd = fileno_unlocked(file);              \
     if (fd >= 0) FdClose(thr, pc, fd);           \
   }
 
 #define COMMON_INTERCEPTOR_LIBRARY_LOADED(filename, handle) \
   libignore()->OnLibraryLoaded(filename)