From 32de642fa1f4d6189463a10d33a561130d5e6059 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 22 Mar 2018 15:04:39 -0400 Subject: [PATCH] Use O_PATH to avoid errors on Openat Also add tests for sockets and fifos Replace use of syscall for flags with unix --- pkg/util/mount/mount_linux.go | 20 ++++++-- pkg/util/mount/mount_linux_test.go | 77 ++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 0b54b3fd36..012b32673f 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -54,7 +54,9 @@ const ( // place for subpath mounts containerSubPathDirectoryName = "volume-subpaths" // syscall.Openat flags used to traverse directories not following symlinks - nofollowFlags = syscall.O_RDONLY | syscall.O_NOFOLLOW + nofollowFlags = unix.O_RDONLY | unix.O_NOFOLLOW + // flags for getting file descriptor without following the symlink + openFDFlags = unix.O_NOFOLLOW | unix.O_PATH ) // Mounter provides the default implementation of mount.Interface @@ -1074,7 +1076,9 @@ func findExistingPrefix(base, pathname string) (string, []string, error) { } }() for i, dir := range dirs { - childFD, err := syscall.Openat(fd, dir, syscall.O_RDONLY, 0) + // Using O_PATH here will prevent hangs in case user replaces directory with + // fifo + childFD, err := syscall.Openat(fd, dir, unix.O_PATH, 0) if err != nil { if os.IsNotExist(err) { return currentPath, dirs[i:], nil @@ -1136,11 +1140,21 @@ func doSafeOpen(pathname string, base string) (int, error) { } glog.V(5).Infof("Opening path %s", currentPath) - childFD, err = syscall.Openat(parentFD, seg, nofollowFlags, 0) + childFD, err = syscall.Openat(parentFD, seg, openFDFlags, 0) if err != nil { return -1, fmt.Errorf("cannot open %s: %s", currentPath, err) } + var deviceStat unix.Stat_t + err := unix.Fstat(childFD, &deviceStat) + if err != nil { + return -1, fmt.Errorf("Error running fstat on %s with %v", currentPath, err) + } + fileFmt := deviceStat.Mode & syscall.S_IFMT + if fileFmt == syscall.S_IFLNK { + return -1, fmt.Errorf("Unexpected symlink found %s", currentPath) + } + // Close parentFD if err = syscall.Close(parentFD); err != nil { return -1, fmt.Errorf("closing fd for %q failed: %v", filepath.Dir(currentPath), err) diff --git a/pkg/util/mount/mount_linux_test.go b/pkg/util/mount/mount_linux_test.go index cbbf0cacfb..5eb512bd70 100644 --- a/pkg/util/mount/mount_linux_test.go +++ b/pkg/util/mount/mount_linux_test.go @@ -21,6 +21,7 @@ package mount import ( "fmt" "io/ioutil" + "net" "os" "path/filepath" "reflect" @@ -1179,6 +1180,44 @@ func TestBindSubPath(t *testing.T) { }, expectError: false, }, + { + name: "subpath-mounting-unix-socket", + prepare: func(base string) ([]string, string, string, error) { + volpath, subpathMount := getTestPaths(base) + mounts := []string{subpathMount} + if err := os.MkdirAll(volpath, defaultPerm); err != nil { + return nil, "", "", err + } + + if err := os.MkdirAll(subpathMount, defaultPerm); err != nil { + return nil, "", "", err + } + + testSocketFile := filepath.Join(volpath, "mount_test.sock") + _, err := net.Listen("unix", testSocketFile) + return mounts, volpath, testSocketFile, err + }, + expectError: false, + }, + { + name: "subpath-mounting-fifo", + prepare: func(base string) ([]string, string, string, error) { + volpath, subpathMount := getTestPaths(base) + mounts := []string{subpathMount} + if err := os.MkdirAll(volpath, defaultPerm); err != nil { + return nil, "", "", err + } + + if err := os.MkdirAll(subpathMount, defaultPerm); err != nil { + return nil, "", "", err + } + + testFifo := filepath.Join(volpath, "mount_test.fifo") + err := syscall.Mkfifo(testFifo, 0) + return mounts, volpath, testFifo, err + }, + expectError: false, + }, } for _, test := range tests { @@ -1308,6 +1347,7 @@ func TestParseMountInfo(t *testing.T) { func TestSafeOpen(t *testing.T) { defaultPerm := os.FileMode(0750) + tests := []struct { name string // Function that prepares directory structure for the test under given @@ -1435,6 +1475,32 @@ func TestSafeOpen(t *testing.T) { "test", true, }, + { + "mounting-unix-socket", + func(base string) error { + testSocketFile := filepath.Join(base, "mount_test.sock") + _, err := net.Listen("unix", testSocketFile) + if err != nil { + return fmt.Errorf("Error preparing socket file %s with %v", testSocketFile, err) + } + return nil + }, + "mount_test.sock", + false, + }, + { + "mounting-unix-socket-in-middle", + func(base string) error { + testSocketFile := filepath.Join(base, "mount_test.sock") + _, err := net.Listen("unix", testSocketFile) + if err != nil { + return fmt.Errorf("Error preparing socket file %s with %v", testSocketFile, err) + } + return nil + }, + "mount_test.sock/bar", + true, + }, } for _, test := range tests { @@ -1551,6 +1617,17 @@ func TestFindExistingPrefix(t *testing.T) { []string{"test", "directory"}, false, }, + { + "with-fifo-in-middle", + func(base string) error { + testFifo := filepath.Join(base, "mount_test.fifo") + return syscall.Mkfifo(testFifo, 0) + }, + "mount_test.fifo/directory", + "", + nil, + true, + }, } for _, test := range tests {