Add a fallback lazy mode to chroot unmount (#6216)

* Add a fallback lazy mode to chroot unmount

toolkit/tools/go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/jinzhu/copier v0.3.2
 	github.com/juliangruber/go-intersect v1.1.0
 	github.com/klauspost/pgzip v1.2.5
+	github.com/moby/sys/mountinfo v0.6.2
 	github.com/muesli/crunchy v0.4.0
 	github.com/rivo/tview v0.0.0-20200219135020-0ba8301b415c
 	github.com/sirupsen/logrus v1.9.3

toolkit/tools/go.sum

@@ -41,6 +41,8 @@ github.com/lucasb-eyer/go-colorful v1.0.3/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i
 github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
 github.com/mattn/go-runewidth v0.0.7 h1:Ei8KR0497xHyKJPAv59M1dkC+rOZCMBJ+t3fZ+twI54=
 github.com/mattn/go-runewidth v0.0.7/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
+github.com/moby/sys/mountinfo v0.6.2 h1:BzJjoreD5BMFNmD9Rus6gdd1pLuecOFPt8wC+Vygl78=
+github.com/moby/sys/mountinfo v0.6.2/go.mod h1:IJb6JQeOklcdMU9F5xQ8ZALD+CUr5VlGpwtX+VE0rpI=
 github.com/muesli/crunchy v0.4.0 h1:qdiml8gywULHBsztiSAf6rrE6EyuNasNKZ104mAaahM=
 github.com/muesli/crunchy v0.4.0/go.mod h1:9k4x6xdSbb7WwtAVy0iDjaiDjIk6Wa5AgUIqp+HqOpU=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
@@ -79,6 +81,7 @@ golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190626150813-e07cf5db2756/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191018095205-727590c5006e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s=
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

toolkit/tools/internal/safechroot/safechroot.go

@@ -19,6 +19,7 @@ import (
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/shell"
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/systemdependency"
 
+	"github.com/moby/sys/mountinfo"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -82,6 +83,11 @@ var defaultChrootEnv = []string{
 	"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
 }
 
+const (
+	unmountTypeLazy   = true
+	unmountTypeNormal = !unmountTypeLazy
+)
+
 // init will always be called if this package is loaded
 func init() {
 	registerSIGTERMCleanup()
@@ -207,8 +213,8 @@ func (c *Chroot) Initialize(tarPath string, extraDirectories []string, extraMoun
 		if err != nil {
 			if buildpipeline.IsRegularBuild() {
 				// mount/unmount is only supported in regular pipeline
-				// Best effort cleanup in case mountpoint creation failed mid-way through
-				cleanupErr := c.unmountAndRemove(leaveChrootOnDisk)
+				// Best effort cleanup in case mountpoint creation failed mid-way through. We will not try again so treat as final attempt.
+				cleanupErr := c.unmountAndRemove(leaveChrootOnDisk, unmountTypeLazy)
 				if cleanupErr != nil {
 					logger.Log.Warnf("Failed to cleanup chroot (%s) during failed initialization. Error: %s", c.rootDir, cleanupErr)
 				}
@@ -398,7 +404,11 @@ func (c *Chroot) Close(leaveOnDisk bool) (err error) {
 
 	if buildpipeline.IsRegularBuild() {
 		// mount is only supported in regular pipeline
-		err = c.unmountAndRemove(leaveOnDisk)
+		err = c.unmountAndRemove(leaveOnDisk, unmountTypeNormal)
+		if err != nil {
+			logger.Log.Warnf("Chroot cleanup failed, will retry with lazy unmount. Error: %s", err)
+			err = c.unmountAndRemove(leaveOnDisk, unmountTypeLazy)
+		}
 		if err == nil {
 			const emptyLen = 0
 			// Remove this chroot from the list of active ones since it has now been cleaned up.
@@ -469,37 +479,63 @@ func cleanupAllChroots() {
 	inChrootMutex.Lock()
 
 	// mount is only supported in regular pipeline
+	failedToUnmount := false
 	if buildpipeline.IsRegularBuild() {
 		// Cleanup chroots in LIFO order incase any are interdependent (e.g. nested safe chroots)
 		logger.Log.Info("Cleaning up all active chroots")
 		for i := len(activeChroots) - 1; i >= 0; i-- {
 			logger.Log.Infof("Cleaning up chroot (%s)", activeChroots[i].rootDir)
-			err := activeChroots[i].unmountAndRemove(leaveChrootOnDisk)
+			err := activeChroots[i].unmountAndRemove(leaveChrootOnDisk, unmountTypeLazy)
 			// Perform best effort cleanup: unmount as many chroots as possible,
 			// even if one fails.
 			if err != nil {
 				logger.Log.Errorf("Failed to unmount chroot (%s)", activeChroots[i].rootDir)
+				failedToUnmount = true
 			}
 		}
 	}
 
-	logger.Log.Info("Cleanup finished")
+	if failedToUnmount {
+		logger.Log.Fatalf("Failed to unmount a chroot, manual unmount required. See above errors for details on which mounts failed.")
+	} else {
+		logger.Log.Info("Cleanup finished")
+	}
 }
 
 // unmountAndRemove retries to unmount directories that were mounted into
 // the chroot until the unmounts succeed or too many failed attempts.
 // This is to avoid leaving folders like /dev mounted when the chroot folder is forcefully deleted in cleanup.
 // Iff all mounts were successfully unmounted, the chroot's root directory will be removed if requested.
-func (c *Chroot) unmountAndRemove(leaveOnDisk bool) (err error) {
+// If doLazyUnmount is true, use the lazy unmount flag which will allow the unmount to succeed even if the mount point is busy.
+func (c *Chroot) unmountAndRemove(leaveOnDisk, lazyUnmount bool) (err error) {
 	const (
-		totalAttempts = 3
-		retryDuration = time.Second
-		unmountFlags  = 0
+		retryDuration      = time.Second
+		totalAttempts      = 3
+		unmountFlagsNormal = 0
+		// Do a lazy unmount as a fallback. This will allow the unmount to succeed even if the mount point is busy.
+		// This is to avoid leaving folders like /dev mounted if the chroot folder is forcefully deleted by the user. Even
+		// if the mount is busy at least it will be detached from the filesystem and will not damage the host.
+		unmountFlagsLazy = unix.MNT_DETACH
 	)
+	unmountFlags := unmountFlagsNormal
+	if lazyUnmount {
+		unmountFlags = unmountFlagsLazy
+	}
 
 	for _, mountPoint := range c.mountPoints {
 		fullPath := filepath.Join(c.rootDir, mountPoint.target)
 
+		var isMounted bool
+		isMounted, err = mountinfo.Mounted(fullPath)
+		if err != nil {
+			err = fmt.Errorf("failed to check if mount point (%s) is mounted. Error: %s", fullPath, err)
+			return
+		}
+		if !isMounted {
+			logger.Log.Debugf("Skipping unmount of (%s) because it is not mounted", fullPath)
+			continue
+		}
+
 		logger.Log.Debugf("Unmounting (%s)", fullPath)
 
 		// Skip mount points if they were not successfully created
@@ -507,9 +543,11 @@ func (c *Chroot) unmountAndRemove(leaveOnDisk bool) (err error) {
 			continue
 		}
 
-		err = retry.Run(func() error {
-			return unix.Unmount(fullPath, unmountFlags)
-		}, totalAttempts, retryDuration)
+		_, err = retry.RunWithExpBackoff(func() error {
+			logger.Log.Debugf("Calling unmount on path(%s) with flags (%v)", fullPath, unmountFlags)
+			umountErr := unix.Unmount(fullPath, unmountFlags)
+			return umountErr
+		}, totalAttempts, retryDuration, 2.0, nil)
 
 		if err != nil {
 			logger.Log.Warnf("Failed to unmount (%s). Error: %s", fullPath, err)