From 1348285060d855c885934b886a542b534b56a784 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Fri, 29 Oct 2021 10:28:38 +0200
Subject: [PATCH] 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>
---
 libpod/container_internal.go | 24 +++++++++++++++++++---
 libpod/runtime_ctr.go        |  9 --------
 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 d71179017abe..54d6b1303995 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 7d3891f6e1bf..f27e854a4602 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 3a6098de847d..de00eb8e05fa 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
 }