From 10c596a4ff6cf77470cf4a661c03702abaa8eed9 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Mon, 16 Apr 2018 11:37:11 -0700 Subject: [PATCH 1/3] dev: Revert "Don't ignore container mounts based on their path" This reverts commit 08909b2213a23ab00a442a505f4b1a146e34b41b. We should not be passing any bind-mounts from /dev, /sys and /proc. Mounting these from the host inside the container does not make sense as these files are relevant to the host OS. Fixes #219 Signed-off-by: Archana Shinde --- virtcontainers/container.go | 2 +- virtcontainers/mount.go | 12 ++++++++++++ virtcontainers/mount_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 38dfab88a6..32e3abde9c 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -290,7 +290,7 @@ func (c *Container) createContainersDirs() error { func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ([]Mount, error) { var sharedDirMounts []Mount for idx, m := range c.mounts { - if m.Type != "bind" { + if isSystemMount(m.Destination) || m.Type != "bind" { continue } diff --git a/virtcontainers/mount.go b/virtcontainers/mount.go index da512779ad..e4a7dfb275 100644 --- a/virtcontainers/mount.go +++ b/virtcontainers/mount.go @@ -18,6 +18,18 @@ import ( var rootfsDir = "rootfs" +var systemMountPrefixes = []string{"/proc", "/dev", "/sys"} + +func isSystemMount(m string) bool { + for _, p := range systemMountPrefixes { + if m == p || strings.HasPrefix(m, p+"/") { + return true + } + } + + return false +} + func major(dev uint64) int { return int((dev >> 8) & 0xfff) } diff --git a/virtcontainers/mount_test.go b/virtcontainers/mount_test.go index d907b11252..4795932f53 100644 --- a/virtcontainers/mount_test.go +++ b/virtcontainers/mount_test.go @@ -18,6 +18,30 @@ import ( "testing" ) +func TestIsSystemMount(t *testing.T) { + tests := []struct { + mnt string + expected bool + }{ + {"/sys", true}, + {"/sys/", true}, + {"/sys//", true}, + {"/sys/fs", true}, + {"/sys/fs/", true}, + {"/sys/fs/cgroup", true}, + {"/sysfoo", false}, + {"/home", false}, + {"/dev/block/", true}, + } + + for _, test := range tests { + result := isSystemMount(test.mnt) + if result != test.expected { + t.Fatalf("Expected result for path %s : %v, got %v", test.mnt, test.expected, result) + } + } +} + func TestMajorMinorNumber(t *testing.T) { devices := []string{"/dev/zero", "/dev/net/tun"} From 70c3fe9dcd258e33d32dd33ee2af288d761ab1ca Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Tue, 17 Apr 2018 11:51:28 -0700 Subject: [PATCH 2/3] virtcontainers: Remove /dev from the ignored system mounts Since we want to handle certain files in /dev for k8s case, remove /dev from the mounts list that we ignore. Signed-off-by: Archana Shinde --- virtcontainers/mount.go | 2 +- virtcontainers/mount_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/virtcontainers/mount.go b/virtcontainers/mount.go index e4a7dfb275..ba712ac4b0 100644 --- a/virtcontainers/mount.go +++ b/virtcontainers/mount.go @@ -18,7 +18,7 @@ import ( var rootfsDir = "rootfs" -var systemMountPrefixes = []string{"/proc", "/dev", "/sys"} +var systemMountPrefixes = []string{"/proc", "/sys"} func isSystemMount(m string) bool { for _, p := range systemMountPrefixes { diff --git a/virtcontainers/mount_test.go b/virtcontainers/mount_test.go index 4795932f53..6818af6d92 100644 --- a/virtcontainers/mount_test.go +++ b/virtcontainers/mount_test.go @@ -31,7 +31,8 @@ func TestIsSystemMount(t *testing.T) { {"/sys/fs/cgroup", true}, {"/sysfoo", false}, {"/home", false}, - {"/dev/block/", true}, + {"/dev/block/", false}, + {"/mnt/dev/foo", false}, } for _, test := range tests { From 71c7a9c13ecbd981ffad99b1b3edb79c9405cd76 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Tue, 17 Apr 2018 11:54:16 -0700 Subject: [PATCH 3/3] virtcontainers: Handle regular files in /dev The k8s test creates a log file in /dev under /dev/termination-log, which is not the right place to create logs, but we need to handle this. With this commit, we handle regular files under /dev by passing them as 9p shares. All other special files including device files and directories are not passed as 9p shares as these are specific to the host. Any operations on these in the guest would fail anyways. Signed-off-by: Archana Shinde --- virtcontainers/container.go | 7 +++++++ virtcontainers/mount.go | 28 ++++++++++++++++++++++++++++ virtcontainers/mount_test.go | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 32e3abde9c..b9bf4b3294 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -327,6 +327,13 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ( continue } + // Ignore /dev, directories and all other device files. We handle + // only regular files in /dev. It does not make sense to pass the host + // device nodes to the guest. + if isHostDevice(m.Destination) { + continue + } + randBytes, err := generateRandomBytes(8) if err != nil { return nil, err diff --git a/virtcontainers/mount.go b/virtcontainers/mount.go index ba712ac4b0..13e6c3098b 100644 --- a/virtcontainers/mount.go +++ b/virtcontainers/mount.go @@ -30,6 +30,34 @@ func isSystemMount(m string) bool { return false } +func isHostDevice(m string) bool { + if m == "/dev" { + return true + } + + if strings.HasPrefix(m, "/dev/") { + // Check if regular file + s, err := os.Stat(m) + + // This should not happen. In case file does not exist let the + // error be handled by the agent, simply return false here. + if err != nil { + return false + } + + if s.Mode().IsRegular() { + return false + } + + // This is not a regular file in /dev. It is either a + // device file, directory or any other special file which is + // specific to the host system. + return true + } + + return false +} + func major(dev uint64) int { return int((dev >> 8) & 0xfff) } diff --git a/virtcontainers/mount_test.go b/virtcontainers/mount_test.go index 6818af6d92..25f00dbc28 100644 --- a/virtcontainers/mount_test.go +++ b/virtcontainers/mount_test.go @@ -43,6 +43,41 @@ func TestIsSystemMount(t *testing.T) { } } +func TestIsHostDevice(t *testing.T) { + tests := []struct { + mnt string + expected bool + }{ + {"/dev", true}, + {"/dev/zero", true}, + {"/dev/block", true}, + {"/mnt/dev/block", false}, + } + + for _, test := range tests { + result := isHostDevice(test.mnt) + if result != test.expected { + t.Fatalf("Expected result for path %s : %v, got %v", test.mnt, test.expected, result) + } + } + + // Create regular file in /dev + path := "/dev/foobar" + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + f.Close() + + if isHostDevice(path) != false { + t.Fatalf("Expected result for path %s : %v, got %v", path, false, true) + } + + if err := os.Remove(path); err != nil { + t.Fatal(err) + } +} + func TestMajorMinorNumber(t *testing.T) { devices := []string{"/dev/zero", "/dev/net/tun"}