From 4b547a72ab32f86537fec0907c0fbbb98595a688 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 12 Jul 2022 10:27:42 +0200 Subject: [PATCH 1/4] podman: move MaybeMoveToSubCgroup to utils/ Signed-off-by: Giuseppe Scrivano --- cmd/podman/system/service_abi.go | 23 +---------------------- utils/utils.go | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cmd/podman/system/service_abi.go b/cmd/podman/system/service_abi.go index 6823d77ba5ba..8d0240a8dbc0 100644 --- a/cmd/podman/system/service_abi.go +++ b/cmd/podman/system/service_abi.go @@ -11,7 +11,6 @@ import ( "os" "path/filepath" - "github.com/containers/common/pkg/cgroups" "github.com/containers/podman/v4/cmd/podman/registry" api "github.com/containers/podman/v4/pkg/api/server" "github.com/containers/podman/v4/pkg/domain/entities" @@ -24,26 +23,6 @@ import ( "golang.org/x/sys/unix" ) -// maybeMoveToSubCgroup moves the current process in a sub cgroup when -// it is running in the root cgroup on a system that uses cgroupv2. -func maybeMoveToSubCgroup() error { - unifiedMode, err := cgroups.IsCgroup2UnifiedMode() - if err != nil { - return err - } - if !unifiedMode { - return nil - } - cgroup, err := utils.GetOwnCgroup() - if err != nil { - return err - } - if cgroup == "/" { - return utils.MoveUnderCgroupSubtree("init") - } - return nil -} - func restService(flags *pflag.FlagSet, cfg *entities.PodmanConfig, opts entities.ServiceOptions) error { var ( listener net.Listener @@ -125,7 +104,7 @@ func restService(flags *pflag.FlagSet, cfg *entities.PodmanConfig, opts entities return err } - if err := maybeMoveToSubCgroup(); err != nil { + if err := utils.MaybeMoveToSubCgroup(); err != nil { return err } diff --git a/utils/utils.go b/utils/utils.go index 997de150dc13..7cf28fda5819 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -190,3 +190,23 @@ func MovePauseProcessToScope(pausePidPath string) { } } } + +// MaybeMoveToSubCgroup moves the current process in a sub cgroup when +// it is running in the root cgroup on a system that uses cgroupv2. +func MaybeMoveToSubCgroup() error { + unifiedMode, err := cgroups.IsCgroup2UnifiedMode() + if err != nil { + return err + } + if !unifiedMode { + return nil + } + cgroup, err := GetOwnCgroup() + if err != nil { + return err + } + if cgroup == "/" { + return MoveUnderCgroupSubtree("init") + } + return nil +} From 16b8d77f9ef5a06b90d1507fff1f9c21fbbcd0cb Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 12 Jul 2022 10:55:39 +0200 Subject: [PATCH 2/4] utils: call MaybeMoveToSubCgroup once memoize its result and use it for subsequent calls. Signed-off-by: Giuseppe Scrivano --- utils/utils.go | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index 7cf28fda5819..aa1c6a958fb8 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -191,22 +191,32 @@ func MovePauseProcessToScope(pausePidPath string) { } } +var ( + maybeMoveToSubCgroupSync sync.Once + maybeMoveToSubCgroupSyncErr error +) + // MaybeMoveToSubCgroup moves the current process in a sub cgroup when // it is running in the root cgroup on a system that uses cgroupv2. func MaybeMoveToSubCgroup() error { - unifiedMode, err := cgroups.IsCgroup2UnifiedMode() - if err != nil { - return err - } - if !unifiedMode { - return nil - } - cgroup, err := GetOwnCgroup() - if err != nil { - return err - } - if cgroup == "/" { - return MoveUnderCgroupSubtree("init") - } - return nil + maybeMoveToSubCgroupSync.Do(func() { + unifiedMode, err := cgroups.IsCgroup2UnifiedMode() + if err != nil { + maybeMoveToSubCgroupSyncErr = err + return + } + if !unifiedMode { + maybeMoveToSubCgroupSyncErr = nil + return + } + cgroup, err := GetOwnCgroup() + if err != nil { + maybeMoveToSubCgroupSyncErr = err + return + } + if cgroup == "/" { + maybeMoveToSubCgroupSyncErr = MoveUnderCgroupSubtree("init") + } + }) + return maybeMoveToSubCgroupSyncErr } From 7b4afbf621a787ead00ae83bdaebabeec3b0c707 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 12 Jul 2022 11:47:30 +0200 Subject: [PATCH 3/4] podman: always call into SetupRootless Signed-off-by: Giuseppe Scrivano --- cmd/podman/common/completion.go | 3 +-- cmd/podman/root.go | 3 +-- pkg/domain/infra/abi/system.go | 4 ++++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index 6e6c33f9b78f..02369c74a399 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -18,7 +18,6 @@ import ( "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/pkg/domain/entities" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/signal" systemdDefine "github.com/containers/podman/v4/pkg/systemd/define" "github.com/containers/podman/v4/pkg/util" @@ -54,7 +53,7 @@ func setupContainerEngine(cmd *cobra.Command) (entities.ContainerEngine, error) cobra.CompErrorln(err.Error()) return nil, err } - if !registry.IsRemote() && rootless.IsRootless() { + if !registry.IsRemote() { _, noMoveProcess := cmd.Annotations[registry.NoMoveProcess] err := containerEngine.SetupRootless(registry.Context(), noMoveProcess) diff --git a/cmd/podman/root.go b/cmd/podman/root.go index f28d92e2f119..0520a0784f73 100644 --- a/cmd/podman/root.go +++ b/cmd/podman/root.go @@ -19,7 +19,6 @@ import ( "github.com/containers/podman/v4/pkg/checkpoint/crutils" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/parallel" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/version" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -265,7 +264,7 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { // 2) running as non-root // 3) command doesn't require Parent Namespace _, found := cmd.Annotations[registry.ParentNSRequired] - if !registry.IsRemote() && rootless.IsRootless() && !found { + if !registry.IsRemote() && !found { _, noMoveProcess := cmd.Annotations[registry.NoMoveProcess] err := registry.ContainerEngine().SetupRootless(registry.Context(), noMoveProcess) if err != nil { diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index 0faae01c8c06..eed80dd792c8 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -67,6 +67,10 @@ func (ic *ContainerEngine) Info(ctx context.Context) (*define.Info, error) { } func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) error { + if !rootless.IsRootless() { + return nil + } + // do it only after podman has already re-execed and running with uid==0. hasCapSysAdmin, err := unshare.HasCapSysAdmin() if err != nil { From e3419c03245c5639d457cb27f4081cee400f3a36 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 12 Jul 2022 11:12:27 +0200 Subject: [PATCH 4/4] abi: create new cgroup when running in a container if podman is running in the root cgroup, it will create a new subcgroup and move itself there. [NO NEW TESTS NEEDED] it needs nested podman Closes: https://github.com/containers/podman/issues/14884 Signed-off-by: Giuseppe Scrivano --- pkg/domain/infra/abi/system.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index eed80dd792c8..3389abd8844a 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -67,6 +67,18 @@ func (ic *ContainerEngine) Info(ctx context.Context) (*define.Info, error) { } func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) error { + runsUnderSystemd := utils.RunsOnSystemd() + if !runsUnderSystemd { + isPid1 := os.Getpid() == 1 + if _, found := os.LookupEnv("container"); isPid1 || found { + if err := utils.MaybeMoveToSubCgroup(); err != nil { + // it is a best effort operation, so just print the + // error for debugging purposes. + logrus.Debugf("Could not move to subcgroup: %v", err) + } + } + } + if !rootless.IsRootless() { return nil } @@ -86,7 +98,6 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) if err != nil { return err } - runsUnderSystemd := utils.RunsOnSystemd() unitName := fmt.Sprintf("podman-%d.scope", os.Getpid()) if runsUnderSystemd || conf.Engine.CgroupManager == config.SystemdCgroupsManager { if err := utils.RunUnderSystemdScope(os.Getpid(), "user.slice", unitName); err != nil {