add mount options to sysfs
RFC 1/1 add mount options to sysfs
Currently there is no good way to effectively globally restrict an access to sysfs files. It's possible only to chmod the sysfs' root/directories to fully deny access to sysfs (sub-)tree to some users or chmod files after they are created. The latter approach is racy, however.
The patch introduces sysfs mount options parsing and adds 4 new options:
uid, gid, mode and umask. uid, gid, and umask are classical options, mode is a global restricting mode mask that defined the most relaxed possible file mode. E.g. if mode=0750 then "chmod 0664" changes file's permissions to 0640.
If one uses "umask" then he also has to chmod already existing sysfs files. "mode" tries to simplify things by restricting _all_ files' permissions - both already existing and will be created in the future.
uid, gid, and umask values should be reachable when kobject is created, so it must not be located in sysfs' superblock - there is no one to one mapping of kobjects and sysfs_super_info. (If there are any thoughts about how kobject <-> superblock connection may be introduced, please tell me.) Only networking kobjects are connected to sysfs_super_info with the same net namespace, but if/when other namespaces are added to sysfs_super_info.ns it would be changed. Currently uid, gid, umask are stored in global struct. Only root process of the main namespace should change uid, gid, and umask options because most kobjects are system wide. I made a fake check of capable(CAP_SYS_ADMIN), but it needs some other check. In OpenVZ kernel container's root and real root have different capabilities, but this is not implemented in upstream kernel AFAIK.
TODO:
- introduce real check for uid/gid/umask
- document mount options
- divide the patch into option parsing and adding actual options
fs/sysfs/dir.c +3 -1
@@ -325,8 +325,10 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)atomic_set (&sd->s_active,0 ); sd->s_name = name; - sd->s_mode = mode; + sd->s_mode = mode & ~sysfs_perms.umask; sd->s_flags = type; + sd->s_uid = sysfs_perms.uid; + sd->s_gid = sysfs_perms.gid;return sd;
fs/sysfs/inode.c +5 -1
@@ -17,6 +17,7 @@#include <linux/backing-dev.h> #include <linux/capability.h> #include <linux/errno.h> +#include <linux/mount.h> #include <linux/sched.h> #include <linux/slab.h> #include <linux/sysfs.h>
@@ -217,8 +218,9 @@ static int sysfs_count_nlink(struct sysfs_dirent *sd)static void sysfs_refresh_inode (struct sysfs_dirent *sd,struct inode *inode) {struct sysfs_inode_attrs *iattrs = sd->s_iattr; +struct sysfs_super_info *sb_info =sysfs_info (inode->i_sb); - inode->i_mode = sd->s_mode; + inode->i_mode = sd->s_mode & (sb_info->mode | ~0777 );if (iattrs) {/* sysfs_dirent has non-default attributes * get them from persistent copy in sysfs_dirent
@@ -256,6 +258,8 @@ static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode) inode->i_op = &sysfs_inode_operations;set_default_inode_attr (inode, sd->s_mode); + inode->i_uid = sd->s_uid; + inode->i_gid = sd->s_gid;sysfs_refresh_inode (sd, inode);/* initialize inode according to type */
fs/sysfs/mount.c +82 -0
126 lines not on this page · raw message
fs/sysfs/sysfs.h +10 -0
@@ -69,6 +69,8 @@ struct sysfs_dirent {unsigned int s_flags;unsigned short s_mode; +uid_t s_uid; +gid_t s_gid;ino_t s_ino;struct sysfs_inode_attrs *s_iattr; };
@@ -130,6 +132,13 @@ struct sysfs_addrm_cxt {* mount.c */ +struct sysfs_perms { +unsigned int umask; +gid_t gid; +uid_t uid; +}; +extern struct sysfs_perms sysfs_perms; +/* * Each sb is associated with a set of namespace tags (i.e. * the network namespace of the task which mounted this sysfs
@@ -137,6 +146,7 @@ struct sysfs_addrm_cxt {*/ struct sysfs_super_info {const void *ns[KOBJ_NS_TYPES ]; +unsigned int mode; };#define sysfs_info (SB ) ((struct sysfs_super_info *)(SB->s_fs_info))extern struct sysfs_dirent sysfs_root;
Quoted 5 lines · Vasiliy Kulikov
Currently there is no good way to effectively globally restrict an access to sysfs files. It's possible only to chmod the sysfs' root/directories to fully deny access to sysfs (sub-)tree to some users or chmod files after they are created. The latter approach is racy, however.
Why do you want to do this? What is in sysfs files that is not gloabally ok to access? That should be fixed first, if at all, instead of wanting to modify the whole sysfs tree, right?
Quoted 5 lines
The patch introduces sysfs mount options parsing and adds 4 new options:
uid, gid, mode and umask. uid, gid, and umask are classical options, mode is a global restricting mode mask that defined the most relaxed possible file mode. E.g. if mode=0750 then "chmod 0664" changes file's permissions to 0640.
What is going to break if you do this? Have you tested it? I'd be very worried about this.
Again, what's the root problem you are trying to solve here?
thanks,
greg k-h
Quoted 10 lines · Greg KH
Quoted 5 lines · Vasiliy Kulikov
Currently there is no good way to effectively globally restrict an access to sysfs files. It's possible only to chmod the sysfs' root/directories to fully deny access to sysfs (sub-)tree to some users or chmod files after they are created. The latter approach is racy, however.
Why do you want to do this? What is in sysfs files that is not gloabally ok to access? That should be fixed first, if at all, instead of wanting to modify the whole sysfs tree, right?
I don't hide the goal (I didn't find any other weird permissions, if you mean this). It is primary about additional global controlable layer of defining permission:
1) *IF* another sensitive file with weird permissions is found, mount option is IMO the best temporary workaround.
2) Somebody might be worried about information leaks via world readable files - not strict bugs, but leaks in sense of local policy. See numerous discussions about hiding kernel addresses - there is no unified opinion about it. Some admins would be happy with denying access to almost all system information except some white list.
Quoted 8 lines
Quoted 5 lines
The patch introduces sysfs mount options parsing and adds 4 new options:
uid, gid, mode and umask. uid, gid, and umask are classical options, mode is a global restricting mode mask that defined the most relaxed possible file mode. E.g. if mode=0750 then "chmod 0664" changes file's permissions to 0640.What is going to break if you do this? Have you tested it? I'd be very worried about this.
I've tested it on my laptop (I'm writing booted with this kernel). By default nothing has changed (umask=0000, mode=0777). Mounting sysfs with mode=0770 leads to the predictable state - no sysfs information is available to nonroot, e.g. no ACPI battery state. umask is tested with pluging a USB flash.
mode has one advantage over umask. It is temporary - "mount -o remount,mode=0777" restores the initial state.
I'd want to implement similar options for debugfs.
Thanks,
Signature · 1 line
Vasiliy
Quoted 12 lines · Vasiliy Kulikov
Quoted 10 lines · Greg KH
Quoted 5 lines · Vasiliy Kulikov
Currently there is no good way to effectively globally restrict an access to sysfs files. It's possible only to chmod the sysfs' root/directories to fully deny access to sysfs (sub-)tree to some users or chmod files after they are created. The latter approach is racy, however.
Why do you want to do this? What is in sysfs files that is not gloabally ok to access? That should be fixed first, if at all, instead of wanting to modify the whole sysfs tree, right?
Quoted 6 lines
I don't hide the goal (I didn't find any other weird permissions, if you mean this). It is primary about additional global controlable layer of defining permission:
1) *IF* another sensitive file with weird permissions is found, mount option is IMO the best temporary workaround.
Maybe, but fixing the file would be the obvious solution.
Quoted 5 lines
2) Somebody might be worried about information leaks via world readable files - not strict bugs, but leaks in sense of local policy. See numerous discussions about hiding kernel addresses - there is no unified opinion about it. Some admins would be happy with denying access to almost all system information except some white list.
What file in sysfs "leaks" information like this? Please let us know and we will be glad to fix that.
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
The patch introduces sysfs mount options parsing and adds 4 new options:
uid, gid, mode and umask. uid, gid, and umask are classical options, mode is a global restricting mode mask that defined the most relaxed possible file mode. E.g. if mode=0750 then "chmod 0664" changes file's permissions to 0640.
Quoted 8 lines
Quoted 2 lines
What is going to break if you do this? Have you tested it? I'd be very worried about this.
I've tested it on my laptop (I'm writing booted with this kernel). By default nothing has changed (umask=0000, mode=0777). Mounting sysfs with mode=0770 leads to the predictable state - no sysfs information is available to nonroot, e.g. no ACPI battery state. umask is tested with pluging a USB flash.
A usb storage device is hardly a huge consumer of sysfs files, sorry. This would need a whole lot more testing before I would ever feel confortable with it, even if I agreed that something like this should be added (hint, I still don't.)
Quoted 4 lines
mode has one advantage over umask. It is temporary - "mount -o remount,mode=0777" restores the initial state.
I'd want to implement similar options for debugfs.
I wouldn't.
Again, lets fix the root problems here, if any, instead of adding complexity and probably breaking systems by changing permissions without anyone knowing about it.
thanks,
greg k-h
Quoted 4 lines · Greg KH
Quoted 2 lines
1) *IF* another sensitive file with weird permissions is found, mount option is IMO the best temporary workaround.
Maybe, but fixing the file would be the obvious solution.
I mean for a sysadmin, not for a developer.
What do you mean by "breaking system"? Root is able to chmod and chown sysfs files already, he may do "chmod -R" or similar. I suggest sane, race free way to globally restrict permissions *IF* root wants it.
Here https://lkml.org/lkml/2011/2/25/300 you, not aware of usefull applications of world-writable debugfs file, agreeded to statically restrict permissions of all files. I suggest more flexible and configurable in runtime solution. It doesn't break anything - default behaviour doesn't differ from current one. What has changed in your mind since 2/25?
Thanks,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Quoted 7 lines · Vasiliy Kulikov
Quoted 4 lines · Greg KH
Quoted 2 lines
1) *IF* another sensitive file with weird permissions is found, mount option is IMO the best temporary workaround.
Maybe, but fixing the file would be the obvious solution.
I mean for a sysadmin, not for a developer.
And I mean for the developer.
We have checks in place now to prevent this type of thing from happening again in the future. If it does, and it might, we will fix it, it's that simple.
Quoted 4 lines
What do you mean by "breaking system"? Root is able to chmod and chown sysfs files already, he may do "chmod -R" or similar. I suggest sane, race free way to globally restrict permissions *IF* root wants it.
If root wants it, they can do this today with a simple 1 line bash command, so I don't see the issue.
Quoted 6 lines
Here https://lkml.org/lkml/2011/2/25/300 you, not aware of usefull applications of world-writable debugfs file, agreeded to statically restrict permissions of all files. I suggest more flexible and configurable in runtime solution. It doesn't break anything - default behaviour doesn't differ from current one. What has changed in your mind since 2/25?
That's debugfs, not sysfs, which we are talking about here, right?
thanks,
greg k-h
Quoted 11 lines · Greg KH
Quoted 4 lines · Vasiliy Kulikov
Quoted 1 line · Greg KH
Maybe, but fixing the file would be the obvious solution.
I mean for a sysadmin, not for a developer.
And I mean for the developer.
We have checks in place now to prevent this type of thing from happening again in the future. If it does, and it might, we will fix it, it's that simple.
Simple indeed. But not as fast as simple:
https://lkml.org/lkml/2011/2/4/74
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=49d50fb1c28738ef6bad0c2b87d5355a1653fed5
More than 40 days from the report to the actual commit. Sometimes it needs some workaround.
Quoted 7 lines
Quoted 4 lines
What do you mean by "breaking system"? Root is able to chmod and chown sysfs files already, he may do "chmod -R" or similar. I suggest sane, race free way to globally restrict permissions *IF* root wants it.
If root wants it, they can do this today with a simple 1 line bash command, so I don't see the issue.
The issue is a race condition between the file creation and chmod'ing.
Quoted 8 lines
Quoted 6 lines
Here https://lkml.org/lkml/2011/2/25/300 you, not aware of usefull applications of world-writable debugfs file, agreeded to statically restrict permissions of all files. I suggest more flexible and configurable in runtime solution. It doesn't break anything - default behaviour doesn't differ from current one. What has changed in your mind since 2/25?
That's debugfs, not sysfs, which we are talking about here, right?
Correct. So, if I understood you, you are OK with adding mount options for debugfs, but not sysfs, right? What is the difference between them in sense of permissions?
Thanks,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Quoted 13 lines · Vasiliy Kulikov
Quoted 11 lines · Greg KH
Quoted 4 lines · Vasiliy Kulikov
Quoted 1 line · Greg KH
Maybe, but fixing the file would be the obvious solution.
I mean for a sysadmin, not for a developer.
And I mean for the developer.
We have checks in place now to prevent this type of thing from happening again in the future. If it does, and it might, we will fix it, it's that simple.
Quoted 7 lines
Simple indeed. But not as fast as simple:
https://lkml.org/lkml/2011/2/4/74
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=49d50fb1c28738ef6bad0c2b87d5355a1653fed5More than 40 days from the report to the actual commit. Sometimes it needs some workaround.
That's sad, but it is for a very rare device, right?
You can always pester us to get those changes in sooner as well.
Quoted 17 lines
Quoted 7 lines
Quoted 4 lines
What do you mean by "breaking system"? Root is able to chmod and chown sysfs files already, he may do "chmod -R" or similar. I suggest sane, race free way to globally restrict permissions *IF* root wants it.
If root wants it, they can do this today with a simple 1 line bash command, so I don't see the issue.
The issue is a race condition between the file creation and chmod'ing.
Quoted 7 lines
Quoted 6 lines
Here https://lkml.org/lkml/2011/2/25/300 you, not aware of usefull applications of world-writable debugfs file, agreeded to statically restrict permissions of all files. I suggest more flexible and configurable in runtime solution. It doesn't break anything - default behaviour doesn't differ from current one. What has changed in your mind since 2/25?
Quoted 5 lines
Quoted 1 line
That's debugfs, not sysfs, which we are talking about here, right?
Correct. So, if I understood you, you are OK with adding mount options for debugfs, but not sysfs, right? What is the difference between them in sense of permissions?
debugfs is "there are no rules", so changing the permissions on it shouldn't break anything as no userspace tools "should" rely on it. Now that really isn't true (see the perf stuff), but overall it is, so I don't worry about changing things there as much as sysfs, which has hundreds of tools relying on it.
thanks,
greg k-h
Quoted 13 lines · Greg KH
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Here https://lkml.org/lkml/2011/2/25/300 you, not aware of usefull applications of world-writable debugfs file, agreeded to statically restrict permissions of all files. I suggest more flexible and configurable in runtime solution. It doesn't break anything - default behaviour doesn't differ from current one. What has changed in your mind since 2/25?
Quoted 5 lines
Quoted 1 line
That's debugfs, not sysfs, which we are talking about here, right?
Correct. So, if I understood you, you are OK with adding mount options for debugfs, but not sysfs, right? What is the difference between them in sense of permissions?
Quoted 5 lines
debugfs is "there are no rules", so changing the permissions on it shouldn't break anything as no userspace tools "should" rely on it. Now that really isn't true (see the perf stuff), but overall it is, so I don't worry about changing things there as much as sysfs, which has hundreds of tools relying on it.
What would break if the default behaviour is not changed?
Thanks,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Quoted 5 lines · Vasiliy Kulikov
Quoted 4 lines · Greg KH
Quoted 3 lines
Correct. So, if I understood you, you are OK with adding mount options for debugfs, but not sysfs, right? What is the difference between them in sense of permissions?
Quoted 7 lines
Quoted 5 lines
debugfs is "there are no rules", so changing the permissions on it shouldn't break anything as no userspace tools "should" rely on it. Now that really isn't true (see the perf stuff), but overall it is, so I don't worry about changing things there as much as sysfs, which has hundreds of tools relying on it.
What would break if the default behaviour is not changed?
Err... sorry, s/would break/would it break/
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Quoted 12 lines · Vasiliy Kulikov
Quoted 11 lines · Vasiliy Kulikov
Quoted 9 lines · Greg KH
Quoted 3 lines
Correct. So, if I understood you, you are OK with adding mount options for debugfs, but not sysfs, right? What is the difference between them in sense of permissions?
debugfs is "there are no rules", so changing the permissions on it shouldn't break anything as no userspace tools "should" rely on it. Now that really isn't true (see the perf stuff), but overall it is, so I don't worry about changing things there as much as sysfs, which has hundreds of tools relying on it.
Quoted 3 lines
Quoted 1 line
What would break if the default behaviour is not changed?
Err... sorry, s/would break/would it break/
That's the question for you, we would be changing the kernel/user api here, and odds are, something will break.
thansk,
greg k-h
Hi Greg,
Quoted 2 lines · Greg KH
That's the question for you, we would be changing the kernel/user api here, and odds are, something will break.
I feel like this is going in circles. :)
Adding this feature (with no changes to the defaults) will:
- not break anything in the standard case
- allow a system owner to locally choose the DAC perms on the mounts
The question is about providing a temporal buffer between when a flaw is found in sysfs or debugfs and when the fix for it gets to the system owner. This, of course, presupposes that flaws are even publicly discovered in the first place. It gives paranoid system owners the option to lock down sysfs and debugfs from non-root users while still having them mounted without needing to make rather non-standard changes to boot time scripts beyond changing the fstab.
Given that things like tmpfs support these options, it seems only sane to suppor them here too.
-Kees
Signature · 2 lines
Kees Cook Ubuntu Security Team