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

Allow null pointer as mount syscall options #3771

Closed
bjin opened this issue Sep 13, 2015 · 4 comments
Closed

Allow null pointer as mount syscall options #3771

bjin opened this issue Sep 13, 2015 · 4 comments
Milestone

Comments

@bjin
Copy link

bjin commented Sep 13, 2015

I find that docker is not working properly with zfs 0.6.5 (See moby/moby#16258). The docker daemon log shows that mount failed with EINVAL for a newly created mountpoint=legacy dataset.

I write two simple program in C and Go, and it turns out that while mount("zroot/test", "/tmp/test", "zfs", 0, "") would succeed, mount("zroot/test", "/tmp/test", "zfs", 0, NULL) will trigger EINTVAL. The syscall Go package seems to prefer null pointer as mount options.

Additional tracing lead me to 0282c41, in zpl_parse_options(). What do you think of the idea to allow null mntopts here?

@behlendorf
Copy link
Contributor

@bjin this was done intentionally to ensure certain hints were passed via mount options to the kernel via the mount helper. Assuming NULL was invalid was a convenient way to simplify the code. I didn't expect this to be an issue because I wasn't aware of any applications which bypassed mount.zfs. We can certainly allow a NULL here but I'm curious why docker is bypassing the helper. Without it certain zfs properties will be ignored and some other functionally won't work properly.

@aderouineau
Copy link

@behlendorf, The following 2 points in moby/moby/issues/16258 should be of interest.

@Mic92 mentions that:

zfs mount is not an option, because selinux labels have to passed with the mount.

@bjin referenced the following snippet from https://github.com/golang/sys/blob/master/unix/syscall_linux.go#L821

// Certain file systems get rather angry and EINVAL if you give
// them an empty string of data, rather than NULL.

@Mic92
Copy link
Contributor

Mic92 commented Sep 16, 2015

mount.zfs could be an option, since it supports selinux.
I will try this one. Do you know, if this is also part of other zfs distribution such as freebsd?

behlendorf added a commit to behlendorf/zfs that referenced this issue Sep 19, 2015
Passing NULL for the mount data should not result in EINVAL.  It
should be treated as if an empty string were passed and succeed.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#3771
@behlendorf
Copy link
Contributor

The bug here has been addressed in 0.6.5.1. But just so you're aware you can pass selinux contexts as a mount option using -o with ZoL. See mount.zfs(8). Alternately you can set this as dataset properties on Linux if that's more convenient.

zfs mount -o fscontext|defcontext|rootcontext tank/fs

@behlendorf behlendorf added this to the 0.7.0 milestone Sep 19, 2015
behlendorf added a commit that referenced this issue Sep 19, 2015
Passing NULL for the mount data should not result in EINVAL.  It
should be treated as if an empty string were passed and succeed.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ned Bass <bass6@llnl.gov>
Issue #3771
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

No branches or pull requests

4 participants