diff --git a/toolkit/tools/go.mod b/toolkit/tools/go.mod index eccd1a2de8..d5c674cf82 100644 --- a/toolkit/tools/go.mod +++ b/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 diff --git a/toolkit/tools/go.sum b/toolkit/tools/go.sum index 0ead06de29..067f238f4b 100644 --- a/toolkit/tools/go.sum +++ b/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= diff --git a/toolkit/tools/internal/safechroot/safechroot.go b/toolkit/tools/internal/safechroot/safechroot.go index 4d14260fce..e33a7a14cd 100644 --- a/toolkit/tools/internal/safechroot/safechroot.go +++ b/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)