exec: do not leak session IDs on errors

always cleanup the exec session when the command specified to the "exec" is not found. Closes: #20392 Signed-off-by: Giuseppe Scrivano <[email protected]>
containers · Oct 18, 2023 · fa19e1b · fa19e1b
1 parent 6417104
commit fa19e1b
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 10 deletions.
diff --git a/libpod/container_exec.go b/libpod/container_exec.go
@@ -759,11 +759,19 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi
 // Exec emulates the old Libpod exec API, providing a single call to create,
 // run, and remove an exec session. Returns exit code and error. Exit code is
 // not guaranteed to be set sanely if error is not nil.
-func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resizeChan <-chan resize.TerminalSize, isHealthcheck bool) (int, error) {
+func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resizeChan <-chan resize.TerminalSize, isHealthcheck bool) (exitCode int, retErr error) {
 	sessionID, err := c.ExecCreate(config)
 	if err != nil {
 		return -1, err
 	}
+	defer func() {
+		if err := c.ExecRemove(sessionID, false); err != nil {
+			if retErr == nil && !errors.Is(err, define.ErrNoSuchExecSession) {
+				exitCode = -1
+				retErr = err
+			}
+		}
+	}()
 
 	// Start resizing if we have a resize channel.
 	// This goroutine may likely leak, given that we cannot close it here.
@@ -813,15 +821,7 @@ func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resi
 		}
 		return -1, err
 	}
-	exitCode := session.ExitCode
-	if err := c.ExecRemove(sessionID, false); err != nil {
-		if errors.Is(err, define.ErrNoSuchExecSession) {
-			return exitCode, nil
-		}
-		return -1, err
-	}
-
-	return exitCode, nil
+	return session.ExitCode, nil
 }
 
 // cleanupExecBundle cleanups an exec session after its done

diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
@@ -904,6 +904,7 @@ func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrID s
 
 	// TODO: we should try and retrieve exit code if this fails.
 	if err := ctr.ExecStart(id); err != nil {
+		_ = ctr.ExecRemove(id, true)
 		return "", err
 	}
 	return id, nil

diff --git a/test/system/075-exec.bats b/test/system/075-exec.bats
@@ -166,4 +166,17 @@ load helpers
     run_podman rm -f -t0 $cid
 }
 
+@test "podman exec - does not leak session IDs on invalid command" {
+    run_podman run -d $IMAGE top
+    cid="$output"
+
+    for i in {1..3}; do
+        run_podman 127 exec $cid blahblah
+        run_podman 125 exec -d $cid blahblah
+    done
+
+    run_podman inspect --format "{{len .ExecIDs}}" $cid
+    assert "$output" = "0" ".ExecIDs must be empty"
+}
+
 # vim: filetype=sh