diff --git a/g3doc/user_guide/containerd/configuration.md b/g3doc/user_guide/containerd/configuration.md index a8d5908965..96cebbc406 100644 --- a/g3doc/user_guide/containerd/configuration.md +++ b/g3doc/user_guide/containerd/configuration.md @@ -188,6 +188,12 @@ to handle each volume: - `bind`: Indicates a bind mount from the host. - `dev.gvisor.spec.mount..options`: Comma-separated volume-mount options (e.g., `rw,rprivate`). +- `dev.gvisor.spec.mount..directfs`: `default` or `off`. `default` + (or unset) follows the global `--directfs` setting. `off` suppresses the + `directfs` mount option for this mount even when `--directfs` is enabled + globally. Use this for mounts served by a custom gofer that cannot donate a + host file descriptor for the mount root (for example, virtual or + network-backed filesystems). Other mounts continue to use directfs. Below is an example Pod spec for a shared `emptyDir` volume named `shared-folder`, with annotations that enable cross-container inotify: diff --git a/runsc/boot/mount_hints.go b/runsc/boot/mount_hints.go index e2c31575aa..85596716f5 100644 --- a/runsc/boot/mount_hints.go +++ b/runsc/boot/mount_hints.go @@ -102,7 +102,7 @@ func NewPodMountHints(spec *specs.Spec) (*PodMountHints, error) { // Validate all the parsed hints. for name, m := range mnts { - log.Infof("Mount annotation found, name: %s, source: %q, type: %s, share: %v", name, m.Mount.Source, m.Mount.Type, m.Share) + log.Infof("Mount annotation found, name: %s, source: %q, type: %s, share: %v, suppress_directfs: %t", name, m.Mount.Source, m.Mount.Type, m.Share, m.SuppressDirectFS) if m.Share == invalid || len(m.Mount.Source) == 0 || len(m.Mount.Type) == 0 { log.Warningf("ignoring mount annotations for %q because of missing required field(s)", name) delete(mnts, name) @@ -121,13 +121,20 @@ func NewPodMountHints(spec *specs.Spec) (*PodMountHints, error) { } // MountHint represents extra information about mounts that are provided via -// annotations. They can override mount type, and provide sharing information -// so that mounts can be correctly shared inside the pod. +// annotations. They can override mount type, provide sharing information so +// that mounts can be correctly shared inside the pod, and tune gofer-specific +// behavior such as suppressing directfs. // It is part of the sandbox.Sandbox struct, so it must be serializable. type MountHint struct { Name string `json:"name"` Share ShareType `json:"share"` Mount specs.Mount `json:"mount"` + + // SuppressDirectFS suppresses the "directfs" gofer mount option for this + // mount even if --directfs is enabled globally. It does not enable directfs + // when --directfs is disabled because directfs requires sandbox, sentry, and + // gofer setup that is keyed off the global --directfs flag. + SuppressDirectFS bool `json:"suppressDirectFS"` } func (m *MountHint) setField(key, val string) error { @@ -143,6 +150,8 @@ func (m *MountHint) setField(key, val string) error { return m.setShare(val) case "options": m.Mount.Options = specutils.FilterMountOptions(strings.Split(val, ",")) + case "directfs": + return m.setDirectFS(val) default: return fmt.Errorf("invalid mount annotation: %s=%s", key, val) } @@ -173,6 +182,18 @@ func (m *MountHint) setShare(val string) error { return nil } +func (m *MountHint) setDirectFS(val string) error { + switch val { + case "default": + m.SuppressDirectFS = false + case "off": + m.SuppressDirectFS = true + default: + return fmt.Errorf("invalid directfs value %q, want \"default\" or \"off\"", val) + } + return nil +} + // ShouldShareMount returns true if this mount should be configured as a shared // mount that is shared among multiple containers in a pod. func (m *MountHint) ShouldShareMount() bool { diff --git a/runsc/boot/mount_hints_test.go b/runsc/boot/mount_hints_test.go index df41658618..8ef54e83fc 100644 --- a/runsc/boot/mount_hints_test.go +++ b/runsc/boot/mount_hints_test.go @@ -59,6 +59,9 @@ func TestPodMountHintsHappy(t *testing.T) { if want := []string(nil); !slices.Equal(want, mount1.Mount.Options) { t.Errorf("mount1 type, want: %q, got: %q", want, mount1.Mount.Options) } + if mount1.SuppressDirectFS { + t.Errorf("mount1 SuppressDirectFS, want: false, got: true") + } mount2 := podHints.Mounts["mount2"] if want := "mount2"; want != mount2.Name { @@ -171,6 +174,77 @@ func TestPodMountHintsIgnore(t *testing.T) { } } +func TestPodMountHintsDirectFSInvalid(t *testing.T) { + spec := &specs.Spec{ + Annotations: map[string]string{ + MountPrefix + "mount1.source": "foo", + MountPrefix + "mount1.type": "bind", + MountPrefix + "mount1.share": "container", + MountPrefix + "mount1.directfs": "maybe", + + MountPrefix + "mount2.source": "bar", + MountPrefix + "mount2.type": "bind", + MountPrefix + "mount2.share": "container", + MountPrefix + "mount2.directfs": "off", + }, + } + podHints, err := NewPodMountHints(spec) + if err != nil { + t.Fatalf("NewPodMountHints failed: %v", err) + } + mount1, ok := podHints.Mounts["mount1"] + if !ok { + t.Fatalf("mount1 hint should be retained when directfs value is invalid") + } + if mount1.SuppressDirectFS { + t.Errorf("mount1 SuppressDirectFS, want: false, got: true") + } + mount2, ok := podHints.Mounts["mount2"] + if !ok { + t.Fatalf("mount2 hint should be retained when sibling has invalid directfs") + } + if !mount2.SuppressDirectFS { + t.Errorf("mount2 SuppressDirectFS, want: true, got: false") + } +} + +func TestPodMountHintsDirectFS(t *testing.T) { + for _, tc := range []struct { + name string + value string + suppress bool + }{ + { + name: "default", + value: "default", + suppress: false, + }, + { + name: "off", + value: "off", + suppress: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + spec := &specs.Spec{ + Annotations: map[string]string{ + MountPrefix + "mount1.source": "foo", + MountPrefix + "mount1.type": "bind", + MountPrefix + "mount1.share": "container", + MountPrefix + "mount1.directfs": tc.value, + }, + } + podHints, err := NewPodMountHints(spec) + if err != nil { + t.Fatalf("NewPodMountHints failed: %v", err) + } + if got := podHints.Mounts["mount1"].SuppressDirectFS; got != tc.suppress { + t.Errorf("SuppressDirectFS = %t, want %t", got, tc.suppress) + } + }) + } +} + func TestIgnoreInvalidMountOptions(t *testing.T) { spec := &specs.Spec{ Annotations: map[string]string{ diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index 3a8ed80075..cca7bf1886 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -322,7 +322,7 @@ func compileMounts(spec *specs.Spec, conf *config.Config, containerID string) [] } // goferMountData creates a slice of gofer mount data. -func goferMountData(fd int, fa config.FileAccessType, conf *config.Config) []string { +func goferMountData(fd int, fa config.FileAccessType, conf *config.Config, hint *MountHint) []string { opts := []string{ "trans=fd", "rfdno=" + strconv.Itoa(fd), @@ -331,7 +331,7 @@ func goferMountData(fd int, fa config.FileAccessType, conf *config.Config) []str if fa == config.FileAccessShared { opts = append(opts, "cache=remote_revalidating") } - if conf.DirectFS { + if conf.DirectFS && (hint == nil || !hint.SuppressDirectFS) { opts = append(opts, "directfs") } if !conf.HostFifo.AllowOpen() { @@ -514,7 +514,7 @@ func (c *containerMounter) createMountNamespace(ctx context.Context, conf *confi case rootfsConf.ShouldUseLisafs(): fsName = gofer.Name - data := goferMountData(ioFD, conf.FileAccess, conf) + data := goferMountData(ioFD, conf.FileAccess, conf, nil) // We can't check for overlayfs here because sandbox is chroot'ed and gofer // can only send mount options for specs.Mounts (specs.Root is missing @@ -1014,7 +1014,7 @@ func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo, if err != nil { return "", nil, err } - data = append(data, goferMountData(m.goferFD.Release(), getMountAccessType(conf, m.hint), conf)...) + data = append(data, goferMountData(m.goferFD.Release(), getMountAccessType(conf, m.hint), conf, m.hint)...) internalData = gofer.InternalFilesystemOptions{ UniqueID: checkpoint.ResourceID{ ContainerName: containerName, diff --git a/runsc/boot/vfs_test.go b/runsc/boot/vfs_test.go index fa22eb60d6..0709db8eb9 100644 --- a/runsc/boot/vfs_test.go +++ b/runsc/boot/vfs_test.go @@ -15,6 +15,7 @@ package boot import ( + "slices" "testing" specs "github.com/opencontainers/runtime-spec/specs-go" @@ -97,6 +98,55 @@ func TestGetMountAccessType(t *testing.T) { } } +func TestGoferMountDataDirectFS(t *testing.T) { + for _, tc := range []struct { + name string + directFS bool + hint *MountHint + wantEnabled bool + }{ + { + name: "global on, no hint", + directFS: true, + hint: nil, + wantEnabled: true, + }, + { + name: "global on, default hint", + directFS: true, + hint: &MountHint{}, + wantEnabled: true, + }, + { + name: "global on, hint suppresses", + directFS: true, + hint: &MountHint{SuppressDirectFS: true}, + wantEnabled: false, + }, + { + name: "global off, hint suppresses", + directFS: false, + hint: &MountHint{SuppressDirectFS: true}, + wantEnabled: false, + }, + { + name: "global off, no hint", + directFS: false, + hint: nil, + wantEnabled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + conf := &config.Config{DirectFS: tc.directFS, HostFifo: config.HostFifoOpen} + opts := goferMountData(7, config.FileAccessExclusive, conf, tc.hint) + gotEnabled := slices.Contains(opts, "directfs") + if gotEnabled != tc.wantEnabled { + t.Errorf("directfs option present = %t, want %t (opts=%v)", gotEnabled, tc.wantEnabled, opts) + } + }) + } +} + func TestCgroupfsCPUDefaults(t *testing.T) { for _, tc := range []struct { name string diff --git a/runsc/cmd/BUILD b/runsc/cmd/BUILD index bb747d8162..9b575c3d3c 100644 --- a/runsc/cmd/BUILD +++ b/runsc/cmd/BUILD @@ -167,6 +167,7 @@ go_test( "//pkg/sentry/control", "//pkg/sentry/kernel/auth", "//pkg/test/testutil", + "//runsc/boot", "//runsc/cmd/sandboxsetup", "//runsc/cmd/util", "//runsc/config", diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index bb465c0d9d..20b7f8485c 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -28,6 +28,7 @@ import ( "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/unet" "gvisor.dev/gvisor/pkg/urpc" + "gvisor.dev/gvisor/runsc/boot" "gvisor.dev/gvisor/runsc/cmd/sandboxsetup" "gvisor.dev/gvisor/runsc/cmd/util" "gvisor.dev/gvisor/runsc/config" @@ -172,6 +173,11 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomm if err != nil { util.Fatalf("reading spec: %v", err) } + mountHints, err := boot.NewPodMountHints(spec) + if err != nil { + util.Fatalf("parsing mount hints: %v", err) + } + lisafsNeeded := lisafsNeededForDirectFSSuppression(spec, mountHints, g.mountConfs) g.syncFDs.syncChroot() g.syncFDs.syncUsernsForRootless(uint32(g.uid), uint32(g.gid)) @@ -291,6 +297,7 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomm UDSCreateEnabled: conf.GetHostUDS().AllowCreate(), ProfileEnabled: profileOpts.Enabled(), DirectFS: conf.DirectFS, + LisafsNeeded: lisafsNeeded, CgoEnabled: config.CgoEnabled, } if err := filter.Install(opts); err != nil { @@ -388,6 +395,34 @@ func (g *Gofer) serve(spec *specs.Spec, conf *config.Config, root string, ruid i return subcommands.ExitSuccess } +// lisafsNeededForDirectFSSuppression returns true if this gofer serves a mount +// that suppresses directfs and therefore still needs LisaFS syscalls. +func lisafsNeededForDirectFSSuppression(spec *specs.Spec, mountHints *boot.PodMountHints, mountConfs []specutils.GoferMountConf) bool { + if mountHints == nil { + return false + } + mountIdx := 1 // First mount config is the root. + for _, m := range spec.Mounts { + if !specutils.HasMountConfig(m) { + continue + } + if mountIdx >= len(mountConfs) { + // Invalid mount config counts are rejected elsewhere. Keep LisaFS + // filters when the gofer cannot prove they are unnecessary. + return true + } + mountConf := mountConfs[mountIdx] + mountIdx++ + if !mountConf.ShouldUseLisafs() { + continue + } + if hint := mountHints.FindMount(m.Source); hint != nil && hint.SuppressDirectFS { + return true + } + } + return false +} + // makeRPCMountOpener returns a MountOpener that opens mount sources via the // gofer-to-host RPC channel. func makeRPCMountOpener(goferToHostRPC *urpc.Client) sandboxsetup.MountOpener { diff --git a/runsc/cmd/gofer_test.go b/runsc/cmd/gofer_test.go index d3eebbe016..a731f813d0 100644 --- a/runsc/cmd/gofer_test.go +++ b/runsc/cmd/gofer_test.go @@ -21,7 +21,10 @@ import ( "path/filepath" "testing" + specs "github.com/opencontainers/runtime-spec/specs-go" + "gvisor.dev/gvisor/runsc/boot" "gvisor.dev/gvisor/runsc/cmd/sandboxsetup" + "gvisor.dev/gvisor/runsc/specutils" ) func tmpDir() string { @@ -162,3 +165,78 @@ func TestResolveSymlinksLoop(t *testing.T) { t.Errorf("ResolveSymlinks() should have failed") } } + +func TestLisafsNeededForDirectFSSuppression(t *testing.T) { + for _, tc := range []struct { + name string + spec *specs.Spec + mountConfs []specutils.GoferMountConf + want bool + }{ + { + name: "no suppression", + spec: suppressedMountSpec("foo", "default"), + mountConfs: lisafsMountConfs(), + }, + { + name: "suppressed lisafs mount", + spec: suppressedMountSpec("foo", "off"), + mountConfs: lisafsMountConfs(), + want: true, + }, + { + name: "suppressed mount not in spec", + spec: suppressedMountSpec("bar", "off"), + mountConfs: lisafsMountConfs(), + }, + { + name: "suppressed erofs mount", + spec: suppressedMountSpec("foo", "off"), + mountConfs: []specutils.GoferMountConf{ + {Lower: specutils.Lisafs, Upper: specutils.NoOverlay}, + {Lower: specutils.Erofs, Upper: specutils.NoOverlay}, + }, + }, + { + name: "missing mount config", + spec: suppressedMountSpec("foo", "off"), + mountConfs: []specutils.GoferMountConf{{Lower: specutils.Lisafs, Upper: specutils.NoOverlay}}, + want: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + mountHints, err := boot.NewPodMountHints(tc.spec) + if err != nil { + t.Fatalf("NewPodMountHints failed: %v", err) + } + if got := lisafsNeededForDirectFSSuppression(tc.spec, mountHints, tc.mountConfs); got != tc.want { + t.Errorf("lisafsNeededForDirectFSSuppression = %t, want %t", got, tc.want) + } + }) + } +} + +func suppressedMountSpec(hintSource, directfs string) *specs.Spec { + return &specs.Spec{ + Annotations: map[string]string{ + boot.MountPrefix + "mount1.source": hintSource, + boot.MountPrefix + "mount1.type": "bind", + boot.MountPrefix + "mount1.share": "container", + boot.MountPrefix + "mount1.directfs": directfs, + }, + Mounts: []specs.Mount{ + { + Source: "foo", + Type: "bind", + Destination: "/mnt", + }, + }, + } +} + +func lisafsMountConfs() []specutils.GoferMountConf { + return []specutils.GoferMountConf{ + {Lower: specutils.Lisafs, Upper: specutils.NoOverlay}, + {Lower: specutils.Lisafs, Upper: specutils.NoOverlay}, + } +} diff --git a/runsc/fsgofer/filter/BUILD b/runsc/fsgofer/filter/BUILD index c54cf62ec6..16ad30e556 100644 --- a/runsc/fsgofer/filter/BUILD +++ b/runsc/fsgofer/filter/BUILD @@ -1,4 +1,4 @@ -load("//tools:defs.bzl", "go_library") +load("//tools:defs.bzl", "go_library", "go_test") package( default_applicable_licenses = ["//:license"], @@ -31,3 +31,13 @@ go_library( "@org_golang_x_sys//unix:go_default_library", ], ) + +go_test( + name = "filter_test", + size = "small", + srcs = ["filter_test.go"], + library = ":filter", + deps = [ + "@org_golang_x_sys//unix:go_default_library", + ], +) diff --git a/runsc/fsgofer/filter/filter.go b/runsc/fsgofer/filter/filter.go index 12b3aab5b8..617b00f670 100644 --- a/runsc/fsgofer/filter/filter.go +++ b/runsc/fsgofer/filter/filter.go @@ -28,6 +28,7 @@ type Options struct { UDSCreateEnabled bool ProfileEnabled bool DirectFS bool + LisafsNeeded bool CgoEnabled bool } @@ -64,8 +65,9 @@ func Rules(opt Options) seccomp.SyscallRules { // when not enabled. s.Merge(instrumentationFilters()) - // When DirectFS is not enabled, filters for LisaFS are installed. - if !opt.DirectFS { + // When DirectFS is not enabled, filters for LisaFS are installed. They are + // also needed when this gofer serves a mount that suppresses DirectFS. + if !opt.DirectFS || opt.LisafsNeeded { s.Merge(lisafsFilters) } diff --git a/runsc/fsgofer/filter/filter_test.go b/runsc/fsgofer/filter/filter_test.go new file mode 100644 index 0000000000..c80375dcf2 --- /dev/null +++ b/runsc/fsgofer/filter/filter_test.go @@ -0,0 +1,58 @@ +// Copyright 2026 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package filter + +import ( + "testing" + + "golang.org/x/sys/unix" +) + +func TestRulesLisaFSFilters(t *testing.T) { + for _, tc := range []struct { + name string + directFS bool + lisafsNeeded bool + wantAllowed bool + }{ + { + name: "directfs disabled", + wantAllowed: true, + }, + { + name: "directfs enabled", + directFS: true, + wantAllowed: false, + }, + { + name: "directfs enabled with lisafs mount", + directFS: true, + lisafsNeeded: true, + wantAllowed: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + rules := Rules(Options{ + DirectFS: tc.directFS, + LisafsNeeded: tc.lisafsNeeded, + }) + // SYS_FGETXATTR is part of lisafsFilters but not the base gofer + // rules, so it proves whether the LisaFS filter block was merged. + if got := rules.Has(unix.SYS_FGETXATTR); got != tc.wantAllowed { + t.Errorf("Rules().Has(SYS_FGETXATTR) = %t, want %t", got, tc.wantAllowed) + } + }) + } +}