From a8a46b4c7f36896dc5fdc206ebda5605e9e60c98 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Fri, 3 Nov 2023 17:06:53 -0700 Subject: [PATCH] Only tmpfs is allowed on an upper level of overlayfs All other file systems don't support whiteouts and trusted.overlay attributes. This restriction is applied only from mounts created from inside the sandbox. We need this change to support applications such as Docker that is trying to construct overlay mounts and falls back to other options if it fails. PiperOrigin-RevId: 579343274 --- pkg/sentry/fsimpl/overlay/overlay.go | 8 +++++++ pkg/sentry/kernel/kernel.go | 1 + pkg/sentry/vfs/filesystem_type.go | 6 +++++ pkg/sentry/vfs/mount.go | 2 +- pkg/sentry/vfs/options.go | 6 ----- runsc/boot/controller.go | 4 ++-- runsc/boot/vfs.go | 17 ++++++++------ test/image/image_test.go | 34 ++++++++++++++++++++-------- 8 files changed, 52 insertions(+), 26 deletions(-) diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go index 8628ff108..a99277267 100644 --- a/pkg/sentry/fsimpl/overlay/overlay.go +++ b/pkg/sentry/fsimpl/overlay/overlay.go @@ -205,6 +205,14 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt ctx.Infof("overlay.FilesystemType.GetFilesystem: failed to resolve upperdir %q: %v", upperPathname, err) return nil, nil, err } + // TODO(b/286942303): Only tmpfs supports whiteouts and + // trusted.overlay attributes. Don't allow to use non-tmpfs + // mounts on upper levels for mounts created through the mount + // syscall. In gVisor configs, users can specify any + // configurations on their own risk. + if !opts.InternalMount && upperRoot.Mount().Filesystem().FilesystemType().Name() != "tmpfs" { + return nil, nil, linuxerr.EINVAL + } privateUpperRoot, err := clonePrivateMount(vfsObj, upperRoot, false /* forceReadOnly */) upperRoot.DecRef(ctx) if err != nil { diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index 22e1b0827..ae8751d05 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -456,6 +456,7 @@ func (k *Kernel) Init(args InitKernelArgs) error { // value for sbinfo->max_blocks when SB_KERNMOUNT is set. DisableDefaultSizeLimit: true, }, + InternalMount: true, } tmpfsFilesystem, tmpfsRoot, err := tmpfs.FilesystemType{}.GetFilesystem(ctx, &k.vfs, auth.NewRootCredentials(k.rootUserNamespace), "", tmpfsOpts) if err != nil { diff --git a/pkg/sentry/vfs/filesystem_type.go b/pkg/sentry/vfs/filesystem_type.go index 583de11ff..020f19a15 100644 --- a/pkg/sentry/vfs/filesystem_type.go +++ b/pkg/sentry/vfs/filesystem_type.go @@ -41,6 +41,12 @@ type FilesystemType interface { // GetFilesystemOptions contains options to FilesystemType.GetFilesystem. type GetFilesystemOptions struct { + // InternalMount indicates whether the mount operation is coming from the + // application, i.e. through mount(2). If InternalMount is true, allow the use + // of filesystem types for which RegisterFilesystemTypeOptions.AllowUserMount + // == false. + InternalMount bool + // Data is the string passed as the 5th argument to mount(2), which is // usually a comma-separated list of filesystem-specific mount options. Data string diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go index 291a7614c..451db54dd 100644 --- a/pkg/sentry/vfs/mount.go +++ b/pkg/sentry/vfs/mount.go @@ -224,7 +224,7 @@ func (vfs *VirtualFilesystem) NewFilesystem(ctx context.Context, creds *auth.Cre if rft == nil { return nil, nil, linuxerr.ENODEV } - if !opts.InternalMount && !rft.opts.AllowUserMount { + if !opts.GetFilesystemOptions.InternalMount && !rft.opts.AllowUserMount { return nil, nil, linuxerr.ENODEV } return rft.fsType.GetFilesystem(ctx, vfs, creds, source, opts.GetFilesystemOptions) diff --git a/pkg/sentry/vfs/options.go b/pkg/sentry/vfs/options.go index bc9583896..044bd4f17 100644 --- a/pkg/sentry/vfs/options.go +++ b/pkg/sentry/vfs/options.go @@ -112,12 +112,6 @@ type MountOptions struct { // GetFilesystemOptions contains options to FilesystemType.GetFilesystem(). GetFilesystemOptions GetFilesystemOptions - - // InternalMount indicates whether the mount operation is coming from the - // application, i.e. through mount(2). If InternalMount is true, allow the use - // of filesystem types for which RegisterFilesystemTypeOptions.AllowUserMount - // == false. - InternalMount bool } // OpenOptions contains options to VirtualFilesystem.OpenAt() and diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 9ddb7d7f3..501f1648d 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -754,9 +754,9 @@ func (cm *containerManager) Mount(args *MountArgs, _ *struct{}) error { opts = vfs.MountOptions{ ReadOnly: true, GetFilesystemOptions: vfs.GetFilesystemOptions{ - Data: fmt.Sprintf("ifd=%d", imageFD), + InternalMount: true, + Data: fmt.Sprintf("ifd=%d", imageFD), }, - InternalMount: true, } default: diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index 8d777bdae..aca0ed12d 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -461,12 +461,12 @@ func (c *containerMounter) createMountNamespace(ctx context.Context, conf *confi opts := &vfs.MountOptions{ ReadOnly: c.root.Readonly, GetFilesystemOptions: vfs.GetFilesystemOptions{ - Data: strings.Join(data, ","), + InternalMount: true, + Data: strings.Join(data, ","), InternalData: gofer.InternalFilesystemOptions{ UniqueID: "/", }, }, - InternalMount: true, } fsName := gofer.Name @@ -529,10 +529,10 @@ func (c *containerMounter) configureOverlay(ctx context.Context, conf *config.Co // First copy options from lower layer to upper layer and overlay. Clear // filesystem specific options. upperOpts := *lowerOpts - upperOpts.GetFilesystemOptions = vfs.GetFilesystemOptions{} + upperOpts.GetFilesystemOptions = vfs.GetFilesystemOptions{InternalMount: true} overlayOpts := *lowerOpts - overlayOpts.GetFilesystemOptions = vfs.GetFilesystemOptions{} + overlayOpts.GetFilesystemOptions = vfs.GetFilesystemOptions{InternalMount: true} // All writes go to the upper layer, be paranoid and make lower readonly. lowerOpts.ReadOnly = true @@ -844,8 +844,9 @@ func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo, opts := ParseMountOptions(m.mount.Options) opts.GetFilesystemOptions = vfs.GetFilesystemOptions{ - Data: strings.Join(data, ","), - InternalData: internalData, + Data: strings.Join(data, ","), + InternalData: internalData, + InternalMount: true, } return fsName, opts, nil @@ -854,7 +855,9 @@ func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo, // ParseMountOptions converts specs.Mount.Options to vfs.MountOptions. func ParseMountOptions(opts []string) *vfs.MountOptions { mountOpts := &vfs.MountOptions{ - InternalMount: true, + GetFilesystemOptions: vfs.GetFilesystemOptions{ + InternalMount: true, + }, } // Note: update mountHint.CheckCompatible when more options are added. for _, o := range opts { diff --git a/test/image/image_test.go b/test/image/image_test.go index 20bfc51cf..6be773c52 100644 --- a/test/image/image_test.go +++ b/test/image/image_test.go @@ -320,7 +320,17 @@ func TestStdio(t *testing.T) { } } +func TestDockerOverlay(t *testing.T) { + testDocker(t, true) +} + func TestDocker(t *testing.T) { + // Overlayfs can't be built on top of another overlayfs, so docket has + // to fall back to the vfs driver. + testDocker(t, false) +} + +func testDocker(t *testing.T, overlay bool) { if testutil.IsRunningWithHostNet() { t.Skip("docker doesn't work with hostinet") } @@ -332,24 +342,28 @@ func TestDocker(t *testing.T) { opts := dockerutil.RunOpts{ Image: "basic/docker", Privileged: true, - Mounts: []mount.Mount{ + } + if overlay { + opts.Mounts = []mount.Mount{ { Target: "/var/lib/docker", Type: mount.TypeTmpfs, }, - }, + } } if err := d.Spawn(ctx, opts); err != nil { t.Fatalf("docker run failed: %v", err) } - // Docker creates tmpfs mounts with the noexec flag. - output, err := d.Exec(ctx, - dockerutil.ExecOpts{Privileged: true}, - "mount", "-o", "remount,exec", "/var/lib/docker", - ) - if err != nil { - t.Fatalf("docker exec failed: %v\n%s", err, output) + if overlay { + // Docker creates tmpfs mounts with the noexec flag. + output, err := d.Exec(ctx, + dockerutil.ExecOpts{Privileged: true}, + "mount", "-o", "remount,exec", "/var/lib/docker", + ) + if err != nil { + t.Fatalf("docker exec failed: %v\n%s", err, output) + } } // Wait for the docker daemon. for i := 0; i < 10; i++ { @@ -365,7 +379,7 @@ func TestDocker(t *testing.T) { p, err := d.ExecProcess(ctx, dockerutil.ExecOpts{}, "docker", "run", "--network", "host", "--rm", "alpine", "echo", "Hello World") if err != nil { - t.Fatalf("docker exec failed: %v\n%s", err, output) + t.Fatalf("docker exec failed: %v", err) } stdout, stderr, err := p.Read() t.Logf("Container output: == stdout ==\n%s\n== stderr ==\n%s", stdout, stderr)