kata-containers/runtime/patches/0067-CVE-2020-28914-2.patch

From 3f0e61c010556846e2f96811059d86cb10309748 Mon Sep 17 00:00:00 2001
From: Peng Tao <bergwolf@hyper.sh>
Date: Fri, 30 Oct 2020 17:40:12 +0800
Subject: [PATCH] runtime: mount shared mountpoint readonly

bindmount remount events are not propagated through mount subtrees,
so we have to remount the shared dir mountpoint directly.

E.g.,
```
mkdir -p source dest foo source/foo

mount -o bind --make-shared source dest

mount -o bind foo source/foo
echo bind mount rw
mount | grep foo
echo remount ro
mount -o remount,bind,ro source/foo
mount | grep foo
```
would result in:
```
bind mount rw
/dev/xvda1 on /home/ubuntu/source/foo type ext4 (rw,relatime,discard,data=ordered)
/dev/xvda1 on /home/ubuntu/dest/foo type ext4 (rw,relatime,discard,data=ordered)
remount ro
/dev/xvda1 on /home/ubuntu/source/foo type ext4 (ro,relatime,discard,data=ordered)
/dev/xvda1 on /home/ubuntu/dest/foo type ext4 (rw,relatime,discard,data=ordered)
```

The reason is that bind mount creats new mount structs and attaches them to different mount subtrees.
However, MS_REMOUNT only looks for existing mount structs to modify and does not try to propagate the
change to mount structs in other subtrees.

Fixes: #3041
Signed-off-by: Peng Tao <bergwolf@hyper.sh>
Reference: https://github.com/kata-containers/runtime/commit/3f0e61c010556846e2f96811059d86cb10309748
	   https://github.com/kata-containers/community/blob/master/VMT/KCSA/KCSA-CVE-2020-28914.md
(cherry picked from commit 77399058bf3ded60aaeb08978073d000f178478f)
---
 virtcontainers/container.go    | 15 +++++++++++----
 virtcontainers/kata_agent.go   |  2 +-
 virtcontainers/mount.go        |  5 +++++
 virtcontainers/sandbox_test.go |  2 +-
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/virtcontainers/container.go b/virtcontainers/container.go
index 6973c8328..37117d7e3 100644
--- a/virtcontainers/container.go
+++ b/virtcontainers/container.go
@@ -446,7 +446,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
@@ -480,12 +480,19 @@ 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)
+		mountDest := filepath.Join(hostMountDir, filename)
 		if err := bindMount(c.ctx, m.Source, mountDest, m.ReadOnly, "private"); err != nil {
 			return "", false, err
 		}
 		// Save HostPath mount value into the mount list of the container.
 		c.mounts[idx].HostPath = mountDest
+		// bindmount remount event is not propagated to mount subtrees, so we have to remount the shared dir mountpoint directly.
+		if m.ReadOnly {
+			mountDest = filepath.Join(hostSharedDir, filename)
+			if err := remountRo(c.ctx, mountDest); err != nil {
+				return "", false, err
+			}
+		}
 	}
 
 	return guestDest, false, nil
@@ -496,7 +503,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
@@ -546,7 +553,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
 		}
diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go
index a308952a8..a26e0411b 100644
--- a/virtcontainers/kata_agent.go
+++ b/virtcontainers/kata_agent.go
@@ -1387,7 +1387,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
 	}
diff --git a/virtcontainers/mount.go b/virtcontainers/mount.go
index ab9e9f8d0..2f9846e80 100644
--- a/virtcontainers/mount.go
+++ b/virtcontainers/mount.go
@@ -273,6 +273,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 {
diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go
index 55e0ffea0..9f9d50ac1 100644
--- a/virtcontainers/sandbox_test.go
+++ b/virtcontainers/sandbox_test.go
@@ -1377,7 +1377,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")