[PATCH] build: container: Use the same clone flags as fork(3).

[PATCH] build: container: Use the same clone flags as fork(3).

[David Thompson]

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

This patch resolves an issue I was having when working with containers
at the REPL, which means it probably presented undetected issues
elsewhere.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-build-container-Use-the-same-clone-flags-as-fork-3.patch --]
[-- Type: text/x-patch, Size: 2730 bytes --]

From 61ebbe55f7f6d4d4eb42db957d6fc7b4eaf282a6 Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Sat, 5 Sep 2015 14:10:08 -0400
Subject: [PATCH] build: container: Use the same clone flags as fork(3).

The intent is to make 'clone' behave a lot more like 'primitive-fork', which
calls clone(2) with SIGCHLD, CLONE_CHILD_CLEARTID, and CLONE_CHILD_SETTID
flags.  Notably, running 'clone' at the REPL without these flags would break
the REPL beyond repair.

* guix/build/syscalls.scm (CLONE_CHILD_CLEARTID, CLONE_CHILD_SETTID): New
  variables.
* gnu/build/linux-container.scm (namespaces->bit-mask): Add
  CLONE_CHILD_CLEARTID and CLONE_CHILD_SETTID to bit mask.
---
 gnu/build/linux-container.scm |  3 ++-
 guix/build/syscalls.scm       | 16 ++++++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/gnu/build/linux-container.scm b/gnu/build/linux-container.scm
index 4262748..80a8d94 100644
--- a/gnu/build/linux-container.scm
+++ b/gnu/build/linux-container.scm
@@ -162,7 +162,8 @@ host user identifiers to map into the user namespace."
 (define (namespaces->bit-mask namespaces)
   "Return the number suitable for the 'flags' argument of 'clone' that
 corresponds to the symbols in NAMESPACES."
-  (apply logior SIGCHLD
+  ;; Use the same flags as fork(3) in addition to the namespace flags.
+  (apply logior SIGCHLD CLONE_CHILD_CLEARTID CLONE_CHILD_SETTID
          (map (match-lambda
                ('mnt  CLONE_NEWNS)
                ('uts  CLONE_NEWUTS)
diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
index 093eb0a..2c2fbde 100644
--- a/guix/build/syscalls.scm
+++ b/guix/build/syscalls.scm
@@ -50,6 +50,8 @@
             mkdtemp!
             pivot-root
 
+            CLONE_CHILD_CLEARTID
+            CLONE_CHILD_SETTID
             CLONE_NEWNS
             CLONE_NEWUTS
             CLONE_NEWIPC
@@ -303,12 +305,14 @@ string TMPL and return its file name.  TMPL must end with 'XXXXXX'."
         (pointer->string result)))))
 
 ;; Linux clone flags, from linux/sched.h
-(define CLONE_NEWNS   #x00020000)
-(define CLONE_NEWUTS  #x04000000)
-(define CLONE_NEWIPC  #x08000000)
-(define CLONE_NEWUSER #x10000000)
-(define CLONE_NEWPID  #x20000000)
-(define CLONE_NEWNET  #x40000000)
+(define CLONE_CHILD_CLEARTID #x00200000)
+(define CLONE_CHILD_SETTID   #x01000000)
+(define CLONE_NEWNS          #x00020000)
+(define CLONE_NEWUTS         #x04000000)
+(define CLONE_NEWIPC         #x08000000)
+(define CLONE_NEWUSER        #x10000000)
+(define CLONE_NEWPID         #x20000000)
+(define CLONE_NEWNET         #x40000000)
 
 ;; The libc interface to sys_clone is not useful for Scheme programs, so the
 ;; low-level system call is wrapped instead.
-- 
2.5.0


[-- Attachment #3: Type: text/plain, Size: 38 bytes --]


-- 
David Thompson
GPG Key: 0FF1D807

Re: [PATCH] build: container: Use the same clone flags as fork(3).

[Ludovic Courtès]

David Thompson <dthompson2@worcester.edu> skribis:

> This patch resolves an issue I was having when working with containers
> at the REPL, which means it probably presented undetected issues
> elsewhere.

Calling ‘primitive-fork’ at the REPL is not very useful anyway since you
end up with two Guiles trying to read from the same tty.

> From 61ebbe55f7f6d4d4eb42db957d6fc7b4eaf282a6 Mon Sep 17 00:00:00 2001
> From: David Thompson <dthompson2@worcester.edu>
> Date: Sat, 5 Sep 2015 14:10:08 -0400
> Subject: [PATCH] build: container: Use the same clone flags as fork(3).
>
> The intent is to make 'clone' behave a lot more like 'primitive-fork', which
> calls clone(2) with SIGCHLD, CLONE_CHILD_CLEARTID, and CLONE_CHILD_SETTID
> flags.  Notably, running 'clone' at the REPL without these flags would break
> the REPL beyond repair.
>
> * guix/build/syscalls.scm (CLONE_CHILD_CLEARTID, CLONE_CHILD_SETTID): New
>   variables.
> * gnu/build/linux-container.scm (namespaces->bit-mask): Add
>   CLONE_CHILD_CLEARTID and CLONE_CHILD_SETTID to bit mask.

Looking at clone(2) and libc, I’m guessing that without these flags, the
child would have a wrong idea of its thread ID, which in turn may cause
all sorts of problems, right?

LGTM.

Thanks,
Ludo’.

Re: [PATCH] build: container: Use the same clone flags as fork(3).

[Thompson, David]

On Mon, Sep 7, 2015 at 12:13 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> David Thompson <dthompson2@worcester.edu> skribis:
>
>> This patch resolves an issue I was having when working with containers
>> at the REPL, which means it probably presented undetected issues
>> elsewhere.
>
> Calling ‘primitive-fork’ at the REPL is not very useful anyway since you
> end up with two Guiles trying to read from the same tty.

Yes, this is true, but it made it glaringly obvious that something was wrong.

    (match (primitive-fork) (0 (primitive-exit 0)) (pid pid))  ; OK
since it exits immediately

    (match (clone (logior CLONE_NEWUSER SIGCHLD)) (0 (primitive-exit
0)) (pid pid)) ; Broken REPL!

>> From 61ebbe55f7f6d4d4eb42db957d6fc7b4eaf282a6 Mon Sep 17 00:00:00 2001
>> From: David Thompson <dthompson2@worcester.edu>
>> Date: Sat, 5 Sep 2015 14:10:08 -0400
>> Subject: [PATCH] build: container: Use the same clone flags as fork(3).
>>
>> The intent is to make 'clone' behave a lot more like 'primitive-fork', which
>> calls clone(2) with SIGCHLD, CLONE_CHILD_CLEARTID, and CLONE_CHILD_SETTID
>> flags.  Notably, running 'clone' at the REPL without these flags would break
>> the REPL beyond repair.
>>
>> * guix/build/syscalls.scm (CLONE_CHILD_CLEARTID, CLONE_CHILD_SETTID): New
>>   variables.
>> * gnu/build/linux-container.scm (namespaces->bit-mask): Add
>>   CLONE_CHILD_CLEARTID and CLONE_CHILD_SETTID to bit mask.
>
> Looking at clone(2) and libc, I’m guessing that without these flags, the
> child would have a wrong idea of its thread ID, which in turn may cause
> all sorts of problems, right?

Yes, that seems to be the case.  I was always a little suspicious
about not using the same clone flags as fork, and I finally ran into a
case where it made a difference.

> LGTM.

Pushed, thanks!

- Dave