Re: [PATCH] Build sandbox support etc. unconditionally on Linux.

[Mark H Weaver]

Manolis Ragkousis <address@hidden> writes:

> This patch is from upstream nix, commit 8f67325, modified to apply to
> our master. It deals with the issue of the CHROOT_ENABLED macro and
> makes my life easier to apply Hurd specific changes to the daemon.

I'm very reluctant to apply this patch.  In general, it's preferable to
rely on autoconf to test for individual features, rather than testing
for particular kernels by name.  It seems to me that this patch will
hinder portability to other kernels.

I'd be inclined to return to the approach you were proposing before
discovering this upstream patch.  I'll take a look at it soon.

What do you think?

      Mark


> From cb5f4c8d2a01ce32f9b15bf3b41728b36a6738a9 Mon Sep 17 00:00:00 2001
> From: Eelco Dolstra <address@hidden>
> Date: Tue, 9 Aug 2016 20:14:54 +0300
> Subject: [PATCH] Build sandbox support etc. unconditionally on Linux.
>
> ---
>  nix/libstore/build.cc       | 40 ++++++++++------------------------------
>  nix/libstore/local-store.cc |  9 ++-------
>  nix/libutil/affinity.cc     | 10 +++++-----
>  3 files changed, 17 insertions(+), 42 deletions(-)
>
> diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
> index ae78e65..3c48e97 100644
> --- a/nix/libstore/build.cc
> +++ b/nix/libstore/build.cc
> @@ -32,36 +32,18 @@
>  #include <bzlib.h>
>  
>  /* Includes required for chroot support. */
> -#if HAVE_SYS_PARAM_H
> -#include <sys/param.h>
> -#endif
> -#if HAVE_SYS_MOUNT_H
> -#include <sys/mount.h>
> -#endif
> -#if HAVE_SYS_SYSCALL_H
> -#include <sys/syscall.h>
> -#endif
> -#if HAVE_SCHED_H
> -#include <sched.h>
> -#endif
> -
> -/* In GNU libc 2.11, <sys/mount.h> does not define `MS_PRIVATE', but
> -   <linux/fs.h> does.  */
> -#if !defined MS_PRIVATE && defined HAVE_LINUX_FS_H
> -#include <linux/fs.h>
> -#endif
> -
> -#define CHROOT_ENABLED HAVE_CHROOT && HAVE_SYS_MOUNT_H && defined(MS_BIND) 
> && defined(MS_PRIVATE) && defined(CLONE_NEWNS) && defined(SYS_pivot_root)
> -
> -#if CHROOT_ENABLED
> +#if __linux__
>  #include <sys/socket.h>
>  #include <sys/ioctl.h>
>  #include <net/if.h>
>  #include <netinet/ip.h>
> -#endif
> -
> -#if __linux__
>  #include <sys/personality.h>
> +#include <sched.h>
> +#include <sys/param.h>
> +#include <sys/mount.h>
> +#include <sys/syscall.h>
> +#include <linux/fs.h>
> +#define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, 
> put_old))
>  #endif
>  
>  #if HAVE_STATVFS
> @@ -1817,7 +1799,7 @@ void DerivationGoal::startBuilder()
>      }
>  
>      if (useChroot) {
> -#if CHROOT_ENABLED
> +#if __linux__
>          /* Create a temporary directory in which we set up the chroot
>             environment using bind-mounts.  We put it in the Nix store
>             to ensure that we can create hard-links to non-directory
> @@ -1998,7 +1980,7 @@ void DerivationGoal::startBuilder()
>         - The UTS namespace ensures that builders see a hostname of
>           localhost rather than the actual hostname.
>      */
> -#if CHROOT_ENABLED
> +#if __linux__
>      if (useChroot) {
>       char stack[32 * 1024];
>       int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | 
> SIGCHLD;
> @@ -2046,7 +2028,7 @@ void DerivationGoal::runChild()
>  
>          commonChildInit(builderOut);
>  
> -#if CHROOT_ENABLED
> +#if __linux__
>          if (useChroot) {
>              /* Initialise the loopback interface. */
>              AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP));
> @@ -2179,10 +2161,8 @@ void DerivationGoal::runChild()
>              if (mkdir("real-root", 0) == -1)
>                  throw SysError("cannot create real-root directory");
>  
> -#define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, 
> put_old))
>              if (pivot_root(".", "real-root") == -1)
>                  throw SysError(format("cannot pivot old root directory onto 
> '%1%'") % (chrootRootDir + "/real-root"));
> -#undef pivot_root
>  
>              if (chroot(".") == -1)
>                  throw SysError(format("cannot change root directory to 
> '%1%'") % chrootRootDir);
> diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc
> index 347e8a7..782e4e8 100644
> --- a/nix/libstore/local-store.cc
> +++ b/nix/libstore/local-store.cc
> @@ -22,16 +22,11 @@
>  #include <time.h>
>  #include <grp.h>
>  
> -#if HAVE_UNSHARE && HAVE_STATVFS && HAVE_SYS_MOUNT_H
> +#if __linux__
>  #include <sched.h>
>  #include <sys/statvfs.h>
>  #include <sys/mount.h>
> -#endif
> -
> -#if HAVE_LINUX_FS_H
> -#include <linux/fs.h>
>  #include <sys/ioctl.h>
> -#include <errno.h>
>  #endif
>  
>  #include <sqlite3.h>
> @@ -501,7 +496,7 @@ void LocalStore::openDB(bool create)
>     bind mount.  So make the Nix store writable for this process. */
>  void LocalStore::makeStoreWritable()
>  {
> -#if HAVE_UNSHARE && HAVE_STATVFS && HAVE_SYS_MOUNT_H && defined(MS_BIND) && 
> defined(MS_REMOUNT)
> +#if __linux__
>      if (getuid() != 0) return;
>      /* Check if /nix/store is on a read-only mount. */
>      struct statvfs stat;
> diff --git a/nix/libutil/affinity.cc b/nix/libutil/affinity.cc
> index 3e21f43..3cbdf87 100644
> --- a/nix/libutil/affinity.cc
> +++ b/nix/libutil/affinity.cc
> @@ -2,14 +2,14 @@
>  #include "util.hh"
>  #include "affinity.hh"
>  
> -#if HAVE_SCHED_H
> +#if __linux__
>  #include <sched.h>
>  #endif
>  
>  namespace nix {
>  
>  
> -#if HAVE_SCHED_SETAFFINITY
> +#if __linux__
>  static bool didSaveAffinity = false;
>  static cpu_set_t savedAffinity;
>  #endif
> @@ -17,7 +17,7 @@ static cpu_set_t savedAffinity;
>  
>  void setAffinityTo(int cpu)
>  {
> -#if HAVE_SCHED_SETAFFINITY
> +#if __linux__
>      if (sched_getaffinity(0, sizeof(cpu_set_t), &savedAffinity) == -1) 
> return;
>      didSaveAffinity = true;
>      printMsg(lvlDebug, format("locking this thread to CPU %1%") % cpu);
> @@ -32,7 +32,7 @@ void setAffinityTo(int cpu)
>  
>  int lockToCurrentCPU()
>  {
> -#if HAVE_SCHED_SETAFFINITY
> +#if __linux__
>      int cpu = sched_getcpu();
>      if (cpu != -1) setAffinityTo(cpu);
>      return cpu;
> @@ -44,7 +44,7 @@ int lockToCurrentCPU()
>  
>  void restoreAffinity()
>  {
> -#if HAVE_SCHED_SETAFFINITY
> +#if __linux__
>      if (!didSaveAffinity) return;
>      if (sched_setaffinity(0, sizeof(cpu_set_t), &savedAffinity) == -1)
>          printMsg(lvlError, "failed to restore affinity %1%");