libpod: fix data race on deferredErr in attachExecHTTP

[crawfordxx]

Description

ExecContainerHTTP returns attachChan to its caller before attachExecHTTP has finished. The caller holds a deferred close(holdConnOpen) that fires when the calling function returns — which can happen while attachExecHTTP is still running through its own deferred cleanup functions.

This creates a data race: the goroutine inside attachExecHTTP reads the named return deferredErr after <-holdConnOpen unblocks, but attachExecHTTP may still be assigning deferredErr (directly or via deferred functions) at that moment. Concurrent access to an interface value without synchronization is unsound per the Go memory model and can produce torn reads (non-nil type with nil data pointer), leading to the nil-pointer panic observed in CI:

panic: runtime error: invalid memory address or nil pointer dereference
net.(*OpError).Unwrap(0x2c31560?)
errors.is({0x1f6eee0?, 0x0?}, ...)   ← interface type set, data nil
github.com/containers/podman/v6/libpod.hijackWriteError(...)
github.com/containers/podman/v6/libpod.attachExecHTTP.func3()

Fix

Add a done channel that is closed by the first-registered (last-to-run) deferred function in attachExecHTTP, i.e. after every other cleanup defer has completed. The goroutine now waits for both <-holdConnOpen and <-done before reading deferredErr. This establishes a proper happens-before relationship between the final write to deferredErr and the goroutine's read.

Fixes #28277

Testing

The race is intermittent and only reproducible under CI load. The fix is provably correct by Go memory model analysis: close(done) happens-after the last write to deferredErr, and <-done happens-before the goroutine's read.

[Honny1]

Thanks for the contribution! The fix looks correct to me. But I have a question: Did you consider using a sync.Mutex to protect deferredErr instead of a channel? Could you explain why you went with the channel approach?

Adding No New Test Label.

[crawfordxx]

The channel approach fits the problem more naturally here. is written by multiple deferred functions in (directly, and through ) — protecting those with a would mean adding lock/unlock around every assignment site scattered across the deferred stack, which is fragile.

More importantly, what we actually need is not mutual exclusion but completion signaling: the goroutine should not read until all cleanup code in has finished running. A channel closed by the first-registered (last-to-run) defer precisely expresses that: close(done) happens-after every other deferred write, and <-done happens-before the read. This directly maps to the Go memory model's happens-before guarantee we need.

A sync.WaitGroup would also work, but it is more verbose for a single-event signal. A sync.Mutex would work too, but it would require touching every write site and doesn't convey the intent as clearly.

[Honny1]

Thanks, LGTM.

[giuseppe]

can we use a chan to pass the error instead? So it is clearer what we are waiting for

[crawfordxx]

Good point — updated in the latest commit to use errCh := make(chan error, 1) with defer func() { errCh <- deferredErr }(). The goroutine now reads <-errCh directly instead of the two-step <-done + reading the named return variable, which makes the intent clearer.

[giuseppe]

LGTM

[Honny1]

LGTM! Once you fix the DCO and squash commits, we can merge.