Merge pull request #1485 from egernst/backport-bindmount-fixes

backport: bindmount fixes

src/runtime/cli/utils_arch_base.go

@@ -1,10 +0,0 @@
-// +build !s390x
-//
-// SPDX-License-Identifier: Apache-2.0
-//
-
-package main
-
-func archConvertStatFs(cgroupFsType int) int64 {
-	return int64(cgroupFsType)
-}

src/runtime/cli/utils_s390x.go

@@ -1,10 +0,0 @@
-// Copyright (c) 2018 IBM
-//
-// SPDX-License-Identifier: Apache-2.0
-//
-
-package main
-
-func archConvertStatFs(cgroupFsType int) uint32 {
-	return uint32(cgroupFsType)
-}

src/runtime/virtcontainers/container.go

@@ -435,7 +435,7 @@ func (c *Container) setContainerState(state types.StateString) error {
 	return nil
 }
 
-func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir string) (string, bool, error) {
+func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, hostMountDir, guestSharedDir string) (string, bool, error) {
 	randBytes, err := utils.GenerateRandomBytes(8)
 	if err != nil {
 		return "", false, err
@@ -469,9 +469,34 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s
 		}
 	} else {
 		// These mounts are created in the shared dir
-		mountDest := filepath.Join(hostSharedDir, filename)
-		if err := bindMount(c.ctx, m.Source, mountDest, false, "private"); err != nil {
-			return "", false, err
+		mountDest := filepath.Join(hostMountDir, filename)
+		if !m.ReadOnly {
+			if err := bindMount(c.ctx, m.Source, mountDest, false, "private"); err != nil {
+				return "", false, err
+			}
+		} else {
+			// For RO mounts, bindmount remount event is not propagated to mount subtrees,
+			// and it doesn't present in the virtiofsd standalone mount namespace either.
+			// So we end up a bit tricky:
+			// 1. make a private bind mount to the mount source
+			// 2. make another ro bind mount on the private mount
+			// 3. move the ro bind mount to mountDest
+			// 4. umount the private bind mount created in step 1
+			privateDest := filepath.Join(getPrivatePath(c.sandboxID), filename)
+			if err := bindMount(c.ctx, m.Source, privateDest, false, "private"); err != nil {
+				return "", false, err
+			}
+			defer func() {
+				syscall.Unmount(privateDest, syscall.MNT_DETACH|UmountNoFollow)
+			}()
+			if err := bindMount(c.ctx, privateDest, privateDest, true, "private"); err != nil {
+				return "", false, err
+			}
+			if err := moveMount(c.ctx, privateDest, mountDest); err != nil {
+				return "", false, err
+			}
+
+			syscall.Unmount(privateDest, syscall.MNT_DETACH|UmountNoFollow)
 		}
 		// Save HostPath mount value into the mount list of the container.
 		c.mounts[idx].HostPath = mountDest
@@ -485,7 +510,7 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s
 // It also updates the container mount list with the HostPath info, and store
 // container mounts to the storage. This way, we will have the HostPath info
 // available when we will need to unmount those mounts.
-func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (sharedDirMounts map[string]Mount, ignoredMounts map[string]Mount, err error) {
+func (c *Container) mountSharedDirMounts(hostSharedDir, hostMountDir, guestSharedDir string) (sharedDirMounts map[string]Mount, ignoredMounts map[string]Mount, err error) {
 	sharedDirMounts = make(map[string]Mount)
 	ignoredMounts = make(map[string]Mount)
 	var devicesToDetach []string
@@ -536,7 +561,7 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 
 		var ignore bool
 		var guestDest string
-		guestDest, ignore, err = c.shareFiles(m, idx, hostSharedDir, guestSharedDir)
+		guestDest, ignore, err = c.shareFiles(m, idx, hostSharedDir, hostMountDir, guestSharedDir)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -547,22 +572,12 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 			continue
 		}
 
-		// Check if mount is readonly, let the agent handle the readonly mount
-		// within the VM.
-		readonly := false
-		for _, flag := range m.Options {
-			if flag == "ro" {
-				readonly = true
-				break
-			}
-		}
-
 		sharedDirMount := Mount{
 			Source:      guestDest,
 			Destination: m.Destination,
 			Type:        m.Type,
 			Options:     m.Options,
-			ReadOnly:    readonly,
+			ReadOnly:    m.ReadOnly,
 		}
 
 		sharedDirMounts[sharedDirMount.Destination] = sharedDirMount

src/runtime/virtcontainers/kata_agent.go

@@ -149,9 +149,10 @@ var kataHostSharedDir = func() string {
 }
 
 // Shared path handling:
-// 1. create two directories for each sandbox:
+// 1. create three directories for each sandbox:
 // -. /run/kata-containers/shared/sandboxes/$sbx_id/mounts/, a directory to hold all host/guest shared mounts
 // -. /run/kata-containers/shared/sandboxes/$sbx_id/shared/, a host/guest shared directory (9pfs/virtiofs source dir)
+// -. /run/kata-containers/shared/sandboxes/$sbx_id/private/, a directory to hold all temporary private mounts when creating ro mounts
 //
 // 2. /run/kata-containers/shared/sandboxes/$sbx_id/mounts/ is bind mounted readonly to /run/kata-containers/shared/sandboxes/$sbx_id/shared/, so guest cannot modify it
 //
@@ -164,6 +165,10 @@ func getMountPath(id string) string {
 	return filepath.Join(kataHostSharedDir(), id, "mounts")
 }
 
+func getPrivatePath(id string) string {
+	return filepath.Join(kataHostSharedDir(), id, "private")
+}
+
 func getSandboxPath(id string) string {
 	return filepath.Join(kataHostSharedDir(), id)
 }
@@ -1261,7 +1266,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
 	}
 
 	// Handle container mounts
-	newMounts, ignoredMounts, err := c.mountSharedDirMounts(getMountPath(sandbox.id), kataGuestSharedDir())
+	newMounts, ignoredMounts, err := c.mountSharedDirMounts(getSharePath(sandbox.id), getMountPath(sandbox.id), kataGuestSharedDir())
 	if err != nil {
 		return nil, err
 	}

src/runtime/virtcontainers/mount.go

@@ -212,6 +212,42 @@ func isDeviceMapper(major, minor int) (bool, error) {
 
 const mountPerm = os.FileMode(0755)
 
+func evalMountPath(source, destination string) (string, string, error) {
+	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)
+	}
+
+	return absSource, destination, nil
+}
+
+// moveMount moves a mountpoint to another path with some bookkeeping:
+// * evaluate all symlinks
+// * ensure the source exists
+// * recursively create the destination
+func moveMount(ctx context.Context, source, destination string) error {
+	span, _ := trace(ctx, "moveMount")
+	defer span.Finish()
+
+	source, destination, err := evalMountPath(source, destination)
+	if err != nil {
+		return err
+	}
+
+	return syscall.Mount(source, destination, "move", syscall.MS_MOVE, "")
+}
+
 // bindMount bind mounts a source in to a destination. This will
 // do some bookkeeping:
 // * evaluate all symlinks
@@ -222,20 +258,9 @@ func bindMount(ctx context.Context, source, destination string, readonly bool, p
 	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)
+	absSource, destination, err := evalMountPath(source, destination)
 	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)
+		return err
 	}
 
 	if err := syscall.Mount(absSource, destination, "bind", syscall.MS_BIND, ""); err != nil {
@@ -277,6 +302,11 @@ func remount(ctx context.Context, mountflags uintptr, src string) error {
 	return nil
 }
 
+// remount a mount point as readonly
+func remountRo(ctx context.Context, src string) error {
+	return remount(ctx, syscall.MS_BIND|syscall.MS_RDONLY, src)
+}
+
 // bindMountContainerRootfs bind mounts a container rootfs into a 9pfs shared
 // directory between the guest and the host.
 func bindMountContainerRootfs(ctx context.Context, shareDir, cid, cRootFs string, readonly bool) error {

src/runtime/virtcontainers/pkg/oci/utils.go

@@ -159,11 +159,19 @@ func cmdEnvs(spec specs.Spec, envs []types.EnvVar) []types.EnvVar {
 }
 
 func newMount(m specs.Mount) vc.Mount {
+	readonly := false
+	for _, flag := range m.Options {
+		if flag == "ro" {
+			readonly = true
+			break
+		}
+	}
 	return vc.Mount{
 		Source:      m.Source,
 		Destination: m.Destination,
 		Type:        m.Type,
 		Options:     m.Options,
+		ReadOnly:    readonly,
 	}
 }
 

src/runtime/virtcontainers/sandbox_test.go

@@ -1175,7 +1175,7 @@ func TestPreAddDevice(t *testing.T) {
 		},
 	}
 
-	mounts, ignoreMounts, err := container.mountSharedDirMounts("", "")
+	mounts, ignoreMounts, err := container.mountSharedDirMounts("", "", "")
 	assert.Nil(t, err)
 	assert.Equal(t, len(mounts), 0,
 		"mounts should contain nothing because it only contains a block device")

tools/packaging/static-build/firecracker/Makefile

@@ -1,3 +1,8 @@
+#!/usr/bin/env bash
+# Copyright (c) 2021 Intel Corporation
+#
+# SPDX-License-Identifier: Apache-2.0
+#
 MK_DIR :=$(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))
 CONFIG_DIR := $(MK_DIR)/../../scripts/
 

tools/packaging/static-build/qemu/Makefile

@@ -1,3 +1,8 @@
+#!/usr/bin/env bash
+# Copyright (c) 2021 Intel Corporation
+#
+# SPDX-License-Identifier: Apache-2.0
+#
 MK_DIR :=$(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))
 CONFIG_DIR := $(MK_DIR)/../../scripts/