attempt to make the libc urandom override work for non-glibc too

sandbox/linux/services/libc_urandom_override.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sandbox/linux/services/libc_urandom_override.h"
 
 #include <dlfcn.h>
 #include <pthread.h>
 #include <stdio.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
 #include "base/logging.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/rand_util.h"
 
 // Note: this file is used by the zygote and nacl_helper.
 
+// This is used when mapping stat* calls to __xstat* calls.
+#define XSTAT_VERSION 3
+
 namespace sandbox {
 
 static bool g_override_urandom = false;
 
-void InitLibcUrandomOverrides() {
-  // Make sure /dev/urandom is open.
-  base::GetUrandomFD();
-  g_override_urandom = true;
-}
-
 // TODO(sergeyu): Currently this code doesn't work properly under ASAN
 // - it crashes content_unittests. Make sure it works properly and
 // enable it here. http://crbug.com/123263
 #if !defined(ADDRESS_SANITIZER)
 
 static const char kUrandomDevPath[] = "/dev/urandom";
 
 typedef FILE* (*FopenFunction)(const char* path, const char* mode);
 typedef int (*XstatFunction)(int version, const char *path, struct stat *buf);
 typedef int (*Xstat64Function)(int version, const char *path,
                                struct stat64 *buf);
+typedef int (*StatFunction)(const char *path, struct stat *buf);
+typedef int (*Stat64Function)(const char *path, struct stat64 *buf);
+typedef int (*FxstatFunction)(int version, int fd, struct stat *buf);
+typedef int (*Fxstat64Function)(int version, int fd, struct stat64 *buf);
 
 static pthread_once_t g_libc_file_io_funcs_guard = PTHREAD_ONCE_INIT;
-static FopenFunction g_libc_fopen;
-static FopenFunction g_libc_fopen64;
-static XstatFunction g_libc_xstat;
-static Xstat64Function g_libc_xstat64;
+static FopenFunction g_libc_fopen = NULL;
+static FopenFunction g_libc_fopen64 = NULL;
+static XstatFunction g_libc_xstat = NULL;
+static Xstat64Function g_libc_xstat64 = NULL;
+static StatFunction g_libc_stat = NULL;
+static Stat64Function g_libc_stat64 = NULL;
+static FxstatFunction g_libc_fxstat = NULL;
+static Fxstat64Function g_libc_fxstat64 = NULL;
+
+void InitLibcUrandomOverrides() {
+  // Make sure /dev/urandom is open.

[jln (very slow on Chromium), 2013/06/25 23:00:03]

Calling InitLibcFileIOFunctions() here would guarantee that it's initialized.
Sure, in pratice GetUrandomFD() will stat() and something else will probably
stat() before, so this call will be a no-op in pratice, but the guarantee would
be here.

Then it's much easier to assert, for instance in the Zygote, that
InitLibcFileIOFunctions() will only be called one per browser instance (vs once
per renderer for instance).

So let's add:

CHECK_EQ(0, pthread_once(&g_libc_file_io_funcs_guard,
                         InitLibcFileIOFunctions));

[Mostyn Bramley-Moore, 2013/06/26 10:30:18]

Done.

+  base::GetUrandomFD();
+  g_override_urandom = true;
+}
 
 static void InitLibcFileIOFunctions() {
+  // find the libc's real fopen* and *stat* functions

[jln (very slow on Chromium), 2013/06/25 23:00:03]

Nit: comment above the function, start with a capital, end with a dot.

[Mostyn Bramley-Moore, 2013/06/26 00:09:57]

Done.

+
+  // TODO(mostynb): call this once near the start of the init sequence, don't
+  // call it repeatedly with pthread_once() - ZygoteMain is not early enough.

[jln (very slow on Chromium), 2013/06/25 23:00:03]

In practice it'll only be called once, right?

If pthread_once() isn't readable enough, feel free to add a wrapper
InitLibcFileIOFunctionsIfNecessary() that does the ptrace_once() call instead.

Also document above InitLibcFileIOFunctions() that the function is supposed to
be guarded by g_libc_file_io_funcs_guard.

[Mostyn Bramley-Moore, 2013/06/25 23:40:41]

This function is only called once, what I was trying to say is that pthread_once
is called multiple times.  I thought you wanted this reorganised to avoid the
need for pthread_once...  I'll play around with your suggestions and see if I
can find something that works.

[jln (very slow on Chromium), 2013/06/26 01:57:21]

Yeah, I'm not worried about mutiple calls to pthread_once, that's necessary. I
just think we should guarantee that InitLibcUrandomOverrides() calls
InitLibcFileIOFunctions().

[Mostyn Bramley-Moore, 2013/06/26 10:30:18]

Done in patchset 11.

+
   g_libc_fopen = reinterpret_cast<FopenFunction>(
       dlsym(RTLD_NEXT, "fopen"));
   g_libc_fopen64 = reinterpret_cast<FopenFunction>(
       dlsym(RTLD_NEXT, "fopen64"));
 
   if (!g_libc_fopen) {
     LOG(FATAL) << "Failed to get fopen() from libc.";
   } else if (!g_libc_fopen64) {
 #if !defined(OS_OPENBSD) && !defined(OS_FREEBSD)
     LOG(WARNING) << "Failed to get fopen64() from libc. Using fopen() instead.";
 #endif  // !defined(OS_OPENBSD) && !defined(OS_FREEBSD)
     g_libc_fopen64 = g_libc_fopen;
   }
 
-#if defined(LIBC_GLIBC)
-  // TODO(sergeyu): This works only on systems with glibc. Fix it to
-  // work properly on other systems if necessary.
+  // Note: we attempt to dlsym both stat and __xstat but only expect one
+  // of the calls to succeed (and similarly for the *64 versions).  This
+  // should work with both glibc and uClibc, and hopefully other libc's.
+  // Background: glibc headers inline stat into __xstat which it exports,
+  // while uClibc exports stat and does not implement __xstat.
+
   g_libc_xstat = reinterpret_cast<XstatFunction>(
       dlsym(RTLD_NEXT, "__xstat"));
   g_libc_xstat64 = reinterpret_cast<Xstat64Function>(
       dlsym(RTLD_NEXT, "__xstat64"));
+  g_libc_stat = reinterpret_cast<StatFunction>(
+      dlsym(RTLD_NEXT, "stat"));
+  g_libc_stat64 = reinterpret_cast<Stat64Function>(
+      dlsym(RTLD_NEXT, "stat64"));
+  g_libc_fxstat = reinterpret_cast<FxstatFunction>(
+      dlsym(RTLD_NEXT, "__fxstat"));
+  g_libc_fxstat64 = reinterpret_cast<Fxstat64Function>(
+      dlsym(RTLD_NEXT, "__fxstat64"));
 
-  if (!g_libc_xstat) {
-    LOG(FATAL) << "Failed to get __xstat() from libc.";
+  if (!g_libc_xstat && !g_libc_stat) {
+    LOG(FATAL) << "Neither __xstat nor stat found.";
   }
-  if (!g_libc_xstat64) {
-    LOG(WARNING) << "Failed to get __xstat64() from libc.";
+  if (!g_libc_xstat64 && !g_libc_stat64) {
+    LOG(FATAL) << "Neither __xstat64 nor stat64 found.";
   }
+
+  // Note: it's OK if __fxstat and __fxstat64 aren't found,
+  // we can map them to fstat and fstat64.

[jln (very slow on Chromium), 2013/06/25 23:00:03]

This makes everything way too complicated.

In what configuration is xstat found, but not fxstat ?

[Mostyn Bramley-Moore, 2013/06/25 23:40:41]

In practice, probably none.  As mentioned in my comment below, if we use
fstat/fstat64 instead we would simplify things quite a bit.

[Mostyn Bramley-Moore, 2013/06/26 10:30:18]

Done (removed the fstat* dlsym's now).

 }
 
 // fopen() and fopen64() are intercepted here so that NSS can open
 // /dev/urandom to seed its random number generator. NSS is used by
 // remoting in the sendbox.
 
 // fopen() call may be redirected to fopen64() in stdio.h using
 // __REDIRECT(), which sets asm name for fopen() to "fopen64". This
 // means that we cannot override fopen() directly here. Instead the
 // the code below defines fopen_override() function with asm name
@@ skipping 27 matching lines @@
       return NULL;
     }
     return fdopen(fd, mode);
   } else {
     CHECK_EQ(0, pthread_once(&g_libc_file_io_funcs_guard,
                              InitLibcFileIOFunctions));
     return g_libc_fopen64(path, mode);
   }
 }
 
-// stat() is subject to the same problem as fopen(), so we have to use
-// the same trick to override it.
+// The stat() family of functions are subject to the same problem as
+// fopen(), so we have to use the same trick to override them.
+
 __attribute__ ((__visibility__("default")))
 int xstat_override(int version,
                    const char *path,
                    struct stat *buf)  __asm__ ("__xstat");
 
 __attribute__ ((__visibility__("default")))
 int xstat_override(int version, const char *path, struct stat *buf) {
+  CHECK_EQ(0, pthread_once(&g_libc_file_io_funcs_guard,
+                           InitLibcFileIOFunctions));
+
   if (g_override_urandom && strcmp(path, kUrandomDevPath) == 0) {
-    int result = __fxstat(version, base::GetUrandomFD(), buf);
-    return result;
+    if (g_libc_fxstat) {
+      int result = g_libc_fxstat(version, base::GetUrandomFD(), buf);
+      return result;
+    } else {
+      int result = fstat(base::GetUrandomFD(), buf);
+      return result;

[jln (very slow on Chromium), 2013/06/25 23:00:03]

See my above comment, the logic becomes way too complicated.

[Mostyn Bramley-Moore, 2013/06/25 23:40:41]

It should be safe to simply use fstat directly here instead of a dlsym'ed
__fxstat (and similarly for the *64 versions).  This would remove two dlsyms and
these extra if cases.  Would this be OK with you?

[Mostyn Bramley-Moore, 2013/06/26 10:30:18]

Done (reverted).

+    }
+  }
+  else if (!g_libc_xstat) {
+    return g_libc_stat(path, buf);
   } else {
-    CHECK_EQ(0, pthread_once(&g_libc_file_io_funcs_guard,
-                             InitLibcFileIOFunctions));
     return g_libc_xstat(version, path, buf);
   }
 }
 
 __attribute__ ((__visibility__("default")))
 int xstat64_override(int version,
                      const char *path,
                      struct stat64 *buf)  __asm__ ("__xstat64");
 
 __attribute__ ((__visibility__("default")))
 int xstat64_override(int version, const char *path, struct stat64 *buf) {
+  CHECK_EQ(0, pthread_once(&g_libc_file_io_funcs_guard,
+                           InitLibcFileIOFunctions));
+
   if (g_override_urandom && strcmp(path, kUrandomDevPath) == 0) {
-    int result = __fxstat64(version, base::GetUrandomFD(), buf);
-    return result;
+    if (g_libc_fxstat64) {
+      int result = g_libc_fxstat64(version, base::GetUrandomFD(), buf);
+      return result;
+    } else {
+      int result = fstat64(base::GetUrandomFD(), buf);
+      return result;
+    }
+  }
+  else if (!g_libc_xstat64) {
+    return g_libc_stat64(path, buf);
   } else {
-    CHECK_EQ(0, pthread_once(&g_libc_file_io_funcs_guard,
-                             InitLibcFileIOFunctions));
-    CHECK(g_libc_xstat64);
     return g_libc_xstat64(version, path, buf);
   }
 }
-#endif  // defined(LIBC_GLIBC)
+
+__attribute__ ((__visibility__("default")))
+int stat_override(const char *path,
+                  struct stat *buf)  __asm__ ("stat");
+
+__attribute__ ((__visibility__("default")))
+int stat_override(const char *path, struct stat *buf) {
+  if (g_override_urandom && strcmp(path, kUrandomDevPath) == 0) {
+    int result = fstat(base::GetUrandomFD(), buf);
+    return result;
+  }
+
+  CHECK_EQ(0, pthread_once(&g_libc_file_io_funcs_guard,
+                           InitLibcFileIOFunctions));
+
+  if (!g_libc_stat) {
+    return g_libc_xstat(XSTAT_VERSION, path, buf);
+  } else {
+    return g_libc_stat(path, buf);
+  }
+}
+
+__attribute__ ((__visibility__("default")))
+int stat64_override(const char *path,
+                    struct stat64 *buf)  __asm__ ("stat64");
+
+__attribute__ ((__visibility__("default")))
+int stat64_override(const char *path, struct stat64 *buf) {
+  if (g_override_urandom && strcmp(path, kUrandomDevPath) == 0) {
+    int result = fstat64(base::GetUrandomFD(), buf);
+    return result;
+  }
+
+  CHECK_EQ(0, pthread_once(&g_libc_file_io_funcs_guard,
+                           InitLibcFileIOFunctions));
+
+  if (!g_libc_stat64) {
+    return g_libc_xstat64(XSTAT_VERSION, path, buf);
+  } else {
+    return g_libc_stat64(path, buf);
+  }
+}
 
 #endif  // !defined(ADDRESS_SANITIZER)
 
 }  // namespace content