diff options
author | Valentin Rothberg <rothberg@redhat.com> | 2021-10-29 10:28:38 +0200 |
---|---|---|
committer | Matthew Heon <matthew.heon@pm.me> | 2021-11-12 11:08:25 -0500 |
commit | c8b7ca2ba2ef9a57103b5c05e6e29b54b0c6fa48 (patch) | |
tree | 2fd5d0ee15434453863127715e4c9d26cbdba010 | |
parent | 2dc8db77307a050cab9d030149be34e636f04657 (diff) | |
download | podman-c8b7ca2ba2ef9a57103b5c05e6e29b54b0c6fa48.tar.gz podman-c8b7ca2ba2ef9a57103b5c05e6e29b54b0c6fa48.tar.bz2 podman-c8b7ca2ba2ef9a57103b5c05e6e29b54b0c6fa48.zip |
pod/container create: resolve conflicts of generated names
Address the TOCTOU when generating random names by having at most 10
attempts to assign a random name when creating a pod or container.
[NO TESTS NEEDED] since I do not know a way to force a conflict with
randomly generated names in a reasonable time frame.
Fixes: #11735
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
-rw-r--r-- | libpod/container_internal.go | 24 | ||||
-rw-r--r-- | libpod/runtime_ctr.go | 9 | ||||
-rw-r--r-- | libpod/runtime_pod_linux.go | 40 |
3 files changed, 47 insertions, 26 deletions
diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d71179017..54d6b1303 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -479,9 +479,27 @@ func (c *Container) setupStorage(ctx context.Context) error { c.setupStorageMapping(&options.IDMappingOptions, &c.config.IDMappings) - containerInfo, err := c.runtime.storageService.CreateContainerStorage(ctx, c.runtime.imageContext, c.config.RootfsImageName, c.config.RootfsImageID, c.config.Name, c.config.ID, options) - if err != nil { - return errors.Wrapf(err, "error creating container storage") + // Unless the user has specified a name, use a randomly generated one. + // Note that name conflicts may occur (see #11735), so we need to loop. + generateName := c.config.Name == "" + var containerInfo ContainerInfo + var containerInfoErr error + for { + if generateName { + name, err := c.runtime.generateName() + if err != nil { + return err + } + c.config.Name = name + } + containerInfo, containerInfoErr = c.runtime.storageService.CreateContainerStorage(ctx, c.runtime.imageContext, c.config.RootfsImageName, c.config.RootfsImageID, c.config.Name, c.config.ID, options) + + if !generateName || errors.Cause(containerInfoErr) != storage.ErrDuplicateName { + break + } + } + if containerInfoErr != nil { + return errors.Wrapf(containerInfoErr, "error creating container storage") } c.config.IDMappings.UIDMap = containerInfo.UIDMap diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 7d3891f6e..f27e854a4 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -322,15 +322,6 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai } } - if ctr.config.Name == "" { - name, err := r.generateName() - if err != nil { - return nil, err - } - - ctr.config.Name = name - } - // Check CGroup parent sanity, and set it if it was not set. // Only if we're actually configuring CGroups. if !ctr.config.NoCgroups { diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index 3a6098de8..de00eb8e0 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -43,18 +43,6 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option } } - if pod.config.Name == "" { - name, err := r.generateName() - if err != nil { - return nil, err - } - pod.config.Name = name - } - - if p.InfraContainerSpec != nil && p.InfraContainerSpec.Hostname == "" { - p.InfraContainerSpec.Hostname = pod.config.Name - } - // Allocate a lock for the pod lock, err := r.lockManager.AllocateLock() if err != nil { @@ -131,9 +119,33 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option logrus.Infof("Pod has an infra container, but shares no namespaces") } - if err := r.state.AddPod(pod); err != nil { - return nil, errors.Wrapf(err, "error adding pod to state") + // Unless the user has specified a name, use a randomly generated one. + // Note that name conflicts may occur (see #11735), so we need to loop. + generateName := pod.config.Name == "" + var addPodErr error + for { + if generateName { + name, err := r.generateName() + if err != nil { + return nil, err + } + pod.config.Name = name + } + + if p.InfraContainerSpec != nil && p.InfraContainerSpec.Hostname == "" { + p.InfraContainerSpec.Hostname = pod.config.Name + } + if addPodErr = r.state.AddPod(pod); addPodErr == nil { + return pod, nil + } + if !generateName || (errors.Cause(addPodErr) != define.ErrPodExists && errors.Cause(addPodErr) != define.ErrCtrExists) { + break + } + } + if addPodErr != nil { + return nil, errors.Wrapf(addPodErr, "error adding pod to state") } + return pod, nil } |