From e3e107257581e676208a0493a78e78ddfdbc1d59 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 6 May 2024 17:56:19 -0700
Subject: [PATCH] libct: fix locking in Start/Run/Exec

1. The code to call c.exec from c.Run was initially added by commit
   3aacff695. At the time, there was a lock in c.Run. That lock was
   removed by commit bd3c4f84, which resulted in part of c.Run executing
   without the lock.

2. All the Start/Run/Exec calls were a mere wrappers for start/run/exec
   adding a lock, but some more code crept into Start at some point,
   e.g. by commits 805b8c73 and 108ee85b8. Since the reason mentioned in
   commit 805b8c73 is no longer true after refactoring, we can fix this.

Fix both issues by moving code out of wrappers, and adding locking into
c.Run.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/container_linux.go | 34 +++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
index cb68b30a209..7566afd25b3 100644
--- a/libcontainer/container_linux.go
+++ b/libcontainer/container_linux.go
@@ -204,28 +204,16 @@ func (c *Container) Set(config configs.Config) error {
 func (c *Container) Start(process *Process) error {
 	c.m.Lock()
 	defer c.m.Unlock()
-	if c.config.Cgroups.Resources.SkipDevices {
-		return errors.New("can't start container with SkipDevices set")
-	}
-	if process.Init {
-		if err := c.createExecFifo(); err != nil {
-			return err
-		}
-	}
-	if err := c.start(process); err != nil {
-		if process.Init {
-			c.deleteExecFifo()
-		}
-		return err
-	}
-	return nil
+	return c.start(process)
 }
 
 // Run immediately starts the process inside the container. Returns an error if
 // the process fails to start. It does not block waiting for the exec fifo
 // after start returns but opens the fifo after start returns.
 func (c *Container) Run(process *Process) error {
-	if err := c.Start(process); err != nil {
+	c.m.Lock()
+	defer c.m.Unlock()
+	if err := c.start(process); err != nil {
 		return err
 	}
 	if process.Init {
@@ -314,6 +302,20 @@ type openResult struct {
 }
 
 func (c *Container) start(process *Process) (retErr error) {
+	if c.config.Cgroups.Resources.SkipDevices {
+		return errors.New("can't start container with SkipDevices set")
+	}
+	if process.Init {
+		if err := c.createExecFifo(); err != nil {
+			return err
+		}
+		defer func() {
+			if retErr != nil {
+				c.deleteExecFifo()
+			}
+		}()
+	}
+
 	parent, err := c.newParentProcess(process)
 	if err != nil {
 		return fmt.Errorf("unable to create new parent process: %w", err)