exec: add extra validation for submount sources

While submount paths were already validated there are some
cases where the parent mount may not be immutable while the
submount is created.

Signed-off-by: Tonis Tiigi <[email protected]>
(cherry picked from commit 2529ec4121bcd8c35bcd96218083da175c2e5b77)
(cherry picked from commit cbc233b3b695918d92fd5b1407b829296c53db70)

Author: tonistiigi

executor/oci/spec.go

@@ -12,7 +12,6 @@ import (
 	"github.com/containerd/containerd/namespaces"
 	"github.com/containerd/containerd/oci"
 	"github.com/containerd/containerd/pkg/userns"
-	"github.com/containerd/continuity/fs"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/mitchellh/hashstructure/v2"
 	"github.com/moby/buildkit/executor"
@@ -215,6 +214,7 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
 type mountRef struct {
 	mount   mount.Mount
 	unmount func() error
+	subRefs map[string]mountRef
 }
 
 type submounts struct {
@@ -233,10 +233,17 @@ func (s *submounts) subMount(m mount.Mount, subPath string) (mount.Mount, error)
 		return mount.Mount{}, err
 	}
 	if mr, ok := s.m[h]; ok {
-		sm, err := sub(mr.mount, subPath)
+		if sm, ok := mr.subRefs[subPath]; ok {
+			return sm.mount, nil
+		}
+		sm, unmount, err := sub(mr.mount, subPath)
 		if err != nil {
 			return mount.Mount{}, err
 		}
+		mr.subRefs[subPath] = mountRef{
+			mount:   sm,
+			unmount: unmount,
+		}
 		return sm, nil
 	}
 
@@ -261,12 +268,17 @@ func (s *submounts) subMount(m mount.Mount, subPath string) (mount.Mount, error)
 			Options: opts,
 		},
 		unmount: lm.Unmount,
+		subRefs: map[string]mountRef{},
 	}
 
-	sm, err := sub(s.m[h].mount, subPath)
+	sm, unmount, err := sub(s.m[h].mount, subPath)
 	if err != nil {
 		return mount.Mount{}, err
 	}
+	s.m[h].subRefs[subPath] = mountRef{
+		mount:   sm,
+		unmount: unmount,
+	}
 	return sm, nil
 }
 
@@ -276,6 +288,9 @@ func (s *submounts) cleanup() {
 	for _, m := range s.m {
 		func(m mountRef) {
 			go func() {
+				for _, sm := range m.subRefs {
+					sm.unmount()
+				}
 				m.unmount()
 				wg.Done()
 			}()
@@ -284,15 +299,6 @@ func (s *submounts) cleanup() {
 	wg.Wait()
 }
 
-func sub(m mount.Mount, subPath string) (mount.Mount, error) {
-	src, err := fs.RootPath(m.Source, subPath)
-	if err != nil {
-		return mount.Mount{}, err
-	}
-	m.Source = src
-	return m, nil
-}
-
 func specMapping(s []idtools.IDMap) []specs.LinuxIDMapping {
 	var ids []specs.LinuxIDMapping
 	for _, item := range s {

executor/oci/spec_freebsd.go

@@ -0,0 +1,15 @@
+package oci
+
+import (
+	"github.com/containerd/containerd/mount"
+	"github.com/containerd/continuity/fs"
+)
+
+func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) {
+	src, err := fs.RootPath(m.Source, subPath)
+	if err != nil {
+		return mount.Mount{}, nil, err
+	}
+	m.Source = src
+	return m, func() error { return nil }, nil
+}

executor/oci/spec_linux.go

@@ -0,0 +1,57 @@
+//go:build linux
+// +build linux
+
+package oci
+
+import (
+	"os"
+	"strconv"
+
+	"github.com/containerd/containerd/mount"
+	"github.com/containerd/continuity/fs"
+	"github.com/moby/buildkit/snapshot"
+	"github.com/pkg/errors"
+	"golang.org/x/sys/unix"
+)
+
+func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) {
+	var retries = 10
+	root := m.Source
+	for {
+		src, err := fs.RootPath(root, subPath)
+		if err != nil {
+			return mount.Mount{}, nil, err
+		}
+		// similar to runc.WithProcfd
+		fh, err := os.OpenFile(src, unix.O_PATH|unix.O_CLOEXEC, 0)
+		if err != nil {
+			return mount.Mount{}, nil, err
+		}
+
+		fdPath := "/proc/self/fd/" + strconv.Itoa(int(fh.Fd()))
+		if resolved, err := os.Readlink(fdPath); err != nil {
+			fh.Close()
+			return mount.Mount{}, nil, err
+		} else if resolved != src {
+			retries--
+			if retries <= 0 {
+				fh.Close()
+				return mount.Mount{}, nil, errors.Errorf("unable to safely resolve subpath %s", subPath)
+			}
+			fh.Close()
+			continue
+		}
+
+		m.Source = fdPath
+		lm := snapshot.LocalMounterWithMounts([]mount.Mount{m}, snapshot.ForceRemount())
+		mp, err := lm.Mount()
+		if err != nil {
+			fh.Close()
+			return mount.Mount{}, nil, err
+		}
+		m.Source = mp
+		fh.Close() // release the fd, we don't need it anymore
+
+		return m, lm.Unmount, nil
+	}
+}

executor/oci/spec_windows.go

@@ -7,7 +7,9 @@ import (
 	"fmt"
 	"path/filepath"
 
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/oci"
+	"github.com/containerd/continuity/fs"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/moby/buildkit/solver/pb"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -67,3 +69,12 @@ func getTracingSocket() string {
 func cgroupV2NamespaceSupported() bool {
 	return false
 }
+
+func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) {
+	src, err := fs.RootPath(m.Source, subPath)
+	if err != nil {
+		return mount.Mount{}, nil, err
+	}
+	m.Source = src
+	return m, func() error { return nil }, nil
+}

snapshot/localmounter.go

@@ -11,22 +11,39 @@ type Mounter interface {
 	Unmount() error
 }
 
+type LocalMounterOpt func(*localMounter)
+
 // LocalMounter is a helper for mounting mountfactory to temporary path. In
 // addition it can mount binds without privileges
-func LocalMounter(mountable Mountable) Mounter {
-	return &localMounter{mountable: mountable}
+func LocalMounter(mountable Mountable, opts ...LocalMounterOpt) Mounter {
+	lm := &localMounter{mountable: mountable}
+	for _, opt := range opts {
+		opt(lm)
+	}
+	return lm
 }
 
 // LocalMounterWithMounts is a helper for mounting to temporary path. In
 // addition it can mount binds without privileges
-func LocalMounterWithMounts(mounts []mount.Mount) Mounter {
-	return &localMounter{mounts: mounts}
+func LocalMounterWithMounts(mounts []mount.Mount, opts ...LocalMounterOpt) Mounter {
+	lm := &localMounter{mounts: mounts}
+	for _, opt := range opts {
+		opt(lm)
+	}
+	return lm
 }
 
 type localMounter struct {
-	mu        sync.Mutex
-	mounts    []mount.Mount
-	mountable Mountable
-	target    string
-	release   func() error
+	mu           sync.Mutex
+	mounts       []mount.Mount
+	mountable    Mountable
+	target       string
+	release      func() error
+	forceRemount bool
+}
+
+func ForceRemount() LocalMounterOpt {
+	return func(lm *localMounter) {
+		lm.forceRemount = true
+	}
 }

snapshot/localmounter_unix.go

@@ -5,6 +5,7 @@ package snapshot
 
 import (
 	"os"
+	"path/filepath"
 	"syscall"
 
 	"github.com/containerd/containerd/mount"
@@ -34,30 +35,48 @@ func (lm *localMounter) Mount() (string, error) {
 		}
 	}
 
+	var isFile bool
 	if len(lm.mounts) == 1 && (lm.mounts[0].Type == "bind" || lm.mounts[0].Type == "rbind") {
-		ro := false
-		for _, opt := range lm.mounts[0].Options {
-			if opt == "ro" {
-				ro = true
-				break
+		if !lm.forceRemount {
+			ro := false
+			for _, opt := range lm.mounts[0].Options {
+				if opt == "ro" {
+					ro = true
+					break
+				}
 			}
+			if !ro {
+				return lm.mounts[0].Source, nil
+			}
+		}
+		fi, err := os.Stat(lm.mounts[0].Source)
+		if err != nil {
+			return "", err
 		}
-		if !ro {
-			return lm.mounts[0].Source, nil
+		if !fi.IsDir() {
+			isFile = true
 		}
 	}
 
-	dir, err := os.MkdirTemp("", "buildkit-mount")
+	dest, err := os.MkdirTemp("", "buildkit-mount")
 	if err != nil {
 		return "", errors.Wrap(err, "failed to create temp dir")
 	}
 
-	if err := mount.All(lm.mounts, dir); err != nil {
-		os.RemoveAll(dir)
-		return "", errors.Wrapf(err, "failed to mount %s: %+v", dir, lm.mounts)
+	if isFile {
+		dest = filepath.Join(dest, "file")
+		if err := os.WriteFile(dest, []byte{}, 0644); err != nil {
+			os.RemoveAll(dest)
+			return "", errors.Wrap(err, "failed to create temp file")
+		}
+	}
+
+	if err := mount.All(lm.mounts, dest); err != nil {
+		os.RemoveAll(dest)
+		return "", errors.Wrapf(err, "failed to mount %s: %+v", dest, lm.mounts)
 	}
-	lm.target = dir
-	return dir, nil
+	lm.target = dest
+	return dest, nil
 }
 
 func (lm *localMounter) Unmount() error {