3.8: access permission filesystem
PATCH 1/1 3.8: access permission filesystem
I am in the process of catching up with the last two years or so. Right now, I am at the changes involving user namespaces.
I have two possible implementations, both working equally well in a shared environment. Since I am not familiar with namespaces in general and user namespaces in particular, I would like you to look over the patches and tell me, what you think.
Are the patches good so far? Are there are any things I missed and must consider? Maybe, I am completely off track? Anything else?
I included both patches inline below. The patches are also available as separate branches at github
https://github.com/olafdietsche/linux-accessfs/tree/tmp-user-ns-1
https://github.com/olafdietsche/linux-accessfs/tree/tmp-user-ns-2
I am leaning toward the second patch. Although it is a little bit longer than the first one, it involves no user id conversions.
fs/accessfs/inode.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
fs/accessfs/inode.c +6 -6
@@ -122,8 +122,8 @@ static void accessfs_init_inode(struct inode *inode, struct accessfs_entry *pe) {static const struct timespec epoch = {0 ,0 }; inode->i_private = pe; - inode->i_uid = pe->attr->uid; - inode->i_gid = pe->attr->gid; +i_uid_write (inode, pe->attr->uid); +i_gid_write (inode, pe->attr->gid); inode->i_mode = pe->attr->mode;/* inode->i_blksize = PAGE_CACHE_SIZE;
@@ -269,8 +269,8 @@ static int accessfs_notify_change(struct dentry *dentry, struct iattr *iattr)setattr_copy (i, iattr); pe = (struct accessfs_entry *) i->i_private; - pe->attr->uid = i->i_uid; - pe->attr->gid = i->i_gid; + pe->attr->uid =i_uid_read (i); + pe->attr->gid =i_gid_read (i); pe->attr->mode = i->i_mode;return 0 ; }
@@ -363,9 +363,9 @@ static struct dentry *accessfs_mount(struct file_system_type *fs_type,int accessfs_permitted (struct access_attr *p,int mask) {mode_t mode = p->mode; -if (current_fsuid () == p->uid) +if (uid_eq (current_fsuid (),make_kuid (current_user_ns (), p->uid))) mode >>=6 ; -else if (in_group_p (p->gid)) +else if (in_group_p (make_kgid (current_user_ns (), p->gid))) mode >>=3 ;return (mode & mask) == mask;
fs/accessfs/capabilities.c | 4 ++-- fs/accessfs/inode.c | 8 ++++---- fs/accessfs/ip.c | 4 ++-- include/linux/accessfs_fs.h | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-)
fs/accessfs/capabilities.c +2 -2
@@ -83,8 +83,8 @@ static int __init init_capabilities(void)return -ENOTDIR ;for (i =0 ; i <ARRAY_SIZE (caps); ++i) { - caps[i].uid =0 ; - caps[i].gid =0 ; + caps[i].uid =GLOBAL_ROOT_UID ; + caps[i].gid =GLOBAL_ROOT_GID ; caps[i].mode =S_IXUSR ; err =accessfs_register (dir, names[i], &caps[i]);if (err) {
fs/accessfs/inode.c +4 -4
@@ -115,7 +115,7 @@ static struct accessfs_direntry accessfs_rootdir = {LIST_HEAD_INIT (accessfs_rootdir.node.siblings), 1, &accessfs_rootdir.attr },NULL ,LIST_HEAD_INIT (accessfs_rootdir.children), - { 0, 0, S_IFDIR | 0755 } + { GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, S_IFDIR | 0755 } };static void accessfs_init_inode (struct inode *inode,struct accessfs_entry *pe)
@@ -174,8 +174,8 @@ static int accessfs_node_init(struct accessfs_direntry *parent, de->name[len] =0 ; de->ino = ++ino; de->attr = attr; - de->attr->uid =0 ; - de->attr->gid =0 ; + de->attr->uid =GLOBAL_ROOT_UID ; + de->attr->gid =GLOBAL_ROOT_GID ; de->attr->mode = mode;list_add_tail (&de->hash, &hash);
@@ -363,7 +363,7 @@ static struct dentry *accessfs_mount(struct file_system_type *fs_type,int accessfs_permitted (struct access_attr *p,int mask) {mode_t mode = p->mode; -if (current_fsuid () == p->uid) +if (uid_eq (current_fsuid (), p->uid)) mode >>=6 ;else if (in_group_p (p->gid)) mode >>=3 ;
fs/accessfs/ip.c +2 -2
@@ -66,8 +66,8 @@ static int __init init_ip(void)for (i =1 ; i < max_prot_sock; ++i) {char buf[sizeof ("65536" )]; - bind_to_port[i].uid =0 ; - bind_to_port[i].gid =0 ; + bind_to_port[i].uid =GLOBAL_ROOT_UID ; + bind_to_port[i].gid =GLOBAL_ROOT_GID ; bind_to_port[i].mode = i <PROT_SOCK ?S_IXUSR :S_IXUGO ;sprintf (buf,"%d" , i);accessfs_register (dir, buf, &bind_to_port[i]);
include/linux/accessfs_fs.h +2 -2
@@ -14,8 +14,8 @@#include <net/sock.h> struct access_attr { -uid_t uid; -gid_t gid; +kuid_t uid; +kgid_t gid;mode_t mode; };
Quoted 11 lines · Olaf Dietsche
I am in the process of catching up with the last two years or so. Right now, I am at the changes involving user namespaces.
I have two possible implementations, both working equally well in a shared environment. Since I am not familiar with namespaces in general and user namespaces in particular, I would like you to look over the patches and tell me, what you think.
Are the patches good so far? Are there are any things I missed and must consider? Maybe, I am completely off track? Anything else?
Quoted 8 lines
I included both patches inline below. The patches are also available as separate branches at github
https://github.com/olafdietsche/linux-accessfs/tree/tmp-user-ns-1
https://github.com/olafdietsche/linux-accessfs/tree/tmp-user-ns-2I am leaning toward the second patch. Although it is a little bit longer than the first one, it involves no user id conversions.
Using kuid's and kgid's throughout as your second patch does is best. Conversion is only needed on normal filesystems because they have a backing store and reside on disk. As accessfs appears not to have backing store, storing things with kuid's and kgid's is the preferred method.
Your first patch is buggy as it uses current_user_ns(). Something a filesystem in general should not care about.
I don't see anything wrong with your second patch.
Quoted 1 line
From what little I understand of accessfs, I expect you will want to
play with and come up to speed on namespaces, as namespaces change the universe of objects you will be dealing with, in some subtle but interesting ways. At least assuming anyone in who uses accessfs is going to be using more than a single container.
Eric
Quoted 78 lines
fs/accessfs/capabilities.c +2
-2@@ -83,8 +83,8 @@ static int __init init_capabilities(void)return -ENOTDIR ;for (i =0 ; i <ARRAY_SIZE (caps); ++i) { - caps[i].uid =0 ; - caps[i].gid =0 ; + caps[i].uid =GLOBAL_ROOT_UID ; + caps[i].gid =GLOBAL_ROOT_GID ; caps[i].mode =S_IXUSR ; err =accessfs_register (dir, names[i], &caps[i]);if (err) {fs/accessfs/inode.c +4
-4@@ -115,7 +115,7 @@ static struct accessfs_direntry accessfs_rootdir = {LIST_HEAD_INIT (accessfs_rootdir.node.siblings), 1, &accessfs_rootdir.attr },NULL ,LIST_HEAD_INIT (accessfs_rootdir.children), - { 0, 0, S_IFDIR | 0755 } + { GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, S_IFDIR | 0755 } };static void accessfs_init_inode (struct inode *inode,struct accessfs_entry *pe)@@ -174,8 +174,8 @@ static int accessfs_node_init(struct accessfs_direntry *parent, de->name[len] =0 ; de->ino = ++ino; de->attr = attr; - de->attr->uid =0 ; - de->attr->gid =0 ; + de->attr->uid =GLOBAL_ROOT_UID ; + de->attr->gid =GLOBAL_ROOT_GID ; de->attr->mode = mode;list_add_tail (&de->hash, &hash);@@ -363,7 +363,7 @@ static struct dentry *accessfs_mount(struct file_system_type *fs_type,int accessfs_permitted (struct access_attr *p,int mask) {mode_t mode = p->mode; -if (current_fsuid () == p->uid) +if (uid_eq (current_fsuid (), p->uid)) mode >>=6 ;else if (in_group_p (p->gid)) mode >>=3 ;fs/accessfs/ip.c +2
-2@@ -66,8 +66,8 @@ static int __init init_ip(void)for (i =1 ; i < max_prot_sock; ++i) {char buf[sizeof ("65536" )]; - bind_to_port[i].uid =0 ; - bind_to_port[i].gid =0 ; + bind_to_port[i].uid =GLOBAL_ROOT_UID ; + bind_to_port[i].gid =GLOBAL_ROOT_GID ; bind_to_port[i].mode = i <PROT_SOCK ?S_IXUSR :S_IXUGO ;sprintf (buf,"%d" , i);accessfs_register (dir, buf, &bind_to_port[i]);include/linux/accessfs_fs.h +2
-2@@ -14,8 +14,8 @@#include <net/sock.h> struct access_attr { -uid_t uid; -gid_t gid; +kuid_t uid; +kgid_t gid;mode_t mode; };
Quoted 22 lines · [email protected] (Eric W. Biederman) writes:
Quoted 11 lines · Olaf Dietsche
I am in the process of catching up with the last two years or so. Right now, I am at the changes involving user namespaces.
I have two possible implementations, both working equally well in a shared environment. Since I am not familiar with namespaces in general and user namespaces in particular, I would like you to look over the patches and tell me, what you think.
Are the patches good so far? Are there are any things I missed and must consider? Maybe, I am completely off track? Anything else?
Quoted 8 lines
I included both patches inline below. The patches are also available as separate branches at github
https://github.com/olafdietsche/linux-accessfs/tree/tmp-user-ns-1
https://github.com/olafdietsche/linux-accessfs/tree/tmp-user-ns-2I am leaning toward the second patch. Although it is a little bit longer than the first one, it involves no user id conversions.
Quoted 8 lines
Using kuid's and kgid's throughout as your second patch does is best. Conversion is only needed on normal filesystems because they have a backing store and reside on disk. As accessfs appears not to have backing store, storing things with kuid's and kgid's is the preferred method.
Your first patch is buggy as it uses
current_user_ns(). Something a filesystem in general should not care about.
I have seen similar uses in other filesystems like ext3, jfs or debugfs. What would be the correct way to use make_kuid() or make_gid()?
Quoted 1 line
I don't see anything wrong with your second patch.
Thanks a lot for this fast response and your guidance. So, I will dump the first and continue with the second patch.
Quoted 7 lines
From what little I understand of accessfs, I expect you will want to play with and come up to speed on namespaces, as namespaces change the universe of objects you will be dealing with, in some subtle but interesting ways. At least assuming anyone in who uses accessfs is going to be using more than a single container.
Eric
Regards, Olaf