bug#40006: 33/33: daemon: Workaround issues for the Hurd.

[Jan Nieuwenhuizen <janneke@gnu.org>]

Ludovic Courtès writes:

Hello!

> Jan Nieuwenhuizen <janneke@gnu.org> 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?

It seems that they are; I'm running

    sudo ./pre-inst-env guix-daemon --disable-chroot --build-users-group=guixbuild &

and when starting two build processes, I get

    └─sudo(744,root)───guix-daemon(746)─ /

        ─┬─guix-daemon(1484)─┬─guile(1487,guixbuilder01)─ /
         │                   └─guile-2.2(1485)
         └─guix-daemon(1787)─┬─guile(2203,guixbuilder02)─ /

            ──bash(1497)───bash(3964)
            ──make(2512)───gcc(6043)───cc1(6048)

guixbuilder01 is 999, guixbuilder02 is 998 etc.  Does this mean that
root cannot pid.wait() for the builders?  Note that this error does not occur
when building gnu-make-boot0 or diffutils-boot0.

Hmm, after some more playing on the Hurd and our conversation on IRC, I
found that kill -1 simply does not work at the moment.

I copied sysdeps/mach/hurd/kill.c, renamed __kill to debug_kill and
added

--8<---------------cut here---------------start------------->8---
// libc_hidden_def (__kill)
// weak_alias (__kill, kill)

int
main ()
{
  return debug_kill (-1, SIGKILL);
}
--8<---------------cut here---------------end--------------->8---

Running this as a dummy user, it turns out we run

      err = __proc_getpgrppids (proc, - pid, &pids, &npids);

(effectively asking for PIDs in group of PID=1 ??) and returns only one
PID, in my case 5292

--8<---------------cut here---------------start------------->8---
demo@debian:~$ ps -ef -p 5292
    USER   PID  PPID TTY     TIME COMMAND
       -  5292     5   -  0:00.00 /hurd/storeio -Tzero
--8<---------------cut here---------------end--------------->8---

Hmm?  So it seems that kill -1 is currently not supported, or buggy.
I'll look/ask into this some more today.

>> -#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)?

Okay, let's do that for now.

> 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.

I'm assuming that 1.is what Manolis wanted to support with his
libhurdutil?  In fact, I forward ported (minimal effort) the patch

    https://gitlab.com/janneke/hurd/-/commit/856e86f2105417363b85b4d7c4d3141f9e81fb56

but haven't tried linking against this yet.  That would be a nice first
step.  2. sounds fun, but it would need more getting familiar with the
Hurd for me :-)  You never know..

> Either way, it’s a bit of work, so this can definitely come later.

Great!

I have just reset wip-hurd again with new attempts for these too; all
somewhat and more experimental patches are at

    https://gitlab.com/janneke/guix/-/tree/wip-hurd-system

Greetings,
janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com