Skip to content

Commit

Permalink
exec: add extra validation for submount sources
Browse files Browse the repository at this point in the history
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)
Signed-off-by: Andrey Epifanov <[email protected]>

# Conflicts:
#	executor/oci/spec.go
#	executor/oci/spec_windows.go
#	snapshot/localmounter_unix.go
  • Loading branch information
tonistiigi authored and aepifanov committed Feb 14, 2024
1 parent c635d0a commit 254b150
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 36 deletions.
32 changes: 19 additions & 13 deletions executor/oci/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@ import (
"github.com/containerd/containerd/mount"
"github.com/containerd/containerd/namespaces"
"github.com/containerd/containerd/oci"
"github.com/containerd/continuity/fs"
"github.com/docker/docker/pkg/idtools"
"github.com/mitchellh/hashstructure/v2"
"github.com/moby/buildkit/executor"
"github.com/moby/buildkit/snapshot"
"github.com/moby/buildkit/util/network"
traceexec "github.com/moby/buildkit/util/tracing/exec"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/selinux/go-selinux"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -198,6 +197,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 {
Expand All @@ -216,10 +216,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
}

Expand All @@ -244,12 +251,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
}

Expand All @@ -259,6 +271,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()
}()
Expand All @@ -267,15 +282,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 {
Expand Down
15 changes: 15 additions & 0 deletions executor/oci/spec_freebsd.go
Original file line number Diff line number Diff line change
@@ -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
}
57 changes: 57 additions & 0 deletions executor/oci/spec_linux.go
Original file line number Diff line number Diff line change
@@ -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
}
}
14 changes: 14 additions & 0 deletions executor/oci/spec_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
package oci

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"
"github.com/pkg/errors"
Expand Down Expand Up @@ -43,3 +48,12 @@ func generateRlimitOpts(ulimits []*pb.Ulimit) ([]oci.SpecOpts, error) {
}
return nil, errors.New("no support for POSIXRlimit on Windows")
}

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
}
35 changes: 26 additions & 9 deletions snapshot/localmounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
46 changes: 32 additions & 14 deletions snapshot/localmounter_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
package snapshot

import (
"io/ioutil"
"os"
"path/filepath"
"syscall"

"github.com/containerd/containerd/mount"
Expand All @@ -25,30 +25,48 @@ func (lm *localMounter) Mount() (string, error) {
lm.release = release
}

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 := ioutil.TempDir("", "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 {
Expand Down

0 comments on commit 254b150

Please sign in to comment.