From e1caf80e814a2e0ed026706f0a16371ca40dbc26 Mon Sep 17 00:00:00 2001
From: Paul Holzinger <pholzing@redhat.com>
Date: Thu, 18 Jul 2024 14:08:11 +0200
Subject: [PATCH] podman ps: fix racy pod name query

The pod name was queried without holding the container lock, thus it was
possible that the pod was deleted in the meantime and podman just failed
with "no such pod" as the errors.Is() check matched the wrong error.

Move it into the locked code this should prevent anyone from removing
the pod while the container is part of it. Also fix the returned error,
there is no reason to special case one specific error just wrap any error
here so callers at least know where it happened. However this is not
good enough because the batch doesn't update the state which means it
see everything before the container was locked. In this case it might be
possible the ctr and pod was already removed so let the caller skip both
ctr and pod removed errors.

Fixes #23282

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 pkg/ps/ps.go | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go
index 243d238715..755e04c08f 100644
--- a/pkg/ps/ps.go
+++ b/pkg/ps/ps.go
@@ -76,7 +76,8 @@ func GetContainerLists(runtime *libpod.Runtime, options entities.ContainerListOp
 	for _, con := range cons {
 		listCon, err := ListContainerBatch(runtime, con, options)
 		switch {
-		case errors.Is(err, define.ErrNoSuchCtr):
+		// ignore both no ctr and no such pod errors as it means the ctr is gone now
+		case errors.Is(err, define.ErrNoSuchCtr), errors.Is(err, define.ErrNoSuchPod):
 			continue
 		case err != nil:
 			return nil, err
@@ -148,6 +149,7 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
 		networks                                []string
 		healthStatus                            string
 		restartCount                            uint
+		podName                                 string
 	)
 
 	batchErr := ctr.Batch(func(c *libpod.Container) error {
@@ -201,10 +203,6 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
 			return err
 		}
 
-		if !opts.Size && !opts.Namespace {
-			return nil
-		}
-
 		if opts.Namespace {
 			ctrPID := strconv.Itoa(pid)
 			cgroup, _ = getNamespaceInfo(filepath.Join("/proc", ctrPID, "ns", "cgroup"))
@@ -231,6 +229,14 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
 			size.RootFsSize = rootFsSize
 			size.RwSize = rwSize
 		}
+
+		if opts.Pod && len(conConfig.Pod) > 0 {
+			podName, err = rt.GetPodName(conConfig.Pod)
+			if err != nil {
+				return fmt.Errorf("could not find container %s pod (id %s) in state: %w", conConfig.ID, conConfig.Pod, err)
+			}
+		}
+
 		return nil
 	})
 	if batchErr != nil {
@@ -256,6 +262,7 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
 		Networks:     networks,
 		Pid:          pid,
 		Pod:          conConfig.Pod,
+		PodName:      podName,
 		Ports:        portMappings,
 		Restarts:     restartCount,
 		Size:         size,
@@ -263,16 +270,6 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
 		State:        conState.String(),
 		Status:       healthStatus,
 	}
-	if opts.Pod && len(conConfig.Pod) > 0 {
-		podName, err := rt.GetPodName(conConfig.Pod)
-		if err != nil {
-			if errors.Is(err, define.ErrNoSuchCtr) {
-				return entities.ListContainer{}, fmt.Errorf("could not find container %s pod (id %s) in state: %w", conConfig.ID, conConfig.Pod, define.ErrNoSuchPod)
-			}
-			return entities.ListContainer{}, err
-		}
-		ps.PodName = podName
-	}
 
 	if opts.Namespace {
 		ps.Namespaces = entities.ListContainerNamespaces{