winfsp / cgofuse

Cross-platform FUSE library for Go - Works on Windows, macOS, Linux, FreeBSD, NetBSD, OpenBSD
https://winfsp.dev
MIT License
511 stars 82 forks source link

fuse: search a list of paths for `fusermount` executable #78

Closed djdv closed 1 year ago

djdv commented 1 year ago

The distribution I'm using (Gentoo) places fusermount in /usr/bin rather than /bin. As a result, host.Unmount() returns false (on my system the call stack is: posix_spawn->clone->ENOENT) Since the call to Unmount fails, host.Mount() will continue to block.

This patch replaces posix_spawn with posix_spawnp and changes the actual-parameter to just be the executable name "fusermount". posix_spawnp uses a formal-parameter file instead of path, and searches $PATH to find a location for file. (https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_spawn.html) Which should spawn a user's preferred fusermount executable, regardless of where it resides (as long as it actually exists within a directory contained in $PATH)

billziss-gh commented 1 year ago

I think I dislike the idea of using the PATH environment variable because it can pick up any fusermount program. Instead I suggest the alternative:

diff --git a/fuse/host_cgo.go b/fuse/host_cgo.go
index 8f8a019..a7bcdd2 100644
--- a/fuse/host_cgo.go
+++ b/fuse/host_cgo.go
@@ -634,9 +634,21 @@ static int hostUnmount(struct fuse *fuse, char *mountpoint)
    if (0 == umount2(mountpoint, MNT_DETACH))
        return 1;
    // linux: umount2 failed; try fusermount
-   char *argv[] =
+   char *paths[] =
    {
        "/bin/fusermount",
+       "/usr/bin/fusermount",
+   };
+   char *path = paths[0];
+   for (size_t i = 0; sizeof paths / sizeof paths[0] > i; i++)
+       if (0 == access(paths[i], X_OK))
+       {
+           path = paths[i];
+           break;
+       }
+   char *argv[] =
+   {
+       path,
        "-z",
        "-u",
        mountpoint,

This patch goes through a list of "approved" paths for fusermount and finds the first one that is executable. Then it proceeds as before.

Minimally tested, but it passes go test -v ./fuse.

djdv commented 1 year ago

Thanks! I had this concern as well. On one hand I feel like users/developers would expect their version of fusermount to be called, and that would make this more distro agnostic; but on the other hand, the chance for calling the wrong/malicious binary is there and that's not great. I think the suggestion is fine and it's what I was considering as an alternative too.

The only concern on my end is if users have other layouts from their package managers. I don't know of any Linux systems that do this, but /opt/bin and /usr/local/bin came to mind. Although I usually see those on BSD and SUN systems so it's likely not a concern here. And if those do come up for anyone, they can just be added to the list later.

billziss-gh commented 1 year ago

Merging in. Thanks.