Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mount: use device directly if we are root. #195

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rminnich
Copy link

The mount function in mount_linux.go checks if we are
uid 0 and, if so, tries to just do the mount directly.
If that fails, then it will fall through to the fusermount
path.

I've tested this on a server of my own and it works.

Signed-off-by: Ronald G. Minnich [email protected]

The mount function in mount_linux.go checks if we are
uid 0 and, if so, tries to just do the mount directly.
If that fails, then it will fall through to the fusermount
path.

I've tested this on a server of my own and it works.

Signed-off-by: Ronald G. Minnich <[email protected]>
@chrislusf
Copy link

What is the benefit of this PR? Is there any performance gain?

@zimbatm
Copy link

zimbatm commented Feb 22, 2019

@chrislusf it allows to remove the runtime dependency on fusermount and the c-fuse package IFF the user is guaranteed to be root

chrislusf added a commit to seaweedfs/fuse that referenced this pull request Feb 22, 2019
@chrislusf
Copy link

@zimbatm thanks for confirming!

@Freeaqingme
Copy link

I like this patch. In my case I (initially) wanted to run this in a chroot, but couldn't because of the other binary missing. That's also a use case that is fixed by this patch.

Having said that, I think there's even more room for improvement. At least in theory, it's possible for someone to not have uid 0, but still be allowed to mount devices (e.g. through CAP_SYS_ADMIN). You could also just attempt to mount it directly, and if that doesn't work, fallback on the external binary?

@HeikoSchlittermann
Copy link

I'd like to support this patch. It is definitly a gain of usabililty if we're independend on an external binary (even if for the uid=0 case only). The suggestions to first try it directly and then fallback to fusemount seem sensible.

I'd like to see this patch in the mainline :)

@tv42
Copy link
Member

tv42 commented May 8, 2020

I like the general idea, but you really shouldn't be running as root (or CAP_SYS_ADMIN either). I'd like to see a complete story about that; go is not great at doing setuid (EDIT: I mean dropping privs, not chmod u+s), so I feel like doing this "right" will involve a bit more than just calling these APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants