Merge pull request #2039 from giuseppe/fix-loopback-race-condition

loopback: fix race condition opening loopback device

pkg/loopback/attach_loopback.go

@@ -6,10 +6,12 @@ package loopback
 import (
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"syscall"
 
 	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
 )
 
 // Loopback related errors
@@ -39,7 +41,7 @@ func getNextFreeLoopbackIndex() (int, error) {
 	return index, err
 }
 
-func openNextAvailableLoopback(index int, sparseName string, sparseFile *os.File) (loopFile *os.File, err error) {
+func openNextAvailableLoopback(sparseName string, sparseFile *os.File) (loopFile *os.File, err error) {
 	// Read information about the loopback file.
 	var st syscall.Stat_t
 	err = syscall.Fstat(int(sparseFile.Fd()), &st)
@@ -48,31 +50,51 @@ func openNextAvailableLoopback(index int, sparseName string, sparseFile *os.File
 		return nil, ErrAttachLoopbackDevice
 	}
 
+	// upper bound to avoid infinite loop
+	remaining := 1000
+
 	// Start looking for a free /dev/loop
 	for {
-		target := fmt.Sprintf("/dev/loop%d", index)
-		index++
-
-		fi, err := os.Stat(target)
-		if err != nil {
-			if os.IsNotExist(err) {
-				logrus.Error("There are no more loopback devices available.")
-			}
+		if remaining == 0 {
+			logrus.Errorf("No free loopback devices available")
 			return nil, ErrAttachLoopbackDevice
 		}
+		remaining--
 
-		if fi.Mode()&os.ModeDevice != os.ModeDevice {
-			logrus.Errorf("Loopback device %s is not a block device.", target)
-			continue
+		index, err := getNextFreeLoopbackIndex()
+		if err != nil {
+			logrus.Debugf("Error retrieving the next available loopback: %s", err)
+			return nil, err
 		}
 
+		target := fmt.Sprintf("/dev/loop%d", index)
+
 		// OpenFile adds O_CLOEXEC
 		loopFile, err = os.OpenFile(target, os.O_RDWR, 0o644)
 		if err != nil {
+			// The kernel returns ENXIO when opening a device that is in the "deleting" or "rundown" state, so
+			// just treat ENXIO as if the device does not exist.
+			if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ENXIO) {
+				// Another process could have taken the loopback device in the meantime.  So repeat
+				// the process with the next loopback device.
+				continue
+			}
 			logrus.Errorf("Opening loopback device: %s", err)
 			return nil, ErrAttachLoopbackDevice
 		}
 
+		fi, err := loopFile.Stat()
+		if err != nil {
+			loopFile.Close()
+			logrus.Errorf("Stat loopback device: %s", err)
+			return nil, ErrAttachLoopbackDevice
+		}
+		if fi.Mode()&os.ModeDevice != os.ModeDevice {
+			loopFile.Close()
+			logrus.Errorf("Loopback device %s is not a block device.", target)
+			continue
+		}
+
 		// Try to attach to the loop file
 		if err := ioctlLoopSetFd(loopFile.Fd(), sparseFile.Fd()); err != nil {
 			loopFile.Close()
@@ -124,14 +146,6 @@ func AttachLoopDeviceRO(sparseName string) (loop *os.File, err error) {
 }
 
 func attachLoopDevice(sparseName string, readonly bool) (loop *os.File, err error) {
-	// Try to retrieve the next available loopback device via syscall.
-	// If it fails, we discard error and start looping for a
-	// loopback from index 0.
-	startIndex, err := getNextFreeLoopbackIndex()
-	if err != nil {
-		logrus.Debugf("Error retrieving the next available loopback: %s", err)
-	}
-
 	var sparseFile *os.File
 
 	// OpenFile adds O_CLOEXEC
@@ -146,7 +160,7 @@ func attachLoopDevice(sparseName string, readonly bool) (loop *os.File, err erro
 	}
 	defer sparseFile.Close()
 
-	loopFile, err := openNextAvailableLoopback(startIndex, sparseName, sparseFile)
+	loopFile, err := openNextAvailableLoopback(sparseName, sparseFile)
 	if err != nil {
 		return nil, err
 	}

pkg/loopback/attach_test.go

@@ -0,0 +1,49 @@
+//go:build linux && cgo
+// +build linux,cgo
+
+package loopback
+
+import (
+	"os"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	maxDevicesPerGoroutine = 1000
+	maxGoroutines          = 10
+)
+
+func TestAttachLoopbackDeviceRace(t *testing.T) {
+	createLoopbackDevice := func() {
+		// Create a file to use as a backing file
+		f, err := os.CreateTemp(t.TempDir(), "loopback-test")
+		require.NoError(t, err)
+		defer f.Close()
+
+		defer os.Remove(f.Name())
+
+		lp, err := AttachLoopDevice(f.Name())
+		assert.NoError(t, err)
+		assert.NotNil(t, lp, "loopback device file should not be nil")
+		if lp != nil {
+			lp.Close()
+		}
+	}
+
+	wg := sync.WaitGroup{}
+
+	for i := 0; i < maxGoroutines; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for i := 0; i < maxDevicesPerGoroutine; i++ {
+				createLoopbackDevice()
+			}
+		}()
+	}
+	wg.Wait()
+}