From e2afd00118ba2c34f123d045f054a52248afe2a7 Mon Sep 17 00:00:00 2001 From: albertony <12441419+albertony@users.noreply.github.com> Date: Sat, 11 Jun 2022 16:18:48 +0200 Subject: [PATCH] mount: avoid incorrect or premature overlap check on windows See: #6234 --- cmd/cmount/mount.go | 4 ++-- cmd/cmount/mountpoint_other.go | 9 ++++++- cmd/cmount/mountpoint_windows.go | 23 ++++++++++++++---- cmd/mount/mount.go | 10 ++++++-- cmd/mount2/mount.go | 7 ++++++ cmd/mountlib/mount.go | 7 +----- cmd/mountlib/utils.go | 41 +++++++++++++------------------- 7 files changed, 60 insertions(+), 41 deletions(-) diff --git a/cmd/cmount/mount.go b/cmd/cmount/mount.go index 919f5d895..2b7b5d19e 100644 --- a/cmd/cmount/mount.go +++ b/cmd/cmount/mount.go @@ -149,14 +149,14 @@ func waitFor(fn func() bool) (ok bool) { // report an error when fusermount is called. func mount(VFS *vfs.VFS, mountPath string, opt *mountlib.Options) (<-chan error, func() error, error) { // Get mountpoint using OS specific logic - mountpoint, err := getMountpoint(mountPath, opt) + f := VFS.Fs() + mountpoint, err := getMountpoint(f, mountPath, opt) if err != nil { return nil, nil, err } fs.Debugf(nil, "Mounting on %q (%q)", mountpoint, opt.VolumeName) // Create underlying FS - f := VFS.Fs() fsys := NewFS(VFS) host := fuse.NewFileSystemHost(fsys) host.SetCapReaddirPlus(true) // only works on Windows diff --git a/cmd/cmount/mountpoint_other.go b/cmd/cmount/mountpoint_other.go index 1061d096a..0e69487d1 100644 --- a/cmd/cmount/mountpoint_other.go +++ b/cmd/cmount/mountpoint_other.go @@ -9,9 +9,10 @@ import ( "os" "github.com/rclone/rclone/cmd/mountlib" + "github.com/rclone/rclone/fs" ) -func getMountpoint(mountPath string, opt *mountlib.Options) (string, error) { +func getMountpoint(f fs.Fs, mountPath string, opt *mountlib.Options) (string, error) { fi, err := os.Stat(mountPath) if err != nil { return "", fmt.Errorf("failed to retrieve mount path information: %w", err) @@ -19,5 +20,11 @@ func getMountpoint(mountPath string, opt *mountlib.Options) (string, error) { if !fi.IsDir() { return "", errors.New("mount path is not a directory") } + if err = mountlib.CheckOverlap(f, mountPath); err != nil { + return "", err + } + if err = mountlib.CheckAllowNonEmpty(mountPath, opt); err != nil { + return "", err + } return mountPath, nil } diff --git a/cmd/cmount/mountpoint_windows.go b/cmd/cmount/mountpoint_windows.go index 49f92c9c2..0445187c7 100644 --- a/cmd/cmount/mountpoint_windows.go +++ b/cmd/cmount/mountpoint_windows.go @@ -94,7 +94,7 @@ func handleNetworkShareMountpath(mountpath string, opt *mountlib.Options) (strin } // handleLocalMountpath handles the case where mount path is a local file system path. -func handleLocalMountpath(mountpath string, opt *mountlib.Options) (string, error) { +func handleLocalMountpath(f fs.Fs, mountpath string, opt *mountlib.Options) (string, error) { // Assuming path is drive letter or directory path, not network share (UNC) path. // If drive letter: Must be given as a single character followed by ":" and nothing else. // Else, assume directory path: Directory must not exist, but its parent must. @@ -125,6 +125,9 @@ func handleLocalMountpath(mountpath string, opt *mountlib.Options) (string, erro } return "", fmt.Errorf("failed to retrieve mountpoint directory parent information: %w", err) } + if err = mountlib.CheckOverlap(f, mountpath); err != nil { + return "", err + } } return mountpath, nil } @@ -158,9 +161,19 @@ func handleVolumeName(opt *mountlib.Options, volumeName string) { // getMountpoint handles mounting details on Windows, // where disk and network based file systems are treated different. -func getMountpoint(mountpath string, opt *mountlib.Options) (mountpoint string, err error) { +func getMountpoint(f fs.Fs, mountpath string, opt *mountlib.Options) (mountpoint string, err error) { + // Inform about some options not relevant in this mode + if opt.AllowNonEmpty { + fs.Logf(nil, "--allow-non-empty flag does nothing on Windows") + } + if opt.AllowRoot { + fs.Logf(nil, "--allow-root flag does nothing on Windows") + } + if opt.AllowOther { + fs.Logf(nil, "--allow-other flag does nothing on Windows") + } - // First handle mountpath + // Handle mountpath var volumeName string if isDefaultPath(mountpath) { // Mount path indicates defaults, which will automatically pick an unused drive letter. @@ -172,10 +185,10 @@ func getMountpoint(mountpath string, opt *mountlib.Options) (mountpoint string, volumeName = mountpath[1:] // WinFsp requires volume prefix as UNC-like path but with only a single backslash } else { // Mount path is drive letter or directory path. - mountpoint, err = handleLocalMountpath(mountpath, opt) + mountpoint, err = handleLocalMountpath(f, mountpath, opt) } - // Second handle volume name + // Handle volume name handleVolumeName(opt, volumeName) // Done, return mountpoint to be used, together with updated mount options. diff --git a/cmd/mount/mount.go b/cmd/mount/mount.go index 923681c7c..9be432c5e 100644 --- a/cmd/mount/mount.go +++ b/cmd/mount/mount.go @@ -69,9 +69,17 @@ func mountOptions(VFS *vfs.VFS, device string, opt *mountlib.Options) (options [ // returns an error, and an error channel for the serve process to // report an error when fusermount is called. func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error, func() error, error) { + f := VFS.Fs() if runtime.GOOS == "darwin" { fs.Logf(nil, "macOS users: please try \"rclone cmount\" as it will be the default in v1.54") } + if err := mountlib.CheckOverlap(f, mountpoint); err != nil { + return nil, nil, err + } + if err := mountlib.CheckAllowNonEmpty(mountpoint, opt); err != nil { + return nil, nil, err + } + fs.Debugf(f, "Mounting on %q", mountpoint) if opt.DebugFUSE { fuse.Debug = func(msg interface{}) { @@ -79,8 +87,6 @@ func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error } } - f := VFS.Fs() - fs.Debugf(f, "Mounting on %q", mountpoint) c, err := fuse.Mount(mountpoint, mountOptions(VFS, opt.DeviceName, opt)...) if err != nil { return nil, nil, err diff --git a/cmd/mount2/mount.go b/cmd/mount2/mount.go index 48e2f820e..152f3a89e 100644 --- a/cmd/mount2/mount.go +++ b/cmd/mount2/mount.go @@ -145,9 +145,16 @@ func mountOptions(fsys *FS, f fs.Fs, opt *mountlib.Options) (mountOpts *fuse.Mou // report an error when fusermount is called. func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error, func() error, error) { f := VFS.Fs() + if err := mountlib.CheckOverlap(f, mountpoint); err != nil { + return nil, nil, err + } + if err := mountlib.CheckAllowNonEmpty(mountpoint, opt); err != nil { + return nil, nil, err + } fs.Debugf(f, "Mounting on %q", mountpoint) fsys := NewFS(VFS, opt) + // nodeFsOpts := &fusefs.PathNodeFsOptions{ // ClientInodes: false, // Debug: mountlib.DebugFUSE, diff --git a/cmd/mountlib/mount.go b/cmd/mountlib/mount.go index af0eaf5a0..25fc272e1 100644 --- a/cmd/mountlib/mount.go +++ b/cmd/mountlib/mount.go @@ -237,13 +237,8 @@ func NewMountCommand(commandName string, hidden bool, mount MountFn) *cobra.Comm // Mount the remote at mountpoint func (m *MountPoint) Mount() (daemon *os.Process, err error) { - if err = m.CheckOverlap(); err != nil { - return nil, err - } - if err = m.CheckAllowed(); err != nil { - return nil, err - } + // Ensure sensible defaults m.SetVolumeName(m.MountOpt.VolumeName) m.SetDeviceName(m.MountOpt.DeviceName) diff --git a/cmd/mountlib/utils.go b/cmd/mountlib/utils.go index 5148f818b..bf3a7edf9 100644 --- a/cmd/mountlib/utils.go +++ b/cmd/mountlib/utils.go @@ -34,22 +34,22 @@ func ClipBlocks(b *uint64) { } } -// CheckOverlap checks that root doesn't overlap with mountpoint -func (m *MountPoint) CheckOverlap() error { - name := m.Fs.Name() +// CheckOverlap checks that root doesn't overlap with a mountpoint +func CheckOverlap(f fs.Fs, mountpoint string) error { + name := f.Name() if name != "" && name != "local" { return nil } - rootAbs := absPath(m.Fs.Root()) - mountpointAbs := absPath(m.MountPoint) + rootAbs := absPath(f.Root()) + mountpointAbs := absPath(mountpoint) if strings.HasPrefix(rootAbs, mountpointAbs) || strings.HasPrefix(mountpointAbs, rootAbs) { - const msg = "mount point %q and directory to be mounted %q mustn't overlap" - return fmt.Errorf(msg, m.MountPoint, m.Fs.Root()) + const msg = "mount point %q (%q) and directory to be mounted %q (%q) mustn't overlap" + return fmt.Errorf(msg, mountpoint, mountpointAbs, f.Root(), rootAbs) } return nil } -// absPath is a helper function for MountPoint.CheckOverlap +// absPath is a helper function for CheckOverlap func absPath(path string) string { if abs, err := filepath.EvalSymlinks(path); err == nil { path = abs @@ -58,30 +58,21 @@ func absPath(path string) string { path = abs } path = filepath.ToSlash(path) + if runtime.GOOS == "windows" { + // Removes any UNC long path prefix to make sure a simple HasPrefix test + // in CheckOverlap works when one is UNC (root) and one is not (mountpoint). + path = strings.TrimPrefix(path, `//?/`) + } if !strings.HasSuffix(path, "/") { path += "/" } return path } -// CheckAllowed informs about ignored flags on Windows. If not on Windows -// and not --allow-non-empty flag is used, verify that mountpoint is empty. -func (m *MountPoint) CheckAllowed() error { - opt := &m.MountOpt - if runtime.GOOS == "windows" { - if opt.AllowNonEmpty { - fs.Logf(nil, "--allow-non-empty flag does nothing on Windows") - } - if opt.AllowRoot { - fs.Logf(nil, "--allow-root flag does nothing on Windows") - } - if opt.AllowOther { - fs.Logf(nil, "--allow-other flag does nothing on Windows") - } - return nil - } +// CheckAllowNonEmpty checks --allow-non-empty flag, and if not used verifies that mountpoint is empty. +func CheckAllowNonEmpty(mountpoint string, opt *Options) error { if !opt.AllowNonEmpty { - return CheckMountEmpty(m.MountPoint) + return CheckMountEmpty(mountpoint) } return nil }