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

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

[Manolis Ragkousis]

[-- Attachment #1: Type: text/plain, Size: 233 bytes --]

Hello,

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.

Thank you,
Manolis

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Build-sandbox-support-etc.-unconditionally-on-Linux.patch --]
[-- Type: text/x-patch; name="0001-Build-sandbox-support-etc.-unconditionally-on-Linux.patch", Size: 5497 bytes --]

From cb5f4c8d2a01ce32f9b15bf3b41728b36a6738a9 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <eelco.dolstra@logicblox.com>
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%");
-- 
2.6.4

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

[Mark H Weaver]

Manolis Ragkousis <manolis837@gmail.com> 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 <eelco.dolstra@logicblox.com>
> 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%");

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

[Manolis Ragkousis]

Hello Mark,

On 08/10/16 20:39, Mark H Weaver wrote:
>
> 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?

Well the other solution will be to break the CHROOT_ENABLED into smaller
macros, depending on what we check.  If you think this is a better
solution then okay with me.

Locally I broke CHROOT_ENABLED into

#define CHROOT_ENABLED HAVE_CHROOT && HAVE_SYS_MOUNT_H &&
defined(MS_BIND) && defined(MS_PRIVATE)
#define CLONE_ENABLED defined(CLONE_NEWNS)
#if defined(SYS_pivot_root)
#define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root,
put_old))
#endif

And maybe we should rename CHROOT_ENABLED into SANDBOX_ENABLED or
something similar to better describe its purpose.

WDYT?

Thank you for looking into it,
Manolis

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

[Ludovic Courtès]

Hello Manolis!

Manolis Ragkousis <manolis837@gmail.com> skribis:

> On 08/10/16 20:39, Mark H Weaver wrote:
>>
>> 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?
>
> Well the other solution will be to break the CHROOT_ENABLED into smaller
> macros, depending on what we check.  If you think this is a better
> solution then okay with me.
>
> Locally I broke CHROOT_ENABLED into
>
> #define CHROOT_ENABLED HAVE_CHROOT && HAVE_SYS_MOUNT_H &&
> defined(MS_BIND) && defined(MS_PRIVATE)
> #define CLONE_ENABLED defined(CLONE_NEWNS)
> #if defined(SYS_pivot_root)
> #define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root,
> put_old))
> #endif

That sounds preferable, or maybe just avoid CHROOT_ENABLED altogether?

> And maybe we should rename CHROOT_ENABLED into SANDBOX_ENABLED or
> something similar to better describe its purpose.

I’d rather not (Nix made this change, but that’s largely because OS X
has a similar feature called “sandbox”, so they used this name in the
code.)

Ludo’.