From 5110bd2fc026c0a9849e35322d5b81a3f28c716d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 26 Jul 2021 10:31:06 -0700 Subject: [PATCH] nsenter: remove cgroupns sync mechanism As pointed out in TODO item added by commit 64bb59f59, it is not necessary to have a special sync mechanism for cgroupns, as the parent adds runc init to cgroup way earlier (before sending nl bootstrap data. This sync was added by commit df3fa115f974, which was also added a second cgroup manager.Apply() call, later removed in commit d1ba8e39f8c7512. It seems the original author had the idea to wait for that second Apply(). Fixes: df3fa115f974 Signed-off-by: Kir Kolyshkin --- libcontainer/nsenter/nsexec.c | 25 ++----------------------- libcontainer/process_linux.go | 11 ----------- 2 files changed, 2 insertions(+), 34 deletions(-) diff --git a/libcontainer/nsenter/nsexec.c b/libcontainer/nsenter/nsexec.c index 8199faf3a..49e8f54b6 100644 --- a/libcontainer/nsenter/nsexec.c +++ b/libcontainer/nsenter/nsexec.c @@ -41,12 +41,6 @@ enum sync_t { SYNC_CHILD_FINISH = 0x45, /* The child or grandchild has finished. */ }; -/* - * Synchronisation value for cgroup namespace setup. - * The same constant is defined in process_linux.go as "createCgroupns". - */ -#define CREATECGROUPNS 0x80 - #define STAGE_SETUP -1 /* longjmp() arguments. */ #define STAGE_PARENT 0 @@ -1075,24 +1069,9 @@ void nsexec(void) bail("setgroups failed"); } - /* - * Wait until our topmost parent has finished cgroup setup in - * p.manager.Apply(). - * - * TODO(cyphar): Check if this code is actually needed because we - * should be in the cgroup even from stage-0, so - * waiting until now might not make sense. - */ if (config.cloneflags & CLONE_NEWCGROUP) { - uint8_t value; - if (read(pipenum, &value, sizeof(value)) != sizeof(value)) - bail("read synchronisation value failed"); - if (value == CREATECGROUPNS) { - write_log(DEBUG, "unshare cgroup namespace"); - if (unshare(CLONE_NEWCGROUP) < 0) - bail("failed to unshare cgroup namespace"); - } else - bail("received unknown synchronisation value"); + if (unshare(CLONE_NEWCGROUP) < 0) + bail("failed to unshare cgroup namespace"); } write_log(DEBUG, "signal completion to stage-0"); diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 490a7c8d7..e1649876b 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -25,10 +25,6 @@ import ( "golang.org/x/sys/unix" ) -// Synchronisation value for cgroup namespace setup. -// The same constant is defined in nsexec.c as "CREATECGROUPNS". -const createCgroupns = 0x80 - type parentProcess interface { // pid returns the pid for the running process. pid() int @@ -411,13 +407,6 @@ func (p *initProcess) start() (retErr error) { } p.setExternalDescriptors(fds) - // Now it's time to setup cgroup namesapce - if p.config.Config.Namespaces.Contains(configs.NEWCGROUP) && p.config.Config.Namespaces.PathOf(configs.NEWCGROUP) == "" { - if _, err := p.messageSockPair.parent.Write([]byte{createCgroupns}); err != nil { - return fmt.Errorf("error sending synchronization value to init process: %w", err) - } - } - // Wait for our first child to exit if err := p.waitForChildExit(childPid); err != nil { return fmt.Errorf("error waiting for our first child to exit: %w", err)