From 249259376da98d8e10e6876c80ff3905e9756bdc Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: rename Mount.Filesystem to Mount.FilesystemType Make it clear that this refers to a type of filesystem such as "ext4", rather than to a specific filesystem instance. --- filesystem/mountpoint.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index abd8232..856447f 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -87,9 +87,9 @@ func getMountInfo() error { // Create the Mount structure by converting types. mnt := Mount{ - Path: C.GoString(entry.mnt_dir), - Filesystem: C.GoString(entry.mnt_type), - Options: strings.Split(C.GoString(entry.mnt_opts), ","), + Path: C.GoString(entry.mnt_dir), + FilesystemType: C.GoString(entry.mnt_type), + Options: strings.Split(C.GoString(entry.mnt_opts), ","), } // Skip invalid mountpoints -- cgit v1.2.3 From 9d7a4bf93c0d20a7eb2f07ef2ef63834f2cbf6a9 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: remove Mount.Options fscrypt doesn't currently do anything with the mount options, so remove them from the Mount structure for now. --- filesystem/mountpoint.go | 1 - 1 file changed, 1 deletion(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index 856447f..2905481 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -89,7 +89,6 @@ func getMountInfo() error { mnt := Mount{ Path: C.GoString(entry.mnt_dir), FilesystemType: C.GoString(entry.mnt_type), - Options: strings.Split(C.GoString(entry.mnt_opts), ","), } // Skip invalid mountpoints -- cgit v1.2.3 From d0c3c98acd02d412d271ebb9608d8a9cd0e23a32 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: rename getMountInfo() to loadMountInfo() Make it clearer that this function loads data into global data structures, and doesn't return anything. --- filesystem/mountpoint.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index 2905481..3dc1934 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -57,10 +57,10 @@ var ( uuidDirectory = "/dev/disk/by-uuid" ) -// getMountInfo populates the Mount mappings by parsing the filesystem +// loadMountInfo populates the Mount mappings by parsing the filesystem // description file using the getmntent functions. Returns ErrBadLoad if the // Mount mappings cannot be populated. -func getMountInfo() error { +func loadMountInfo() error { if mountsInitialized { return nil } @@ -122,7 +122,7 @@ func getMountInfo() error { func AllFilesystems() ([]*Mount, error) { mountMutex.Lock() defer mountMutex.Unlock() - if err := getMountInfo(); err != nil { + if err := loadMountInfo(); err != nil { return nil, err } @@ -141,7 +141,7 @@ func UpdateMountInfo() error { mountMutex.Lock() defer mountMutex.Unlock() mountsInitialized = false - return getMountInfo() + return loadMountInfo() } // FindMount returns the corresponding Mount object for some path in a @@ -158,7 +158,7 @@ func FindMount(path string) (*Mount, error) { mountMutex.Lock() defer mountMutex.Unlock() - if err = getMountInfo(); err != nil { + if err = loadMountInfo(); err != nil { return nil, err } @@ -189,7 +189,7 @@ func GetMount(mountpoint string) (*Mount, error) { mountMutex.Lock() defer mountMutex.Unlock() - if err = getMountInfo(); err != nil { + if err = loadMountInfo(); err != nil { return nil, err } @@ -232,7 +232,7 @@ func getMountsFromLink(link string) ([]*Mount, error) { // Lookup mountpoints for device in global store mountMutex.Lock() defer mountMutex.Unlock() - if err := getMountInfo(); err != nil { + if err := loadMountInfo(); err != nil { return nil, err } mnts, ok := mountsByDevice[devicePath] -- cgit v1.2.3 From 4eb1ea869703873f6f4192dcec5262f33a497fcb Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: switch to using /proc/self/mountinfo Change loadMountInfo() to load the mounts directly from /proc/self/mountinfo, rather than use the mntent.h C library calls. This is needed for correct handling of bind mounts and of "/dev/root", since /proc/self/mountinfo has extra fields which show the mounted subtree and the filesystem's device number. /proc/mounts lacks these fields, and the C library calls can't provide them. To start, this patch just switches to using /proc/self/mountinfo, without doing anything with the extra fields yet. As a bonus, this eliminates all C code in mountpoint.go. --- filesystem/mountpoint.go | 124 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 85 insertions(+), 39 deletions(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index 3dc1934..c43e14e 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -22,27 +22,20 @@ package filesystem import ( + "bufio" "fmt" "io/ioutil" "log" "os" "path/filepath" "sort" + "strconv" "strings" "sync" "github.com/pkg/errors" ) -/* -#include // setmntent, getmntent, endmntent - -// The file containing mountpoints info and how we should read it -const char* mountpoints_filename = "/proc/mounts"; -const char* read_mode = "r"; -*/ -import "C" - var ( // These maps hold data about the state of the system's mountpoints. mountsByPath map[string]*Mount @@ -57,38 +50,88 @@ var ( uuidDirectory = "/dev/disk/by-uuid" ) -// loadMountInfo populates the Mount mappings by parsing the filesystem -// description file using the getmntent functions. Returns ErrBadLoad if the -// Mount mappings cannot be populated. +// Unescape octal-encoded escape sequences in a string from the mountinfo file. +// The kernel encodes the ' ', '\t', '\n', and '\\' bytes this way. This +// function exactly inverts what the kernel does, including by preserving +// invalid UTF-8. +func unescapeString(str string) string { + var sb strings.Builder + for i := 0; i < len(str); i++ { + b := str[i] + if b == '\\' && i+3 < len(str) { + if parsed, err := strconv.ParseInt(str[i+1:i+4], 8, 8); err == nil { + b = uint8(parsed) + i += 3 + } + } + sb.WriteByte(b) + } + return sb.String() +} + +// Parse one line of /proc/self/mountinfo. +// +// The line contains the following space-separated fields: +// [0] mount ID +// [1] parent ID +// [2] major:minor +// [3] root +// [4] mount point +// [5] mount options +// [6...n-1] optional field(s) +// [n] separator +// [n+1] filesystem type +// [n+2] mount source +// [n+3] super options +// +// For more details, see https://www.kernel.org/doc/Documentation/filesystems/proc.txt +func parseMountInfoLine(line string) *Mount { + fields := strings.Split(line, " ") + if len(fields) < 10 { + return nil + } + + // Count the optional fields. In case new fields are appended later, + // don't simply assume that n == len(fields) - 4. + n := 6 + for fields[n] != "-" { + n++ + if n >= len(fields) { + return nil + } + } + if n+3 >= len(fields) { + return nil + } + + var mnt *Mount = &Mount{} + mnt.Path = unescapeString(fields[4]) + mnt.FilesystemType = unescapeString(fields[n+1]) + mnt.Device = unescapeString(fields[n+2]) + return mnt +} + +// loadMountInfo populates the Mount mappings by parsing /proc/self/mountinfo. +// It returns an error if the Mount mappings cannot be populated. func loadMountInfo() error { if mountsInitialized { return nil } - - // make new maps mountsByPath = make(map[string]*Mount) mountsByDevice = make(map[string][]*Mount) - // Load the mount information from mountpoints_filename - fileHandle := C.setmntent(C.mountpoints_filename, C.read_mode) - if fileHandle == nil { - return errors.Wrapf(ErrGlobalMountInfo, "could not read %q", - C.GoString(C.mountpoints_filename)) + file, err := os.Open("/proc/self/mountinfo") + if err != nil { + return err } - defer C.endmntent(fileHandle) - - for { - entry := C.getmntent(fileHandle) - // When getmntent returns nil, we have read all of the entries. - if entry == nil { - mountsInitialized = true - return nil - } - - // Create the Mount structure by converting types. - mnt := Mount{ - Path: C.GoString(entry.mnt_dir), - FilesystemType: C.GoString(entry.mnt_type), + defer file.Close() + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := scanner.Text() + mnt := parseMountInfoLine(line) + if mnt == nil { + log.Printf("ignoring invalid mountinfo line %q", line) + continue } // Skip invalid mountpoints @@ -99,22 +142,25 @@ func loadMountInfo() error { } // We can only use mountpoints that are directories for fscrypt. if !isDir(mnt.Path) { - log.Printf("mnt_dir %v: not a directory", mnt.Path) + log.Printf("ignoring mountpoint %q because it is not a directory", mnt.Path) continue } // Note this overrides the info if we have seen the mountpoint // earlier in the file. This is correct behavior because the // filesystems are listed in mount order. - mountsByPath[mnt.Path] = &mnt + mountsByPath[mnt.Path] = mnt - deviceName, err := canonicalizePath(C.GoString(entry.mnt_fsname)) + mnt.Device, err = canonicalizePath(mnt.Device) // Only use real valid devices (unlike cgroups, tmpfs, ...) - if err == nil && isDevice(deviceName) { - mnt.Device = deviceName - mountsByDevice[deviceName] = append(mountsByDevice[deviceName], &mnt) + if err == nil && isDevice(mnt.Device) { + mountsByDevice[mnt.Device] = append(mountsByDevice[mnt.Device], mnt) + } else { + mnt.Device = "" } } + mountsInitialized = true + return nil } // AllFilesystems lists all the Mounts on the current system ordered by path. -- cgit v1.2.3 From c7b963bfa76b9541e01493d21fef3e3596ef9904 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: skip unnecessary mountpoint canonicalization The kernel always shows mountpoints as absolute paths without symlinks, so there's no need to canonicalize them in userspace. --- filesystem/mountpoint.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index c43e14e..861f5b1 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -134,12 +134,6 @@ func loadMountInfo() error { continue } - // Skip invalid mountpoints - var err error - if mnt.Path, err = canonicalizePath(mnt.Path); err != nil { - log.Printf("getting mnt_dir: %v", err) - continue - } // We can only use mountpoints that are directories for fscrypt. if !isDir(mnt.Path) { log.Printf("ignoring mountpoint %q because it is not a directory", mnt.Path) @@ -151,6 +145,7 @@ func loadMountInfo() error { // filesystems are listed in mount order. mountsByPath[mnt.Path] = mnt + var err error mnt.Device, err = canonicalizePath(mnt.Device) // Only use real valid devices (unlike cgroups, tmpfs, ...) if err == nil && isDevice(mnt.Device) { -- cgit v1.2.3 From c7da2443d6ffa51727db09f8ef1df6aea8c7612c Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: get correct device for kernel-mounted rootfs A root filesystem mounted via the kernel command line always has a source of "/dev/root", which isn't a real device node. This makes fscrypt think this filesystem doesn't have a source device, which breaks creating login passphrase-protected directories on other filesystems: fscrypt encrypt: filesystem /: no device for mount "/": system error: cannot create filesystem link This also makes 'fscrypt status' show a blank source device: MOUNTPOINT DEVICE FILESYSTEM ENCRYPTION FSCRYPT / ext4 supported Yes To fix this case, update loadMountInfo() to map the device number to the device name via sysfs rather than use the mount source field. --- filesystem/mountpoint.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index 861f5b1..da6a69a 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -69,6 +69,18 @@ func unescapeString(str string) string { return sb.String() } +// We get the device name via the device number rather than use the mount source +// field directly. This is necessary to handle a rootfs that was mounted via +// the kernel command line, since mountinfo always shows /dev/root for that. +// This assumes that the device nodes are in the standard location. +func getDeviceName(num DeviceNumber) string { + linkPath := fmt.Sprintf("/sys/dev/block/%v", num) + if target, err := os.Readlink(linkPath); err == nil { + return fmt.Sprintf("/dev/%s", filepath.Base(target)) + } + return "" +} + // Parse one line of /proc/self/mountinfo. // // The line contains the following space-separated fields: @@ -105,9 +117,14 @@ func parseMountInfoLine(line string) *Mount { } var mnt *Mount = &Mount{} + var err error + mnt.DeviceNumber, err = newDeviceNumberFromString(fields[2]) + if err != nil { + return nil + } mnt.Path = unescapeString(fields[4]) mnt.FilesystemType = unescapeString(fields[n+1]) - mnt.Device = unescapeString(fields[n+2]) + mnt.Device = getDeviceName(mnt.DeviceNumber) return mnt } @@ -145,13 +162,8 @@ func loadMountInfo() error { // filesystems are listed in mount order. mountsByPath[mnt.Path] = mnt - var err error - mnt.Device, err = canonicalizePath(mnt.Device) - // Only use real valid devices (unlike cgroups, tmpfs, ...) - if err == nil && isDevice(mnt.Device) { + if mnt.Device != "" { mountsByDevice[mnt.Device] = append(mountsByDevice[mnt.Device], mnt) - } else { - mnt.Device = "" } } mountsInitialized = true -- cgit v1.2.3 From fe58e7236a3285e172733aeab0b136bec968ac4d Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: make link handling more robust The previous patch fixed making linked protectors to /dev/root, by setting Mount.Device to the real device node rather than /dev/root. That's good, but it also hints that the linked protector handling is unnecessarily fragile, as it relies on the device node name matching exactly. The Linux kernel allows the same device to have multiple device nodes, and path comparisons are slow and error-prone in general. Change it to compare the device number instead. --- filesystem/mountpoint.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index da6a69a..d40f864 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -39,7 +39,7 @@ import ( var ( // These maps hold data about the state of the system's mountpoints. mountsByPath map[string]*Mount - mountsByDevice map[string][]*Mount + mountsByDevice map[DeviceNumber][]*Mount // Used to make the mount functions thread safe mountMutex sync.Mutex // True if the maps have been successfully initialized. @@ -135,7 +135,7 @@ func loadMountInfo() error { return nil } mountsByPath = make(map[string]*Mount) - mountsByDevice = make(map[string][]*Mount) + mountsByDevice = make(map[DeviceNumber][]*Mount) file, err := os.Open("/proc/self/mountinfo") if err != nil { @@ -162,9 +162,7 @@ func loadMountInfo() error { // filesystems are listed in mount order. mountsByPath[mnt.Path] = mnt - if mnt.Device != "" { - mountsByDevice[mnt.Device] = append(mountsByDevice[mnt.Device], mnt) - } + mountsByDevice[mnt.DeviceNumber] = append(mountsByDevice[mnt.DeviceNumber], mnt) } mountsInitialized = true return nil @@ -277,7 +275,7 @@ func getMountsFromLink(link string) ([]*Mount, error) { if filepath.Base(searchPath) != value { return nil, errors.Wrapf(ErrFollowLink, "value %q is not a UUID", value) } - devicePath, err := canonicalizePath(searchPath) + deviceNumber, err := getDeviceNumber(searchPath) if err != nil { return nil, errors.Wrapf(ErrFollowLink, "no device with UUID %q", value) } @@ -288,9 +286,11 @@ func getMountsFromLink(link string) ([]*Mount, error) { if err := loadMountInfo(); err != nil { return nil, err } - mnts, ok := mountsByDevice[devicePath] + mnts, ok := mountsByDevice[deviceNumber] if !ok { - return nil, errors.Wrapf(ErrFollowLink, "no mounts for device %q", devicePath) + devicePath, _ := canonicalizePath(searchPath) + return nil, errors.Wrapf(ErrFollowLink, "no mounts for device %q (%v)", + devicePath, deviceNumber) } return mnts, nil } @@ -302,9 +302,6 @@ func makeLink(mnt *Mount, token string) (string, error) { if token != uuidToken { return "", errors.Wrapf(ErrMakeLink, "token type %q not supported", token) } - if mnt.Device == "" { - return "", errors.Wrapf(ErrMakeLink, "no device for mount %q", mnt.Path) - } dirContents, err := ioutil.ReadDir(uuidDirectory) if err != nil { @@ -315,14 +312,15 @@ func makeLink(mnt *Mount, token string) (string, error) { continue // Only interested in UUID symlinks } uuid := fileInfo.Name() - devicePath, err := canonicalizePath(filepath.Join(uuidDirectory, uuid)) + deviceNumber, err := getDeviceNumber(filepath.Join(uuidDirectory, uuid)) if err != nil { log.Print(err) continue } - if mnt.Device == devicePath { + if mnt.DeviceNumber == deviceNumber { return fmt.Sprintf("%s=%s", uuidToken, uuid), nil } } - return "", errors.Wrapf(ErrMakeLink, "device %q has no UUID", mnt.Device) + return "", errors.Wrapf(ErrMakeLink, "device %q (%v) has no UUID", + mnt.Device, mnt.DeviceNumber) } -- cgit v1.2.3 From dbafdbaa9b0767f71affaf15fb8c626f64e27122 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:33:54 -0700 Subject: filesystem: handle bind mounts properly Currently, fscrypt treats bind mounts as separate filesystems. This is broken because fscrypt will look for a directory's encryption policy in different places depending on which mount it's accessed through. This forces users to create an fscrypt metadata directory at every bind mount, and to copy fscrypt metadata around between mounts. Fix this by storing fscrypt metadata only at the root of the filesystem. To accomplish this: - Make mountsByDevice store only a single Mount per filesystem, rather than multiple. For this Mount, choose a mount of the full filesystem if available, preferably a read-write mount. If the filesystem has only bind mounts, store a nil entry in mountsByDevice so we can show a proper error message later. - Change FindMount() and GetMount() to look up the Mount by device number rather than by path, so that they don't return different Mounts depending on which path is used. - Change AllFilesystems() to not return bind mounts. - Due to the above changes, the mountsByPath map is no longer needed outside of loadMountInfo(). So make it a local variable there. Resolves https://github.com/google/fscrypt/issues/59 --- filesystem/mountpoint.go | 147 +++++++++++++++++++++++++++-------------------- 1 file changed, 86 insertions(+), 61 deletions(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index d40f864..50389c0 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -37,9 +37,8 @@ import ( ) var ( - // These maps hold data about the state of the system's mountpoints. - mountsByPath map[string]*Mount - mountsByDevice map[DeviceNumber][]*Mount + // This map holds data about the state of the system's filesystems. + mountsByDevice map[DeviceNumber]*Mount // Used to make the mount functions thread safe mountMutex sync.Mutex // True if the maps have been successfully initialized. @@ -122,7 +121,13 @@ func parseMountInfoLine(line string) *Mount { if err != nil { return nil } + mnt.BindMnt = unescapeString(fields[3]) != "/" mnt.Path = unescapeString(fields[4]) + for _, opt := range strings.Split(fields[5], ",") { + if opt == "ro" { + mnt.ReadOnly = true + } + } mnt.FilesystemType = unescapeString(fields[n+1]) mnt.Device = getDeviceName(mnt.DeviceNumber) return mnt @@ -134,8 +139,8 @@ func loadMountInfo() error { if mountsInitialized { return nil } - mountsByPath = make(map[string]*Mount) - mountsByDevice = make(map[DeviceNumber][]*Mount) + mountsByPath := make(map[string]*Mount) + mountsByDevice = make(map[DeviceNumber]*Mount) file, err := os.Open("/proc/self/mountinfo") if err != nil { @@ -159,17 +164,38 @@ func loadMountInfo() error { // Note this overrides the info if we have seen the mountpoint // earlier in the file. This is correct behavior because the - // filesystems are listed in mount order. + // mountpoints are listed in mount order. mountsByPath[mnt.Path] = mnt - - mountsByDevice[mnt.DeviceNumber] = append(mountsByDevice[mnt.DeviceNumber], mnt) + } + // fscrypt only really cares about the root directory of each + // filesystem, because that's where the fscrypt metadata is stored. So + // keep just one Mount per filesystem, ignoring bind mounts. Store that + // Mount in mountsByDevice so that it can be found later from the device + // number. Also, prefer a read-write mount to a read-only one. + // + // If the filesystem has *only* bind mounts, store an explicit nil entry + // so that we can show a useful error message later. + for _, mnt := range mountsByPath { + existingMnt, ok := mountsByDevice[mnt.DeviceNumber] + if mnt.BindMnt { + if !ok { + mountsByDevice[mnt.DeviceNumber] = nil + } + } else if existingMnt == nil || (existingMnt.ReadOnly && !mnt.ReadOnly) { + mountsByDevice[mnt.DeviceNumber] = mnt + } } mountsInitialized = true return nil } -// AllFilesystems lists all the Mounts on the current system ordered by path. -// Use CheckSetup() to see if they are used with fscrypt. +func filesystemRootDirNotVisibleError(deviceNumber DeviceNumber) error { + return errors.Errorf("root of filesystem on device %q (%v) is not visible in the current mount namespace", + getDeviceName(deviceNumber), deviceNumber) +} + +// AllFilesystems lists all non-bind Mounts on the current system ordered by +// path. Use CheckSetup() to see if they are used with fscrypt. func AllFilesystems() ([]*Mount, error) { mountMutex.Lock() defer mountMutex.Unlock() @@ -177,9 +203,11 @@ func AllFilesystems() ([]*Mount, error) { return nil, err } - mounts := make([]*Mount, 0, len(mountsByPath)) - for _, mount := range mountsByPath { - mounts = append(mounts, mount) + mounts := make([]*Mount, 0, len(mountsByDevice)) + for _, mount := range mountsByDevice { + if mount != nil { + mounts = append(mounts, mount) + } } sort.Sort(PathSorter(mounts)) @@ -195,70 +223,64 @@ func UpdateMountInfo() error { return loadMountInfo() } -// FindMount returns the corresponding Mount object for some path in a -// filesystem. Note that in the case of a bind mounts there may be two Mount -// objects for the same underlying filesystem. An error is returned if the path -// is invalid or we cannot load the required mount data. If a filesystem has -// been updated since the last call to one of the mount functions, run -// UpdateMountInfo to see changes. +// FindMount returns the main Mount object for the filesystem which contains the +// file at the specified path. An error is returned if the path is invalid or if +// we cannot load the required mount data. If a mount has been updated since the +// last call to one of the mount functions, run UpdateMountInfo to see changes. func FindMount(path string) (*Mount, error) { - path, err := canonicalizePath(path) - if err != nil { - return nil, err - } - mountMutex.Lock() defer mountMutex.Unlock() - if err = loadMountInfo(); err != nil { + if err := loadMountInfo(); err != nil { return nil, err } - - // Traverse up the directory tree until we find a mountpoint - for { - if mnt, ok := mountsByPath[path]; ok { - return mnt, nil - } - - // Move to the parent directory unless we have reached the root. - parent := filepath.Dir(path) - if parent == path { - return nil, errors.Wrap(ErrNotAMountpoint, path) - } - path = parent + deviceNumber, err := getNumberOfContainingDevice(path) + if err != nil { + return nil, err } + mnt, ok := mountsByDevice[deviceNumber] + if !ok { + return nil, errors.Errorf("couldn't find mountpoint containing %q", path) + } + if mnt == nil { + return nil, filesystemRootDirNotVisibleError(deviceNumber) + } + return mnt, nil } -// GetMount returns the Mount object with a matching mountpoint. An error is -// returned if the path is invalid or we cannot load the required mount data. If -// a filesystem has been updated since the last call to one of the mount -// functions, run UpdateMountInfo to see changes. +// GetMount is like FindMount, except GetMount also returns an error if the path +// isn't the root directory of a filesystem. For example, if a filesystem is +// mounted at "/mnt" and the file "/mnt/a" exists, FindMount("/mnt/a") will +// succeed whereas GetMount("/mnt/a") will fail. func GetMount(mountpoint string) (*Mount, error) { - mountpoint, err := canonicalizePath(mountpoint) + mnt, err := FindMount(mountpoint) + if err != nil { + return nil, errors.Wrap(ErrNotAMountpoint, mountpoint) + } + // Check whether 'mountpoint' is the root directory of the filesystem, + // i.e. is the same directory as 'mnt.Path'. Use os.SameFile() (i.e., + // compare inode numbers) rather than compare canonical paths, since the + // filesystem might be fully mounted in multiple places. + fi1, err := os.Stat(mountpoint) if err != nil { return nil, err } - - mountMutex.Lock() - defer mountMutex.Unlock() - if err = loadMountInfo(); err != nil { + fi2, err := os.Stat(mnt.Path) + if err != nil { return nil, err } - - if mnt, ok := mountsByPath[mountpoint]; ok { - return mnt, nil + if !os.SameFile(fi1, fi2) { + return nil, errors.Wrap(ErrNotAMountpoint, mountpoint) } - - return nil, errors.Wrap(ErrNotAMountpoint, mountpoint) + return mnt, nil } -// getMountsFromLink returns the Mount objects which match the provided link. +// getMountsFromLink returns the Mount object which matches the provided link. // This link is formatted as a tag (e.g. =) similar to how they -// appear in "/etc/fstab". Currently, only "UUID" tokens are supported. Note -// that this can match multiple Mounts (due to the existence of bind mounts). An -// error is returned if the link is invalid or we cannot load the required mount -// data. If a filesystem has been updated since the last call to one of the -// mount functions, run UpdateMountInfo to see the change. -func getMountsFromLink(link string) ([]*Mount, error) { +// appear in "/etc/fstab". Currently, only "UUID" tokens are supported. An error +// is returned if the link is invalid or we cannot load the required mount data. +// If a mount has been updated since the last call to one of the mount +// functions, run UpdateMountInfo to see the change. +func getMountFromLink(link string) (*Mount, error) { // Parse the link linkComponents := strings.Split(link, "=") if len(linkComponents) != 2 { @@ -286,13 +308,16 @@ func getMountsFromLink(link string) ([]*Mount, error) { if err := loadMountInfo(); err != nil { return nil, err } - mnts, ok := mountsByDevice[deviceNumber] + mnt, ok := mountsByDevice[deviceNumber] if !ok { devicePath, _ := canonicalizePath(searchPath) return nil, errors.Wrapf(ErrFollowLink, "no mounts for device %q (%v)", devicePath, deviceNumber) } - return mnts, nil + if mnt == nil { + return nil, filesystemRootDirNotVisibleError(deviceNumber) + } + return mnt, nil } // makeLink returns a link of the form = where value is the tag -- cgit v1.2.3 From e71c5e4f70632b99a08d127b35e80a9e291e1938 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:33:54 -0700 Subject: filesystem: add unit tests for loadMountInfo() Add a version of loadMountInfo() that takes an io.Reader parameter to allow injecting a custom mountinfo file, then add some unit tests. --- filesystem/mountpoint.go | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) (limited to 'filesystem/mountpoint.go') diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index 50389c0..d9dbf37 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -24,6 +24,7 @@ package filesystem import ( "bufio" "fmt" + "io" "io/ioutil" "log" "os" @@ -133,21 +134,12 @@ func parseMountInfoLine(line string) *Mount { return mnt } -// loadMountInfo populates the Mount mappings by parsing /proc/self/mountinfo. -// It returns an error if the Mount mappings cannot be populated. -func loadMountInfo() error { - if mountsInitialized { - return nil - } +// This is separate from loadMountInfo() only for unit testing. +func readMountInfo(r io.Reader) error { mountsByPath := make(map[string]*Mount) mountsByDevice = make(map[DeviceNumber]*Mount) - file, err := os.Open("/proc/self/mountinfo") - if err != nil { - return err - } - defer file.Close() - scanner := bufio.NewScanner(file) + scanner := bufio.NewScanner(r) for scanner.Scan() { line := scanner.Text() mnt := parseMountInfoLine(line) @@ -185,7 +177,23 @@ func loadMountInfo() error { mountsByDevice[mnt.DeviceNumber] = mnt } } - mountsInitialized = true + return nil +} + +// loadMountInfo populates the Mount mappings by parsing /proc/self/mountinfo. +// It returns an error if the Mount mappings cannot be populated. +func loadMountInfo() error { + if !mountsInitialized { + file, err := os.Open("/proc/self/mountinfo") + if err != nil { + return err + } + defer file.Close() + if err := readMountInfo(file); err != nil { + return err + } + mountsInitialized = true + } return nil } -- cgit v1.2.3