From 13390df005cffb7f520a5bd1e818d0a4564feffc Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Thu, 12 Mar 2020 08:50:27 +0000 Subject: [PATCH] Jailer: re-mount jailerRoot with exec The default chrootBaseDir "/run/vc" in many distributions is mounted with `noexec` flag, which will bring 'permission denied' error when running kata-containers with jailer. Therefore, we decided to remount the jailerRoot dir with exec when setting up a new firecracker sandbox and umount it when cleaning up. Fixes: #2511 Signed-off-by: Penny Zheng --- virtcontainers/fc.go | 53 +++++++++++++++++++++++++++++++---------- virtcontainers/mount.go | 18 ++++++++++++++ 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go index 3ebb00c839..09fda7b160 100644 --- a/virtcontainers/fc.go +++ b/virtcontainers/fc.go @@ -331,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 @@ -497,6 +485,24 @@ 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", @@ -645,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), @@ -801,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 93951ac4fd..69db01d039 100644 --- a/virtcontainers/mount.go +++ b/virtcontainers/mount.go @@ -309,6 +309,24 @@ func bindMount(ctx context.Context, source, destination string, readonly bool, p 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 {