Fix possible rootless netns cleanup race

rootlessNetNS.Cleanup() has an issue with how it detects if cleanup
is needed, reading the container state is not good ebough because
containers are first stopped and than cleanup will be called. So at one
time two containers could wait for cleanup but the second one will fail
because the first one triggered already the cleanup thus making rootless
netns unavailable for the second container resulting in an teardown
error. Instead of checking the container state we need to check the
netns state.

Secondly, podman unshare --rootless-netns should not do the cleanup.
This causes more issues than it is worth fixing. Users also might want
to use this to setup the namespace in a special way. If unshare also
cleans this up right away we cannot do this.

[NO NEW TESTS NEEDED]

Fixes #12459

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

1 files changed, 13 insertions, 13 deletions

diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index 7d1214183..b734b9c95 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -322,17 +322,14 @@ func (r *RootlessNetNS) Do(toRun func() error) error {
 
 // Cleanup the rootless network namespace if needed.
 // It checks if we have running containers with the bridge network mode.
-// Cleanup() will try to lock RootlessNetNS, therefore you have to call
-// it with an unlocked lock.
+// Cleanup() expects that r.Lock is locked
 func (r *RootlessNetNS) Cleanup(runtime *Runtime) error {
 	_, err := os.Stat(r.dir)
 	if os.IsNotExist(err) {
 		// the directory does not exists no need for cleanup
 		return nil
 	}
-	r.Lock.Lock()
-	defer r.Lock.Unlock()
-	running := func(c *Container) bool {
+	activeNetns := func(c *Container) bool {
 		// no bridge => no need to check
 		if !c.config.NetMode.IsBridge() {
 			return false
@@ -352,15 +349,18 @@ func (r *RootlessNetNS) Cleanup(runtime *Runtime) error {
 			return false
 		}
 
-		state := c.state.State
-		return state == define.ContainerStateRunning
+		// only check for an active netns, we cannot use the container state
+		// because not running does not mean that the netns does not need cleanup
+		// only if the netns is empty we know that we do not need cleanup
+		return c.state.NetNS != nil
 	}
-	ctrs, err := runtime.GetContainersWithoutLock(running)
+	ctrs, err := runtime.GetContainersWithoutLock(activeNetns)
 	if err != nil {
 		return err
 	}
-	// no cleanup if we found containers
-	if len(ctrs) > 0 {
+	// no cleanup if we found no other containers with a netns
+	// we will always find one container (the container cleanup that is currently calling us)
+	if len(ctrs) > 1 {
 		return nil
 	}
 	logrus.Debug("Cleaning up rootless network namespace")
@@ -809,10 +809,10 @@ func (r *Runtime) teardownNetwork(ns string, opts types.NetworkOptions) error {
 	if rootlessNetNS != nil {
 		// execute the cni setup in the rootless net ns
 		err = rootlessNetNS.Do(tearDownPod)
-		rootlessNetNS.Lock.Unlock()
-		if err == nil {
-			err = rootlessNetNS.Cleanup(r)
+		if cerr := rootlessNetNS.Cleanup(r); cerr != nil {
+			logrus.WithError(err).Error("failed to cleanup rootless netns")
 		}
+		rootlessNetNS.Lock.Unlock()
 	} else {
 		err = tearDownPod()
 	}