[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: 33/33: daemon: Workaround issues for the Hurd.
From: |
Ludovic Courtès |
Subject: |
Re: 33/33: daemon: Workaround issues for the Hurd. |
Date: |
Wed, 11 Mar 2020 15:50:26 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Hi!
Jan Nieuwenhuizen <address@hidden> skribis:
>>> +#if !__GNU__
>>> int status = pid.wait(true);
>>> if (status != 0)
>>> throw Error(format("cannot kill processes for uid `%1%': %2%") %
>>> uid % statusToString(status));
>>> +#endif
>>
>> Do you know what the rationale was? It looks like it could leave
>> zombies behind us.
>
> No, maybe Manolis knows? What I do know is why I used the patch: before
> applying this patch I could only build up to binutils-boot0.
> binutils-boot0 would always fail like so
>
> ./pre-inst-env guix build -e '(@@ (gnu packages commencement)
> binutils-boot0)' --no-offload
> XXX fails: Workaround for nix daemon
> phase `compress-documentation' succeeded after 0.4 seconds
> error: cannot kill processes for uid `999': Operation not permitted
> guix build: error: cannot kill processes for uid `999': failed with exit code
> 1
But is the build process actually running as UID 999? If you pass
‘--disable-chroot’, then I think build users are not used at all, right?
> From 0307646b22fc488e6342f5814fdef336dd154be3 Mon Sep 17 00:00:00 2001
> From: Manolis Ragkousis <address@hidden>
> Date: Sun, 7 Aug 2016 17:48:30 +0300
> Subject: [PATCH 1/2] daemon: Break CHROOT_ENABLED into smaller macros.
>
> Checking for CLONE_NEWNS is only needed for using tha Linux specific clone(2),
> otherwise we can use fork(2).
>
> * nix/libstore/build.cc (CHROOT_ENABLED): Break into CHROOT_ENABLED
> and CLONE_ENABLED.
> (DerivationGoal::startBuilder): Replace CHROOT_ENABLED with CLONE_ENABLED.
> (DerivationGoal::runChild): Only define pivot_root() if SYS_pivot_root is
> defined.
[...]
> -#define CHROOT_ENABLED HAVE_CHROOT && HAVE_SYS_MOUNT_H && defined(MS_BIND)
> && defined(MS_PRIVATE) && defined(CLONE_NEWNS) && defined(SYS_pivot_root)
> +#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
>
> #if CHROOT_ENABLED
> #include <sys/socket.h>
> @@ -2005,7 +2010,7 @@ void DerivationGoal::startBuilder()
> - The UTS namespace ensures that builders see a hostname of
> localhost rather than the actual hostname.
> */
> -#if CHROOT_ENABLED
> +#if CLONE_ENABLED
> if (useChroot) {
> char stack[32 * 1024];
> int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS |
> SIGCHLD;
I’m not sure this is correct. Perhaps we rather need an “#ifdef
__linux__” around the use of clone(2)?
Other options:
1. Implement clone(2) with CLONE_NEW* in libc on GNU/Hurd.
2. Add a “sandbox” abstraction in the daemon, with OS-specific
implementations of the abstraction (the Nix daemon did that at some
point, with the goal of supporting proprietary macOS etc.)
For GNU/Linux, it’d use chroot(2)+clone(NEWNS) etc. as root.
On GNU/Hurd, it could spawn the process in a sub-Hurd, i.e., with
its own proc server, root file system server, and without a pfinet
server running.
Option #2 can be fun to implement and probably easier and less
controversial than Option #1. However, it does mean adding more code of
the C++ code base, which is sad.
Either way, it’s a bit of work, so this can definitely come later.
Ludo’.