From e92de11a69e5aa503fc2fe4ffd569133179cefa5 Mon Sep 17 00:00:00 2001 From: Matthew Heon <mheon@redhat.com> Date: Mon, 1 Jul 2019 09:23:21 -0400 Subject: Ensure locks are freed when ctr/pod creation fails If we don't do this, we can leak locks on every failure, and that is very, very bad - can render Podman unusable without a 'system renumber' being run. Signed-off-by: Matthew Heon <mheon@redhat.com> --- libpod/runtime_ctr.go | 8 ++++++++ libpod/runtime_pod_linux.go | 22 ++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 0d0f700a6..9daac161c 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -132,6 +132,14 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container, restore bo ctr.config.LockID = ctr.lock.ID() logrus.Debugf("Allocated lock %d for container %s", ctr.lock.ID(), ctr.ID()) + defer func() { + if err != nil { + if err2 := ctr.lock.Free(); err2 != nil { + logrus.Errorf("Error freeing lock for container after creation failed: %v", err2) + } + } + }() + ctr.valid = true ctr.state.State = config2.ContainerStateConfigured ctr.runtime = r diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index e9ce130da..d667d3a25 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -19,7 +19,7 @@ import ( ) // NewPod makes a new, empty pod -func (r *Runtime) NewPod(ctx context.Context, options ...PodCreateOption) (*Pod, error) { +func (r *Runtime) NewPod(ctx context.Context, options ...PodCreateOption) (_ *Pod, Err error) { r.lock.Lock() defer r.lock.Unlock() @@ -60,6 +60,14 @@ func (r *Runtime) NewPod(ctx context.Context, options ...PodCreateOption) (*Pod, pod.lock = lock pod.config.LockID = pod.lock.ID() + defer func() { + if Err != nil { + if err := pod.lock.Free(); err != nil { + logrus.Errorf("Error freeing pod lock after failed creation: %v", err) + } + } + }() + pod.valid = true // Check CGroup parent sanity, and set it if it was not set @@ -113,15 +121,17 @@ func (r *Runtime) NewPod(ctx context.Context, options ...PodCreateOption) (*Pod, if err := r.state.AddPod(pod); err != nil { return nil, errors.Wrapf(err, "error adding pod to state") } + defer func() { + if Err != nil { + if err := r.removePod(ctx, pod, true, true); err != nil { + logrus.Errorf("Error removing pod after pause container creation failure: %v", err) + } + } + }() if pod.HasInfraContainer() { ctr, err := r.createInfraContainer(ctx, pod) if err != nil { - // Tear down pod, as it is assumed a the pod will contain - // a pause container, and it does not. - if err2 := r.removePod(ctx, pod, true, true); err2 != nil { - logrus.Errorf("Error removing pod after pause container creation failure: %v", err2) - } return nil, errors.Wrapf(err, "error adding Infra Container") } pod.state.InfraContainerID = ctr.ID() -- cgit v1.2.3-54-g00ecf From fdd98d58b8a03c76f25dfc39d238f7697450d2bd Mon Sep 17 00:00:00 2001 From: Matthew Heon <matthew.heon@pm.me> Date: Tue, 2 Jul 2019 12:52:23 -0400 Subject: Fix release notes Signed-off-by: Matthew Heon <matthew.heon@pm.me> --- RELEASE_NOTES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 1f4d4b1f9..69244bb09 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,6 +7,7 @@ - Fixed a bug where running Podman as root with `sudo -E` would not work after running rootless Podman at least once - Fixed a bug where options for `tmpfs` volumes added with the `--tmpfs` flag were being ignored - Fixed a bug where images with no layers could not properly be displayed and removed by Podman +- Fixed a bug where locks were not properly freed on failure to create a container or pod ### Misc - Updated containers/storage to v1.12.13 -- cgit v1.2.3-54-g00ecf