opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2025-08-06 19:44:14 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix races in the HTTP attach API

Browse Source 
This is very similar to commit 3280da0500, we cannot check the state
then unlock to then lock again and do the action. Everything must
happen under one lock. To fix this move the code into the HTTPAttach
function in libpod. The locking here is a bit weird because attach
blocks for the lifetime of attach which can be very long so we must
unlock before performing the attach.

Fixes #23757

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This commit is contained in:
Paul Holzinger
2024-08-27 11:45:13 +02:00
parent 9ad3e84cb3
commit bf74797c69
2 changed files with 21 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
libpod/container_api.go
View File

@ -40,7 +40,10 @@ func (c *Container) Init(ctx context.Context, recursive bool) error {
return err
}
}
return c.initUnlocked(ctx, recursive)
}
func (c *Container) initUnlocked(ctx context.Context, recursive bool) error {
if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateStopped, define.ContainerStateExited) {
return fmt.Errorf("container %s has already been created in runtime: %w", c.ID(), define.ErrCtrStateInvalid)
}
@ -342,25 +345,36 @@ func (c *Container) HTTPAttach(r *http.Request, w http.ResponseWriter, streams *
close(hijackDone)
}()
locked := false
if !c.batched {
locked = true
c.lock.Lock()
if err := c.syncContainer(); err != nil {
defer func() {
if locked {
c.lock.Unlock()
}
}()
if err := c.syncContainer(); err != nil {
return err
}
// We are NOT holding the lock for the duration of the function.
c.lock.Unlock()
}
if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) {
return fmt.Errorf("can only attach to created or running containers: %w", define.ErrCtrStateInvalid)
// For Docker compatibility, we need to re-initialize containers in these states.
if c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited, define.ContainerStateStopped) {
if err := c.initUnlocked(r.Context(), c.config.Pod != ""); err != nil {
return fmt.Errorf("preparing container %s for attach: %w", c.ID(), err)
}
} else if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) {
return fmt.Errorf("can only attach to created or running containers - currently in state %s: %w", c.state.State.String(), define.ErrCtrStateInvalid)
}
if !streamAttach && !streamLogs {
return fmt.Errorf("must specify at least one of stream or logs: %w", define.ErrInvalidArg)
}
// We are NOT holding the lock for the duration of the function.
locked = false
c.lock.Unlock()
logrus.Infof("Performing HTTP Hijack attach to container %s", c.ID())
c.newContainerEvent(events.Attach)

18
pkg/api/handlers/compat/containers_attach.go
View File

@ -4,11 +4,9 @@ package compat
import (
"errors"
"fmt"
"net/http"
"github.com/containers/podman/v5/libpod"
"github.com/containers/podman/v5/libpod/define"
"github.com/containers/podman/v5/pkg/api/handlers/utils"
"github.com/containers/podman/v5/pkg/api/server/idle"
api "github.com/containers/podman/v5/pkg/api/types"
@ -79,22 +77,6 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) {
return
}
state, err := ctr.State()
if err != nil {
utils.InternalServerError(w, err)
return
}
// For Docker compatibility, we need to re-initialize containers in these states.
if state == define.ContainerStateConfigured || state == define.ContainerStateExited || state == define.ContainerStateStopped {
if err := ctr.Init(r.Context(), ctr.PodID() != ""); err != nil {
utils.Error(w, http.StatusConflict, fmt.Errorf("preparing container %s for attach: %w", ctr.ID(), err))
return
}
} else if !(state == define.ContainerStateCreated || state == define.ContainerStateRunning) {
utils.InternalServerError(w, fmt.Errorf("can only attach to created or running containers - currently in state %s: %w", state.String(), define.ErrCtrStateInvalid))
return
}
logErr := func(e error) {
logrus.Errorf("Error attaching to container %s: %v", ctr.ID(), e)
}