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

Want /dev/zfs umask module parameter #4119

Closed
wants to merge 1 commit into from
Closed

Conversation

ryao
Copy link
Contributor

@ryao ryao commented Dec 17, 2015

We introduce a module parameter that enforces a umask on /dev/zfs. It
was originally meant to be set only at module load time, but some
difficulties with modprobe zfs zfsdev_umask=777 in local testing lead
me to allow its owner (default root) to change it at runtime. This
should ease the transition for those that rely on this behavior,
provided that we state in the release notes that future releases are
expected to remove the ability to modify it at runtime. When the cause
of that modprobe command failing to set the module is found, we can
tighten the permissions for a future release.

Closes #4100

Signed-off-by: Richard Yao [email protected]

We introduce a module parameter that enforces a umask on /dev/zfs. It
was originally meant to be set only at module load time, but some
difficulties with `modprobe zfs zfsdev_umask=777` in local testing lead
me to allow its owner (default root) to change it at runtime. This
should ease the transition for those that rely on this behavior,
provided that we state in the release notes that future releases are
expected to remove the ability to modify it at runtime. When the cause
of that modprobe command failing to set the module is found, we can
tighten the permissions for a future release.

Closes openzfs#4100

Signed-off-by: Richard Yao <[email protected]>
@@ -195,6 +195,9 @@
kmutex_t zfsdev_state_lock;
zfsdev_state_t *zfsdev_state_list;

/* The bits are stored in decimal format, not octal */
int zfsdev_umask = 77;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt using a decimal format would be easier for those reading the setting in /sys/module/zfs/parameters/zfsdev_umask, but I am open to switching to octal if people prefer that.

@tuxoko
Copy link
Contributor

tuxoko commented Dec 17, 2015

I don't see any point of doing this.
It's not at all caused by zfs, and every other device node is also affected, even if we fix the zfs part, the system is still completely security broken.

@behlendorf
Copy link
Contributor

@tuxoko raises a good point, and even if we add a patch like this it won't end up in a tagged release for quite some time. This is perhaps a good example of a patch which should just be carried in to the gentoo patch stack until such time as they fix this issue in their distribution.

@behlendorf
Copy link
Contributor

Closing. @ryao your welcome to apply an updated version of this to Gentoo, but for the moment there doesn't appear to be a need for this elsewhere.

@behlendorf behlendorf closed this Jan 8, 2016
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.

3 participants