fix: clean up and overhaul mount ops

internal/app/machined/pkg/controllers/network/etcfile_test.go

@@ -361,7 +361,7 @@ func (suite *EtcFileConfigSuite) ExtraTearDown() {
 		if suite.etcRoot.FSType() == "os" {
 			suite.Require().NoError(os.Remove(suite.podResolvConfPath))
 		} else {
-			suite.Require().NoError(mount.SafeUnmount(context.Background(), nil, suite.podResolvConfPath, false))
+			suite.Require().NoError(mount.SafeUnmount(context.Background(), nil, suite.podResolvConfPath, false, false))
 		}
 	}
 

internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go

@@ -766,7 +766,7 @@ func UnmountPodMounts(runtime.Sequence, any) (runtime.TaskExecutionFunc, string)
 			if strings.HasPrefix(mountpoint, constants.EphemeralMountPoint+"/") {
 				logger.Printf("unmounting %s\n", mountpoint)
 
-				if err = mountv3.SafeUnmount(ctx, logger.Printf, mountpoint, false); err != nil {
+				if err = mountv3.SafeUnmount(ctx, logger.Printf, mountpoint, false, false); err != nil {
 					if errors.Is(err, syscall.EINVAL) {
 						log.Printf("ignoring unmount error %s: %v", mountpoint, err)
 					} else {
@@ -823,7 +823,7 @@ func UnmountSystemDiskBindMounts(runtime.Sequence, any) (runtime.TaskExecutionFu
 
 			logger.Printf("unmounting %s\n", mountpoint)
 
-			if err = mountv3.SafeUnmount(ctx, logger.Printf, mountpoint, false); err != nil {
+			if err = mountv3.SafeUnmount(ctx, logger.Printf, mountpoint, false, false); err != nil {
 				if errors.Is(err, syscall.EINVAL) {
 					log.Printf("ignoring unmount error %s: %v", mountpoint, err)
 				} else {

internal/pkg/mount/v3/all.go

@@ -6,34 +6,14 @@ package mount
 
 import (
 	"bufio"
-	"fmt"
+	"context"
 	"log"
 	"os"
 	"sort"
 	"strings"
 	"time"
-
-	"golang.org/x/sys/unix"
 )
 
-func unmountWithTimeout(target string, flags int, timeout time.Duration) error {
-	errCh := make(chan error, 1)
-
-	go func() {
-		errCh <- unix.Unmount(target, flags)
-	}()
-
-	timer := time.NewTimer(timeout)
-	defer timer.Stop()
-
-	select {
-	case <-timer.C:
-		return fmt.Errorf("unmounting %s timed out after %s", target, timeout)
-	case err := <-errCh:
-		return err
-	}
-}
-
 // UnmountAll attempts to unmount all the mounted filesystems via "self" mountinfo.
 //
 //nolint:gocyclo
@@ -60,7 +40,7 @@ func UnmountAll() error {
 			}
 
 			if strings.HasPrefix(mountInfo.MountSource, "/dev/") {
-				err = unmountWithTimeout(mountInfo.MountPoint, 0, time.Second)
+				err = SafeUnmount(context.Background(), log.Printf, mountInfo.MountPoint, true, false)
 				if err == nil {
 					log.Printf("unmounted %s (%s)", mountInfo.MountPoint, mountInfo.MountSource)
 				} else {
@@ -93,8 +73,10 @@ func UnmountAll() error {
 }
 
 type mountInfo struct {
-	MountPoint  string
-	MountSource string
+	MountPoint   string
+	MountSource  string
+	MountType    string
+	MountOptions map[string]string
 }
 
 func readMountInfo() ([]mountInfo, error) {
@@ -126,10 +108,27 @@ func readMountInfo() ([]mountInfo, error) {
 			mntInfo.MountPoint = pre[4]
 		}
 
+		if len(post) >= 1 {
+			mntInfo.MountType = post[0]
+		}
+
 		if len(post) >= 2 {
 			mntInfo.MountSource = post[1]
 		}
 
+		if len(post) >= 3 {
+			mntInfo.MountOptions = make(map[string]string)
+
+			for option := range strings.SplitSeq(post[2], ",") {
+				k, v, ok := strings.Cut(option, "=")
+				if ok {
+					mntInfo.MountOptions[k] = v
+				} else {
+					mntInfo.MountOptions[option] = ""
+				}
+			}
+		}
+
 		mounts = append(mounts, mntInfo)
 	}
 
@@ -144,13 +143,33 @@ func getSubmounts(target string) ([]string, error) {
 
 	var submounts []string
 
+	seen := make(map[string]struct{})
+
+	add := func(mountPoint string) {
+		if _, ok := seen[mountPoint]; ok {
+			return
+		}
+
+		seen[mountPoint] = struct{}{}
+		submounts = append(submounts, mountPoint)
+	}
+
 	for _, mnt := range mounts {
 		if mnt.MountPoint == target {
 			continue
 		}
 
+		// mounts nested under the target keep it busy.
 		if strings.HasPrefix(mnt.MountPoint, target+"/") {
-			submounts = append(submounts, mnt.MountPoint)
+			add(mnt.MountPoint)
+
+			continue
+		}
+
+		// overlays mounted elsewhere still pin the target if any of their
+		// backing directories (upper/work/lower) live under it.
+		if mnt.MountType == "overlay" && overlayReferences(mnt, target) {
+			add(mnt.MountPoint)
 		}
 	}
 
@@ -160,3 +179,25 @@ func getSubmounts(target string) ([]string, error) {
 
 	return submounts, nil
 }
+
+// overlayReferences reports whether an overlay mount has any of its backing
+// directories (upperdir, workdir or one of the lowerdirs) located under target.
+func overlayReferences(mnt mountInfo, target string) bool {
+	prefix := target + "/"
+
+	if strings.HasPrefix(mnt.MountOptions["upperdir"], prefix) {
+		return true
+	}
+
+	if strings.HasPrefix(mnt.MountOptions["workdir"], prefix) {
+		return true
+	}
+
+	for lowerdir := range strings.SplitSeq(mnt.MountOptions["lowerdir"], ":") {
+		if strings.HasPrefix(lowerdir, prefix) {
+			return true
+		}
+	}
+
+	return false
+}

internal/pkg/mount/v3/helpers.go

@@ -219,6 +219,7 @@ func PseudoLate(printer func(string, ...any)) Managers {
 			WithMountAttributes(unix.MOUNT_ATTR_RELATIME),
 			WithSelinuxLabel(constants.RunSelinuxLabel),
 			WithRecursiveUnmount(),
+			WithLazyUnmount(),
 			WithFsopen(
 				"tmpfs",
 				fsopen.WithStringParameter("mode", "0755"),
@@ -232,6 +233,7 @@ func PseudoLate(printer func(string, ...any)) Managers {
 			WithMountAttributes(unix.MOUNT_ATTR_RELATIME),
 			WithSelinuxLabel(constants.SystemSelinuxLabel),
 			WithRecursiveUnmount(),
+			WithLazyUnmount(),
 			WithFsopen(
 				"tmpfs",
 				fsopen.WithStringParameter("mode", "0755"),

internal/pkg/mount/v3/manager.go

@@ -32,6 +32,7 @@ type Manager struct {
 	extraDirs             []string
 	extraUnmountCallbacks []func(m *Manager)
 	recursiveUnmount      bool
+	lazyUnmount           bool
 
 	point *Point
 }
@@ -112,6 +113,7 @@ func (m *Manager) Unmount() error {
 	opts := UnmountOptions{
 		Printer:   printer,
 		Recursive: m.recursiveUnmount,
+		Lazy:      m.lazyUnmount,
 	}
 
 	for _, cb := range m.extraUnmountCallbacks {
@@ -264,3 +266,16 @@ func WithRecursiveUnmount() ManagerOption {
 		},
 	}
 }
+
+// WithLazyUnmount enables a lazy detach (MNT_DETACH) as a last resort when the target,
+// or one of its submounts, can't be unmounted otherwise.
+//
+// It's meant for volatile pseudo mounts (e.g. /system, /run) torn down on shutdown,
+// not for real filesystems where a busy mount is better left for the caller to retry.
+func WithLazyUnmount() ManagerOption {
+	return ManagerOption{
+		set: func(m *Manager) {
+			m.lazyUnmount = true
+		},
+	}
+}

internal/pkg/mount/v3/point.go

@@ -133,6 +133,7 @@ func (p *Point) Share() error {
 type UnmountOptions struct {
 	Printer   func(string, ...any)
 	Recursive bool
+	Lazy      bool
 }
 
 // Release closes the file descriptor of the underlying mount point.
@@ -155,7 +156,7 @@ func (p *Point) Unmount(opts UnmountOptions) error {
 	}
 
 	err := p.retry(func() error {
-		return SafeUnmount(context.Background(), opts.Printer, p.target, opts.Recursive)
+		return SafeUnmount(context.Background(), opts.Printer, p.target, opts.Recursive, opts.Lazy)
 	}, true)
 	if err != nil {
 		logSubmounts(opts.Printer, p.target)

internal/pkg/mount/v3/unmount.go

@@ -82,8 +82,20 @@ unmountLoop:
 // SafeUnmount unmounts the target path, first without force, then with force if the first attempt fails.
 //
 // It makes sure that unmounting has a finite operation timeout.
-// If recursive is true, it will first unmount all child mounts under target.
-func SafeUnmount(ctx context.Context, printer func(string, ...any), target string, recursive bool) error {
+//
+// If recursive is true, it first unmounts all child mounts (and overlays whose
+// backing dirs live under target).
+//
+// If lazy is true, it detaches the target (and any submount it can't unmount)
+// with MNT_DETACH as a last resort. This is used to tear down volatile pseudo
+// mounts (e.g. /system, /run) on shutdown, where the target may still be pinned
+// by kernel references (loop devices, peer mounts in other namespaces, ...)
+// that no regular unmount can release. It should not be set for real
+// filesystems, where a busy mount is better left for the caller to retry and
+// diagnose.
+//
+//nolint:gocyclo
+func SafeUnmount(ctx context.Context, printer func(string, ...any), target string, recursive, lazy bool) error {
 	const (
 		unmountTimeout      = 90 * time.Second
 		unmountForceTimeout = 10 * time.Second
@@ -101,7 +113,7 @@ func SafeUnmount(ctx context.Context, printer func(string, ...any), target strin
 			for _, submount := range submounts {
 				printer("recursively unmounting submount %s", submount)
 
-				if err := safeUnmountSingle(ctx, printer, submount, unmountTimeout); err != nil {
+				if err := safeUnmountSingle(ctx, printer, submount, unmountTimeout, lazy); err != nil {
 					printer("failed to unmount submount %s: %v", submount, err)
 				}
 			}
@@ -110,24 +122,59 @@ func SafeUnmount(ctx context.Context, printer func(string, ...any), target strin
 
 	ok, err := unmountLoop(ctx, printer, target, 0, unmountTimeout, "")
 
-	if ok {
+	// the unmount syscall completed within the timeout: a hung unmount (ok ==
+	// false) is the only case the force flag can help with, otherwise the
+	// result (success or e.g. EBUSY) is final for the regular attempt.
+	if !ok {
+		printer("unmounting %s with force", target)
+
+		ok, err = unmountLoop(ctx, printer, target, unix.MNT_FORCE, unmountForceTimeout, " with force flag")
+	}
+
+	switch {
+	case ok && err == nil:
+		return nil
+	case lazy:
+		// volatile pseudo mount still busy or hung: detach it lazily so it
+		// leaves the tree immediately and the kernel reaps it once the
+		// remaining references are gone.
+		return lazyDetach(printer, target)
+	case ok:
 		return err
+	default:
+		return fmt.Errorf("unmounting %s with force flag timed out", target)
 	}
+}
 
-	printer("unmounting %s with force", target)
+// safeUnmountSingle unmounts a single submount that keeps a parent mount busy.
+//
+// When lazy is set, it falls back to a lazy detach (MNT_DETACH) if the regular
+// unmount fails or times out: detaching the submount from the tree is enough to
+// let the parent be unmounted, and the kernel reaps it once it's no longer in
+// use. Otherwise it reports the unmount result for the caller to log.
+func safeUnmountSingle(ctx context.Context, printer func(string, ...any), target string, timeout time.Duration, lazy bool) error {
+	ok, err := unmountLoop(ctx, printer, target, 0, timeout, "")
 
-	ok, err = unmountLoop(ctx, printer, target, unix.MNT_FORCE, unmountForceTimeout, " with force flag")
+	switch {
+	case ok && (err == nil || errors.Is(err, unix.EINVAL)):
+		// unmounted, or already unmounted
+		return nil
+	case !lazy:
+		if ok {
+			return err
+		}
 
-	if ok {
-		return err
+		return nil
 	}
 
-	return fmt.Errorf("unmounting %s with force flag timed out", target)
+	return lazyDetach(printer, target)
 }
 
-func safeUnmountSingle(ctx context.Context, printer func(string, ...any), target string, timeout time.Duration) error {
-	ok, err := unmountLoop(ctx, printer, target, 0, timeout, "")
-	if ok {
+// lazyDetach detaches target with MNT_DETACH, ignoring an EINVAL (not mounted).
+func lazyDetach(printer func(string, ...any), target string) error {
+	printer("lazily detaching %s", target)
+
+	if err := unix.Unmount(target, unix.MNT_DETACH); err != nil && !errors.Is(err, unix.EINVAL) {
 		return err
 	}