[Bug libc/26371] New: [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[Bug libc/26371] New: [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[mztyvop at 0pointer dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

            Bug ID: 26371
           Summary: [RFE] please add clone3() wrapper (in particular the
                    CLONE_INTO_CGROUP feature of it)
           Product: glibc
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: mztyvop at 0pointer dot net
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

In systemd we'd really like to make use of the new CLONE_INTO_CGROUP kernel
feature available for clone3(). It would allows us removing some really ugly
code in systemd: right now whenever PID 1 forks off a service process we
manually migrate the process into the destination cgroup and we do this twice
(once from the parent/PID 1 and once from the child, since we need to be sure
the migration is complete before we continue in either). 

Unfortunately I don't see how we can properly make use of this in systemd,
without glibc providing some form of support for clone3() for us.

The problem is that if we issue clone3() with our own wrapper, glibc won't know
about the call. Then, when in the child we issue setgroups(), setuid() and so
on, the setxuid propagation stuff in glibc tries to propagate these credential
changes to other threads — threads that don't actually exist in the child...

Since issuing setuid() in some form is very typical during service invocation
this means this pretty systematically deadlocks if we use clone3(). And that
sucks.

I am not aware of any work-around for this: we cannot tell glibc that a
clone/fork has happened, we cannot tell glibc that all threads that existed in
the parent suddenly vanished. 

Hence, it would be great if glibc would gain a clone3() wrapper. Or
alternatively some way how we can turn off setxid propagation in the child? Or
some way to tell glibc that a clone/fork/clone3 happened and it should forget
about all its thread info?

Here's a systemd PR that implements things in systemd with a manual wrapper for
clone3(), and that then triggers the setxid deadlock as mentioned in the child:

https://github.com/systemd/systemd/pull/16706

Ideas, suggestions how to deal with this with current glibc would be very
welcome. I'd be quite sad if we'd have to wait for glibc() to wrap clone3()
natively to make use of this new concept.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[christian.brauner at ubuntu dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

Christian Brauner <christian.brauner at ubuntu dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|drepper.fsp at gmail dot com       |carlos at redhat dot com,
                   |                            |christian.brauner at ubuntu dot co
                   |                            |m, fweimer at redhat dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

--- Comment #1 from Florian Weimer <fweimer at redhat dot com> ---
My understanding if this issue is that the clone3 wrapper will not actually
help you that much.

I suspect what you actually need is a way to perform certain system calls after
calling clone3. The newly created userspace thread will eventually call execve,
but it is *not* expected to replace the original process (unlike calling execve
from a full libc thread created by pthread_create).

Can you describe in a bit more detail what you need? To what extent to do you
need to share address space? Are you looking for a vfork-style clone? It makes
things simpler because you could share the stack, and error reporting could use
shared memory.

Which system calls do you need to call?

Would you be able to call different functions than the usual system call
wrappers to perform the tasks you need?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[christian.brauner at ubuntu dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #2 from Christian Brauner <christian.brauner at ubuntu dot com> ---
Please also note that there will be a session at Linux Plumbers on Friday where
we can discuss some of this:

https://linuxplumbersconf.org/event/7/contributions/840/

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[mztyvop at 0pointer dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #3 from Lennart Poettering <mztyvop at 0pointer dot net> ---
> My understanding if this issue is that the clone3 wrapper will not actually help you that much.

It would, if it was a wrapper like fork(), i.e. that it exposes this
return-twice behaviour. It would be much less useful if it was like glibc's
existing clone() wrapper, which hides that, or even worse if it was like
posix_spawn(), that gives us no chance to change process attributes in the
child between the clone3() and the execve().

> I suspect what you actually need is a way to perform certain system calls after calling clone3. The newly created userspace thread will eventually call execve, but it is *not* expected to replace the original process (unlike calling execve from a full libc thread created by pthread_create).

Correct.

> Can you describe in a bit more detail what you need? To what extent to do you need to share address space? Are you looking for a vfork-style clone? It makes things simpler because you could share the stack, and error reporting could use shared memory.

vfork() would not really suffice. We do NSS stuff (getpwnam() and friends) in
the child before we execve(), and that's blocking, but we really can't block
PID 1. In fact we do various other blocking things too there, that we
explicitly chose to do in the child so not to block PID 1.

> Which system calls do you need to call?

Many. The obvious ones are process attributes, such as nice levels, other
scheduling params, prctl, and so on. Then there's the per-process and
per-cgroup stuff exposed in the fs. For that we use open() and related calls. 
keyring stuff, selinux and other MAC stuff. fs namespacing stuff, seccomp
setup. There's some IPC going on, hidden behind NSS and in some form even in
our code. In some cases we ask a question via /dev/console (for confirmation,
if the user asks for interactive boot-up). We also set up some per-service dirs
if that's configured. Also, if people specify RootImage= we'll attach a
loopback block device and mount a file system off it as chroot() env.

We don't do threads in the child process between fork() and execve(), but
pretty much everything else ends up being on the table I guess.

> Would you be able to call different functions than the usual system call wrappers to perform the tasks you need?

Well, we invoke NSS to resolve user names and groups, and we don't know what
might be behind that...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[mztyvop at 0pointer dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #4 from Lennart Poettering <mztyvop at 0pointer dot net> ---
(I'd not be totally opposed to the vfork()-style stuff btw. In some cases the
child might stick around for quite a while between the clone and the execve,
because of blocking NSS or other blocking stuff, as mentioned. It's a bit of a
cow-trap due to that in particular as during boot-up we fork off a lot of these
children very quickly. I hence wondered whether to rework the logic in systemd:
instead of doing the whole shebang of service setup before execve() in the
child, maybe we should insert another binary in between, i.e. an "executor"
program that receives what it is supposed to do in JSON serialized form or so,
then does NSS and whatnot, and then finally execve()'s the final program. In
that model, we'd fork off a child and then have two execve()'s before the final
program is executed, and the "middle" program would be responsible for all the
blocking stuff. In this model having vfork() would be fine I guess (or even
posix_spawn()) given that we'd spend much less time between the vfork() and the
first execve(), and definitely nothing doing blocking IO. That all said, I am
not sure whether such a design change would actually slow us down too much,
i.e. doing two execve()'s for each invoked user binary instead of one might be
more expensive than the cow trap, dunno. So far I was too lazy to do any
peformance analysis there)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[mztyvop at 0pointer dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #5 from Lennart Poettering <mztyvop at 0pointer dot net> ---
> Please also note that there will be a session at Linux Plumbers on Friday where > we can discuss some of this:
> https://linuxplumbersconf.org/event/7/contributions/840/

I only work part-time currently, and in particular not in the mornings. (Also,
I have no LPC ticket…) Hence I cannot really attend this one. Sorry!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #6 from Florian Weimer <fweimer at redhat dot com> ---
(In reply to Lennart Poettering from comment #3)
> vfork() would not really suffice. We do NSS stuff (getpwnam() and friends)
> in the child before we execve(), and that's blocking, but we really can't
> block PID 1. In fact we do various other blocking things too there, that we
> explicitly chose to do in the child so not to block PID 1.

Do you plan to make PID 1 multi-threaded at any point?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[mztyvop at 0pointer dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #7 from Lennart Poettering <mztyvop at 0pointer dot net> ---
So we do employ threads, but in very limited fashion. Primarily there are four
uses for threads in PID 1 right now: 

1. We need an async close() sometimes, for fds that people pass to us via D-Bus
and similar, since they can send us arbitrary stuff, i.e. dead NFS or FUSE fd
handles where close() takes ages to finish. For "dangerous" fds like that we
fork off small threads that do nothing but invoke close().

2. Some ioctls (in particular autofs related) are blocking, but we want to
integrate them into our usual event loop. So we run minimal threads for them.
This is used in the .automount support in systemd.

3. When we notice that a shutdown operation is initiated we fork off a thread
that just calls sync() in order to start flushing out buffers early on without
having to wait for it needlessly.

4. When a service shuts down that has RuntimeDirectory= set we do the
equivalent of "rm -rf" in a background thread to get rid of the directory
again, since apparently people sometimes have ridiculously large runtime dirs,
that take ages to clear out. Similar if people have PrivateTmp= set, we empty
out the private /tmp/ and /var/tmp/ subdirs when services go down, the same
way.

In general we try to avoid threads though, and when we have them we try to
minimize them, i.e. they do simple, ideally single operations only. The "rm
-rf" thing is by far the most complex.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #8 from Florian Weimer <fweimer at redhat dot com> ---
(In reply to Lennart Poettering from comment #7)
> So we do employ threads, but in very limited fashion. Primarily there are
> four uses for threads in PID 1 right now: 

In this case, after fork, you must only call async-signal-safe functions, so
getpwnam is definitely out. This is a requirement glibc inherits from POSIX.
Any clone3 system call wrapper would have at least this restriction (and likely
others).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[crrodriguez at opensuse dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

Cristian Rodríguez <crrodriguez at opensuse dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |crrodriguez at opensuse dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[bluca at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

Luca Boccassi <bluca at debian dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bluca at debian dot org

--- Comment #9 from Luca Boccassi <bluca at debian dot org> ---
Hi, are there any news on this? We are looking again at using clone3 in
systemd, but it seems there is a lot of hairiness around prepping the stack
before it can be used. Was there any progress in adding a wrapper in glibc?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[bluca at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #10 from Luca Boccassi <bluca at debian dot org> ---
Note that we will no longer need to do any work/syscalls in the child at all,
apart from opening and immediately execv'ing another program, that will do all
the setup that were mentioned earlier in the thread.

So what we really need is 'just' a wrapper that sets up the stack correctly,
like the clone() wrapper does.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[carlos at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #11 from Carlos O'Donell <carlos at redhat dot com> ---
(In reply to Luca Boccassi from comment #9)
> Hi, are there any news on this? We are looking again at using clone3 in
> systemd, but it seems there is a lot of hairiness around prepping the stack
> before it can be used. Was there any progress in adding a wrapper in glibc?

(In reply to Luca Boccassi from comment #10)
> Note that we will no longer need to do any work/syscalls in the child at
> all, apart from opening and immediately execv'ing another program, that will
> do all the setup that were mentioned earlier in the thread.
> 
> So what we really need is 'just' a wrapper that sets up the stack correctly,
> like the clone() wrapper does.

Some progress, we need assembly wrappers for all clone3() calls first, and
there is ongoing review for adding an internal-clone3 wrapper for use with
posix_spawn [1], but nothing proposed to be exported as an external interface
like requested here.

At least when we complete the internal assembly calling sequences it should be
easier to mock up a prototype solution.

We owe the community a clone3() syscall wrapper because it can't be used
without coordination with glibc and we should not block users from being able
to use the functionality directly.

The consensus case for adding syscall wrappers holds true [2].

Originally in this bug we were talking about a fork-like wrapper, but it sounds
like you don't need that now, which makes it simple to implement.

[1] https://patchwork.sourceware.org/project/glibc/list/?series=16730
[2] https://sourceware.org/glibc/wiki/Consensus#WIP:_Kernel_syscalls_wrappers

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[sam at gentoo dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

Sam James <sam at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sam at gentoo dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[bluca at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #12 from Luca Boccassi <bluca at debian dot org> ---
In systemd we are also interested in posix_spawn, but there's 3 features we'd
need added:

1) spawn-by-fd version that takes file descriptor instead of path
2) CLONE_INTO_CGROUP support
3) support for returning PID FD instead/together with PID

Is this something that would be possible to implement? Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |adhemerval.zanella at linaro dot o
                   |                            |rg

--- Comment #13 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to Luca Boccassi from comment #12)
> In systemd we are also interested in posix_spawn, but there's 3 features
> we'd need added:
> 
> 1) spawn-by-fd version that takes file descriptor instead of path
> 2) CLONE_INTO_CGROUP support
> 3) support for returning PID FD instead/together with PID
> 
> Is this something that would be possible to implement? Thanks!

I have been working on extending fork and posix_spawn to support PIDFD (BZ
30349) and I think these interfaces might be the way to provide
CLONE_INTO_CGROUP and/or any clone3 extension. These are Linux-specific and
provide wrapper over clone calls.

I added pidfd_fork in a way it can be a superset of both fork and _Fork, and it
should be simple to extend to get a new argument: if this has a non-negative
value it will pass CLONE_INTO_CGROUP along with the group file descriptor.  Now
that clone3 is an extensible interface, I wonder if glibc should also follow a
similar approach (mainly to avoid the need to keep adding versioned symbols for
any additional new clone3 support).

posix_spawn accomplishes it by using a posix_spawnattr_t along with
init/destroy interface and glibc implementation adds some slack space for
possible extensions. Maybe we can add similar support for pidfd_fork, so
instead of named arguments, it uses a sized struct with input/output values.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[bluca at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #14 from Luca Boccassi <bluca at debian dot org> ---
Yes, adding an extensible interface sounds like a good idea.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #15 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
The Add posix_spawnattr_{get, set}cgroup_np added on 2.39 adds support for
posix_spawn/pidfd_spawn.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[bluca at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

--- Comment #16 from Luca Boccassi <bluca at debian dot org> ---
Great news, thank you for taking care of that!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[bluca at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

Luca Boccassi <bluca at debian dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #17 from Luca Boccassi <bluca at debian dot org> ---
To close the loop, we are now using pidfd_spawn() in systemd when running with
glibc 2.39+. Thanks again!

https://github.com/systemd/systemd/commit/2e106312e2e8e8f88516039446abcf7d9afdaff2

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/26371] [RFE] please add clone3() wrapper (in particular the CLONE_INTO_CGROUP feature of it)

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26371

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |2.39

-- 
You are receiving this mail because:
You are on the CC list for the bug.