diff --git a/virtcontainers/container.go b/virtcontainers/container.go index b5517da1cd..1f5d2c4e5f 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -481,7 +481,7 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s } else { // These mounts are created in the shared dir mountDest := filepath.Join(hostSharedDir, c.sandbox.id, filename) - if err := bindMount(c.ctx, m.Source, mountDest, false); err != nil { + if err := bindMount(c.ctx, m.Source, mountDest, false, "private"); err != nil { return "", false, err } // Save HostPath mount value into the mount list of the container. diff --git a/virtcontainers/container_test.go b/virtcontainers/container_test.go index 7eaa245962..acbb02264f 100644 --- a/virtcontainers/container_test.go +++ b/virtcontainers/container_test.go @@ -174,15 +174,15 @@ func TestUnmountHostMountsRemoveBindHostPath(t *testing.T) { ctx: context.Background(), } - if err := bindMount(c.ctx, src, hostPath, false); err != nil { + if err := bindMount(c.ctx, src, hostPath, false, "private"); err != nil { t.Fatal(err) } defer syscall.Unmount(hostPath, 0) - if err := bindMount(c.ctx, src, nonEmptyHostpath, false); err != nil { + if err := bindMount(c.ctx, src, nonEmptyHostpath, false, "private"); err != nil { t.Fatal(err) } defer syscall.Unmount(nonEmptyHostpath, 0) - if err := bindMount(c.ctx, src, devPath, false); err != nil { + if err := bindMount(c.ctx, src, devPath, false, "private"); err != nil { t.Fatal(err) } defer syscall.Unmount(devPath, 0) diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go index 653eae1824..09fda7b160 100644 --- a/virtcontainers/fc.go +++ b/virtcontainers/fc.go @@ -180,46 +180,6 @@ func (fc *firecracker) trace(name string) (opentracing.Span, context.Context) { return span, ctx } -// bindMount bind mounts a source in to a destination. This will -// do some bookkeeping: -// * evaluate all symlinks -// * ensure the source exists -// * recursively create the destination -func (fc *firecracker) bindMount(ctx context.Context, source, destination string, readonly bool) error { - span, _ := trace(ctx, "bindMount") - defer span.Finish() - - if source == "" { - return fmt.Errorf("source must be specified") - } - if destination == "" { - return fmt.Errorf("destination must be specified") - } - - absSource, err := filepath.EvalSymlinks(source) - if err != nil { - return fmt.Errorf("Could not resolve symlink for source %v", source) - } - - if err := ensureDestinationExists(absSource, destination); err != nil { - return fmt.Errorf("Could not create destination mount point %v: %v", destination, err) - } - - fc.Logger().WithFields(logrus.Fields{"src": absSource, "dst": destination}).Debug("Bind mounting resource") - - if err := syscall.Mount(absSource, destination, "bind", syscall.MS_BIND|syscall.MS_SLAVE, ""); err != nil { - return fmt.Errorf("Could not bind mount %v to %v: %v", absSource, destination, err) - } - - // For readonly bind mounts, we need to remount with the readonly flag. - // This is needed as only very recent versions of libmount/util-linux support "bind,ro" - if readonly { - return syscall.Mount(absSource, destination, "bind", uintptr(syscall.MS_BIND|syscall.MS_SLAVE|syscall.MS_REMOUNT|syscall.MS_RDONLY), "") - } - - return nil -} - // For firecracker this call only sets the internal structure up. // The sandbox will be created and started through startSandbox(). func (fc *firecracker) createSandbox(ctx context.Context, id string, networkNS NetworkNamespace, hypervisorConfig *HypervisorConfig, stateful bool) error { @@ -371,19 +331,7 @@ func (fc *firecracker) fcInit(timeout int) error { span, _ := fc.trace("fcInit") defer span.Finish() - // Fetch sandbox network to be able to access it from the sandbox structure. - err := os.MkdirAll(fc.jailerRoot, DirMode) - if err != nil { - return err - } - defer func() { - if err != nil { - if err := os.RemoveAll(fc.vmPath); err != nil { - fc.Logger().WithError(err).Error("Fail to clean up vm directory") - } - } - }() - + var err error //FC version set and check if fc.info.Version, err = fc.getVersionNumber(); err != nil { return err @@ -537,13 +485,31 @@ func (fc *firecracker) createJailedDrive(name string) (string, error) { return r, nil } +// when running with jailer, firecracker binary will firstly be copied into fc.jailerRoot, +// and then being executed there. Therefore we need to ensure fc.JailerRoot has exec permissions. +func (fc *firecracker) fcRemountJailerRootWithExec() error { + if err := bindMount(context.Background(), fc.jailerRoot, fc.jailerRoot, false, "shared"); err != nil { + fc.Logger().WithField("JailerRoot", fc.jailerRoot).Errorf("bindMount failed: %v", err) + return err + } + + // /run is normally mounted with rw, nosuid(MS_NOSUID), relatime(MS_RELATIME), noexec(MS_NOEXEC). + // we re-mount jailerRoot to deliberately leave out MS_NOEXEC. + if err := remount(context.Background(), syscall.MS_NOSUID|syscall.MS_RELATIME, fc.jailerRoot); err != nil { + fc.Logger().WithField("JailerRoot", fc.jailerRoot).Errorf("Re-mount failed: %v", err) + return err + } + + return nil +} + func (fc *firecracker) fcJailResource(src, dst string) (string, error) { if src == "" || dst == "" { return "", fmt.Errorf("fcJailResource: invalid jail locations: src:%v, dst:%v", src, dst) } jailedLocation := filepath.Join(fc.jailerRoot, dst) - if err := fc.bindMount(context.Background(), src, jailedLocation, false); err != nil { + if err := bindMount(context.Background(), src, jailedLocation, false, "slave"); err != nil { fc.Logger().WithField("bindMount failed", err).Error() return "", err } @@ -685,8 +651,23 @@ func (fc *firecracker) fcListenToFifo(fifoName string) (string, error) { } func (fc *firecracker) fcInitConfiguration() error { + err := os.MkdirAll(fc.jailerRoot, DirMode) + if err != nil { + return err + } + defer func() { + if err != nil { + if err := os.RemoveAll(fc.vmPath); err != nil { + fc.Logger().WithError(err).Error("Fail to clean up vm directory") + } + } + }() + if fc.config.JailerPath != "" { fc.jailed = true + if err := fc.fcRemountJailerRootWithExec(); err != nil { + return nil + } } fc.fcSetVMBaseConfig(int64(fc.config.MemorySize), @@ -841,6 +822,12 @@ func (fc *firecracker) cleanupJail() { fc.umountResource(fcLogFifo) fc.umountResource(fcMetricsFifo) fc.umountResource(defaultFcConfig) + // if running with jailer, we also need to umount fc.jailerRoot + if fc.config.JailerPath != "" { + if err := syscall.Unmount(fc.jailerRoot, syscall.MNT_DETACH); err != nil { + fc.Logger().WithField("JailerRoot", fc.jailerRoot).WithError(err).Error("Failed to umount") + } + } fc.Logger().WithField("cleaningJail", fc.vmPath).Info() if err := os.RemoveAll(fc.vmPath); err != nil { diff --git a/virtcontainers/mount.go b/virtcontainers/mount.go index c6827ba037..fc06424c94 100644 --- a/virtcontainers/mount.go +++ b/virtcontainers/mount.go @@ -31,6 +31,13 @@ var rootfsDir = "rootfs" var systemMountPrefixes = []string{"/proc", "/sys"} +var propagationTypes = map[string]uintptr{ + "shared": syscall.MS_SHARED, + "private": syscall.MS_PRIVATE, + "slave": syscall.MS_SLAVE, + "ubind": syscall.MS_UNBINDABLE, +} + func isSystemMount(m string) bool { for _, p := range systemMountPrefixes { if m == p || strings.HasPrefix(m, p+"/") { @@ -260,7 +267,8 @@ const mountPerm = os.FileMode(0755) // * evaluate all symlinks // * ensure the source exists // * recursively create the destination -func bindMount(ctx context.Context, source, destination string, readonly bool) error { +// pgtypes stands for propagation types, which are shared, private, slave, and ubind. +func bindMount(ctx context.Context, source, destination string, readonly bool, pgtypes string) error { span, _ := trace(ctx, "bindMount") defer span.Finish() @@ -284,8 +292,12 @@ func bindMount(ctx context.Context, source, destination string, readonly bool) e return fmt.Errorf("Could not bind mount %v to %v: %v", absSource, destination, err) } - if err := syscall.Mount("none", destination, "", syscall.MS_PRIVATE, ""); err != nil { - return fmt.Errorf("Could not make mount point %v private: %v", destination, err) + if pgtype, exist := propagationTypes[pgtypes]; exist { + if err := syscall.Mount("none", destination, "", pgtype, ""); err != nil { + return fmt.Errorf("Could not make mount point %v %s: %v", destination, pgtypes, err) + } + } else { + return fmt.Errorf("Wrong propagation type %s", pgtypes) } // For readonly bind mounts, we need to remount with the readonly flag. @@ -297,6 +309,24 @@ func bindMount(ctx context.Context, source, destination string, readonly bool) e return nil } +// An existing mount may be remounted by specifying `MS_REMOUNT` in +// mountflags. +// This allows you to change the mountflags of an existing mount. +// The mountflags should match the values used in the original mount() call, +// except for those parameters that you are trying to change. +func remount(ctx context.Context, mountflags uintptr, src string) error { + absSrc, err := filepath.EvalSymlinks(src) + if err != nil { + return fmt.Errorf("Could not resolve symlink for %s", src) + } + + if err := syscall.Mount(absSrc, absSrc, "", syscall.MS_REMOUNT|mountflags, ""); err != nil { + return fmt.Errorf("remount %s failed: %v", absSrc, err) + } + + return nil +} + // bindMountContainerRootfs bind mounts a container rootfs into a 9pfs shared // directory between the guest and the host. func bindMountContainerRootfs(ctx context.Context, sharedDir, sandboxID, cID, cRootFs string, readonly bool) error { @@ -305,7 +335,7 @@ func bindMountContainerRootfs(ctx context.Context, sharedDir, sandboxID, cID, cR rootfsDest := filepath.Join(sharedDir, sandboxID, cID, rootfsDir) - return bindMount(ctx, cRootFs, rootfsDest, readonly) + return bindMount(ctx, cRootFs, rootfsDest, readonly, "private") } // Mount describes a container mount. diff --git a/virtcontainers/mount_test.go b/virtcontainers/mount_test.go index dd3607d423..fc394cc558 100644 --- a/virtcontainers/mount_test.go +++ b/virtcontainers/mount_test.go @@ -189,7 +189,7 @@ func TestGetDeviceForPathBindMount(t *testing.T) { defer os.Remove(dest) - err = bindMount(context.Background(), source, dest, false) + err = bindMount(context.Background(), source, dest, false, "private") assert.NoError(err) defer syscall.Unmount(dest, syscall.MNT_DETACH) @@ -283,6 +283,107 @@ func TestIsEphemeralStorage(t *testing.T) { assert.False(isHostEmptyDir) } +func TestBindMountInvalidSourceSymlink(t *testing.T) { + source := filepath.Join(testDir, "fooFile") + os.Remove(source) + + err := bindMount(context.Background(), source, "", false, "private") + assert.Error(t, err) +} + +func TestBindMountFailingMount(t *testing.T) { + source := filepath.Join(testDir, "fooLink") + fakeSource := filepath.Join(testDir, "fooFile") + os.Remove(source) + os.Remove(fakeSource) + assert := assert.New(t) + + _, err := os.OpenFile(fakeSource, os.O_CREATE, mountPerm) + assert.NoError(err) + + err = os.Symlink(fakeSource, source) + assert.NoError(err) + + err = bindMount(context.Background(), source, "", false, "private") + assert.Error(err) +} + +func TestBindMountSuccessful(t *testing.T) { + assert := assert.New(t) + if tc.NotValid(ktu.NeedRoot()) { + t.Skip(testDisabledAsNonRoot) + } + + source := filepath.Join(testDir, "fooDirSrc") + dest := filepath.Join(testDir, "fooDirDest") + syscall.Unmount(dest, 0) + os.Remove(source) + os.Remove(dest) + + err := os.MkdirAll(source, mountPerm) + assert.NoError(err) + + err = os.MkdirAll(dest, mountPerm) + assert.NoError(err) + + err = bindMount(context.Background(), source, dest, false, "private") + assert.NoError(err) + + syscall.Unmount(dest, 0) +} + +func TestBindMountReadonlySuccessful(t *testing.T) { + assert := assert.New(t) + if tc.NotValid(ktu.NeedRoot()) { + t.Skip(testDisabledAsNonRoot) + } + + source := filepath.Join(testDir, "fooDirSrc") + dest := filepath.Join(testDir, "fooDirDest") + syscall.Unmount(dest, 0) + os.Remove(source) + os.Remove(dest) + + err := os.MkdirAll(source, mountPerm) + assert.NoError(err) + + err = os.MkdirAll(dest, mountPerm) + assert.NoError(err) + + err = bindMount(context.Background(), source, dest, true, "private") + assert.NoError(err) + + defer syscall.Unmount(dest, 0) + + // should not be able to create file in read-only mount + destFile := filepath.Join(dest, "foo") + _, err = os.OpenFile(destFile, os.O_CREATE, mountPerm) + assert.Error(err) +} + +func TestBindMountInvalidPgtypes(t *testing.T) { + assert := assert.New(t) + if tc.NotValid(ktu.NeedRoot()) { + t.Skip(testDisabledAsNonRoot) + } + + source := filepath.Join(testDir, "fooDirSrc") + dest := filepath.Join(testDir, "fooDirDest") + syscall.Unmount(dest, 0) + os.Remove(source) + os.Remove(dest) + + err := os.MkdirAll(source, mountPerm) + assert.NoError(err) + + err = os.MkdirAll(dest, mountPerm) + assert.NoError(err) + + err = bindMount(context.Background(), source, dest, false, "foo") + expectedErr := fmt.Sprintf("Wrong propagation type %s", "foo") + assert.EqualError(err, expectedErr) +} + // TestBindUnmountContainerRootfsENOENTNotError tests that if a file // or directory attempting to be unmounted doesn't exist, then it // is not considered an error diff --git a/virtcontainers/syscall_test.go b/virtcontainers/syscall_test.go index 568e40a2b4..ed321a4de3 100644 --- a/virtcontainers/syscall_test.go +++ b/virtcontainers/syscall_test.go @@ -7,94 +7,13 @@ package virtcontainers import ( - "context" "os" "path/filepath" - "syscall" "testing" - ktu "github.com/kata-containers/runtime/pkg/katatestutils" "github.com/stretchr/testify/assert" ) -func TestBindMountInvalidSourceSymlink(t *testing.T) { - source := filepath.Join(testDir, "fooFile") - os.Remove(source) - - err := bindMount(context.Background(), source, "", false) - assert.Error(t, err) -} - -func TestBindMountFailingMount(t *testing.T) { - source := filepath.Join(testDir, "fooLink") - fakeSource := filepath.Join(testDir, "fooFile") - os.Remove(source) - os.Remove(fakeSource) - assert := assert.New(t) - - _, err := os.OpenFile(fakeSource, os.O_CREATE, mountPerm) - assert.NoError(err) - - err = os.Symlink(fakeSource, source) - assert.NoError(err) - - err = bindMount(context.Background(), source, "", false) - assert.Error(err) -} - -func TestBindMountSuccessful(t *testing.T) { - assert := assert.New(t) - if tc.NotValid(ktu.NeedRoot()) { - t.Skip(testDisabledAsNonRoot) - } - - source := filepath.Join(testDir, "fooDirSrc") - dest := filepath.Join(testDir, "fooDirDest") - syscall.Unmount(dest, 0) - os.Remove(source) - os.Remove(dest) - - err := os.MkdirAll(source, mountPerm) - assert.NoError(err) - - err = os.MkdirAll(dest, mountPerm) - assert.NoError(err) - - err = bindMount(context.Background(), source, dest, false) - assert.NoError(err) - - syscall.Unmount(dest, 0) -} - -func TestBindMountReadonlySuccessful(t *testing.T) { - assert := assert.New(t) - if tc.NotValid(ktu.NeedRoot()) { - t.Skip(testDisabledAsNonRoot) - } - - source := filepath.Join(testDir, "fooDirSrc") - dest := filepath.Join(testDir, "fooDirDest") - syscall.Unmount(dest, 0) - os.Remove(source) - os.Remove(dest) - - err := os.MkdirAll(source, mountPerm) - assert.NoError(err) - - err = os.MkdirAll(dest, mountPerm) - assert.NoError(err) - - err = bindMount(context.Background(), source, dest, true) - assert.NoError(err) - - defer syscall.Unmount(dest, 0) - - // should not be able to create file in read-only mount - destFile := filepath.Join(dest, "foo") - _, err = os.OpenFile(destFile, os.O_CREATE, mountPerm) - assert.Error(err) -} - func TestEnsureDestinationExistsNonExistingSource(t *testing.T) { err := ensureDestinationExists("", "") assert.Error(t, err) @@ -107,8 +26,9 @@ func TestEnsureDestinationExistsWrongParentDir(t *testing.T) { os.Remove(dest) assert := assert.New(t) - _, err := os.OpenFile(source, os.O_CREATE, mountPerm) + file, err := os.OpenFile(source, os.O_CREATE, mountPerm) assert.NoError(err) + defer file.Close() err = ensureDestinationExists(source, dest) assert.Error(err) @@ -123,20 +43,22 @@ func TestEnsureDestinationExistsSuccessfulSrcDir(t *testing.T) { err := os.MkdirAll(source, mountPerm) assert.NoError(err) + defer os.Remove(source) err = ensureDestinationExists(source, dest) assert.NoError(err) } func TestEnsureDestinationExistsSuccessfulSrcFile(t *testing.T) { - source := filepath.Join(testDir, "fooDirSrc") - dest := filepath.Join(testDir, "fooDirDest") + source := filepath.Join(testDir, "fooFileSrc") + dest := filepath.Join(testDir, "fooFileDest") os.Remove(source) os.Remove(dest) assert := assert.New(t) - _, err := os.OpenFile(source, os.O_CREATE, mountPerm) + file, err := os.OpenFile(source, os.O_CREATE, mountPerm) assert.NoError(err) + defer file.Close() err = ensureDestinationExists(source, dest) assert.NoError(err)