From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:46:52
The vfs does not apply the umask for file systems that support acls.
The test used for this used to be called IS_POSIXACL(). Switch to a new
IS_ACL() test to check for either posix acls or richacls instead. Add a
new MS_RICHACL flag and IS_RICHACL() test for richacls alone. The
IS_POSIXACL() test is still needed in some places like nfsd.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Andreas Dilger <redacted>
Reviewed-by: Steve French <redacted>
---
fs/Kconfig | 3 +++
fs/namei.c | 6 +++---
include/linux/fs.h | 12 ++++++++++++
include/uapi/linux/fs.h | 3 ++-
4 files changed, 20 insertions(+), 4 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:46:54
Normally, deleting a file requires MAY_WRITE access to the parent
directory. With richacls, a file may be deleted with MAY_DELETE_CHILD access
to the parent directory or with MAY_DELETE_SELF access to the file.
To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
when checking for delete access inside a directory, and MAY_DELETE_SELF
when checking for delete access to a file itelf.
The MAY_DELETE_SELF permission overrides the sticky directory check.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Steve French <redacted>
---
fs/namei.c | 20 ++++++++++++--------
include/linux/fs.h | 2 ++
2 files changed, 14 insertions(+), 8 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:46:55
We will need to call iop->permission and iop->get_acl from
inode_change_ok() for additional permission checks, and both take a
non-const inode.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Andreas Dilger <redacted>
Reviewed-by: Steve French <redacted>
---
fs/attr.c | 2 +-
include/linux/fs.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:46:57
A richacl consists of an NFSv4 acl and an owner, group, and other mask.
These three masks correspond to the owner, group, and other file
permission bits, but they contain NFSv4 permissions instead of POSIX
permissions.
Each entry in the NFSv4 acl applies to the file owner (OWNER@), the
owning group (GROUP@), everyone (EVERYONE@), or to a specific uid or
gid.
As in the standard POSIX file permission model, each process is the
owner, group, or other file class. A richacl grants a requested access
only if the NFSv4 acl in the richacl grants the access (according to the
NFSv4 permission check algorithm), and the file mask that applies to the
process includes the requested permissions.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/Makefile | 1 +
fs/richacl.c | 65 ++++++++++++++++
include/linux/richacl.h | 179 +++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/richacl.h | 99 ++++++++++++++++++++++++
5 files changed, 345 insertions(+)
create mode 100644 fs/richacl.c
create mode 100644 include/linux/richacl.h
create mode 100644 include/uapi/linux/richacl.h
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:46:59
Compute upper bound owner, group, and other file masks with as few
permissions as possible without denying any permissions that the NFSv4
acl in a richacl grants.
This algorithm is used when a file inherits an acl at create time and
when an acl is set via a mechanism that does not provide file masks
(such as setting an acl via nfsd). When user-space sets an acl via
setxattr, the extended attribute already includes the file masks.
Setting an acl also sets the file mode permission bits: they are
determined by the file masks; see richacl_masks_to_mode().
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 158 insertions(+)
@@ -181,3 +181,160 @@ richacl_want_to_mask(unsigned int want)returnmask;}EXPORT_SYMBOL_GPL(richacl_want_to_mask);++/*+*Note:functionslikerichacl_allowed_to_who(),richacl_group_class_allowed(),+*andrichacl_compute_max_masks()iteratethroughtheentireaclinreverse+*orderasanoptimization.+*+*Inthestandardalgorithm,acesareconsideredinforwardorder.Whena+*processmatchesanace,thepermissionsintheaceareeitherallowedor+*denieddependingontheacetype.Onceapermissionhasbeenallowedor+*denied,itisnolongerconsideredinfurtheraces.+*+*Byiteratingthroughtheaclinreverseorder,wecancomputethesame+*resultwithouthavingtokeeptrackofwhichpermissionshavebeenallowed+*anddeniedalready.+*/++/**+*richacl_allowed_to_who-permissionsallowedtoaspecificwhovalue+*+*Computethemaximummaskvaluesallowedtoaspecificwhovalue,taking+*everyone@acesintoaccount.+*/+staticunsignedintrichacl_allowed_to_who(structrichacl*acl,+structrichace*who)+{+structrichace*ace;+unsignedintallowed=0;++richacl_for_each_entry_reverse(ace,acl){+if(richace_is_inherit_only(ace))+continue;+if(richace_is_same_identifier(ace,who)||+richace_is_everyone(ace)){+if(richace_is_allow(ace))+allowed|=ace->e_mask;+elseif(richace_is_deny(ace))+allowed&=~ace->e_mask;+}+}+returnallowed;+}++/**+*richacl_group_class_allowed-maximumpermissionsofthegroupclass+*+*Computethemaximummaskvaluesallowedtoaprocessinthegroupclass+*(i.e.,aprocesswhichisnottheownerbutisintheowninggroupor+*matchesauserorgroupaclentry).Thisincludespermissionsgrantedor+*deniedbyeveryone@aces.+*+*Seerichacl_compute_max_masks().+*/+staticunsignedintrichacl_group_class_allowed(structrichacl*acl)+{+structrichace*ace;+unsignedinteveryone_allowed=0,group_class_allowed=0;+inthad_group_ace=0;++richacl_for_each_entry_reverse(ace,acl){+if(richace_is_inherit_only(ace)||+richace_is_owner(ace))+continue;++if(richace_is_everyone(ace)){+if(richace_is_allow(ace))+everyone_allowed|=ace->e_mask;+elseif(richace_is_deny(ace))+everyone_allowed&=~ace->e_mask;+}else{+group_class_allowed|=+richacl_allowed_to_who(acl,ace);++if(richace_is_group(ace))+had_group_ace=1;+}+}+/*+*Iftheacldoesn'tcontainanygroup@aces,richacl_allowed_to_who()+*wasn'tcalledfortheowninggroup.Wecouldmakethatcallnow,but+*wealreadyknowtheresult(everyone_allowed).+*/+if(!had_group_ace)+group_class_allowed|=everyone_allowed;+returngroup_class_allowed;+}++/**+*richacl_compute_max_masks-computeupperboundmasks+*+*Computesupperboundowner,group,andothermaskssothatnoneofthe+*permissionsallowedbytheaclaredisabled.+*+*Wedon'tmakeassumptionsaboutwhotheownerissothattheownercan+*changewithnoeffectonthefilemasksorfilemodepermissionbits;this+*meansthatwemustassumethatallentriescanmatchtheowner.+*/+voidrichacl_compute_max_masks(structrichacl*acl)+{+unsignedintgmask=~0;+structrichace*ace;++/*+*@gmaskcontainsallpermissionswhichthegroupclassisever+*allowed.Weuseittoavoidaddingpermissionstothegroupmask+*fromeveryone@allowaceswhichthegroupclassisalwaysdenied+*throughotheraces.Forexample,thefollowingaclwouldotherwise+*resultinagroupmaskofrw:+*+*group@:w::deny+*everyone@:rw::allow+*+*Avoidcomputing@gmaskforaclswhichdonotincludeanygroupclass+*denyaces:insuchacls,thegroupclassisneverdeniedany+*permissionsfromeveryone@allowaces,andthegroupclasscannot+*havefewerpermissionsthantheotherclass.+*/++restart:+acl->a_owner_mask=0;+acl->a_group_mask=0;+acl->a_other_mask=0;++richacl_for_each_entry_reverse(ace,acl){+if(richace_is_inherit_only(ace))+continue;++if(richace_is_owner(ace)){+if(richace_is_allow(ace))+acl->a_owner_mask|=ace->e_mask;+elseif(richace_is_deny(ace))+acl->a_owner_mask&=~ace->e_mask;+}elseif(richace_is_everyone(ace)){+if(richace_is_allow(ace)){+acl->a_owner_mask|=ace->e_mask;+acl->a_group_mask|=ace->e_mask&gmask;+acl->a_other_mask|=ace->e_mask;+}elseif(richace_is_deny(ace)){+acl->a_owner_mask&=~ace->e_mask;+acl->a_group_mask&=~ace->e_mask;+acl->a_other_mask&=~ace->e_mask;+}+}else{+if(richace_is_allow(ace)){+acl->a_owner_mask|=ace->e_mask&gmask;+acl->a_group_mask|=ace->e_mask&gmask;+}elseif(richace_is_deny(ace)&&gmask==~0){+gmask=richacl_group_class_allowed(acl);+if(likely(gmask!=~0))+/* should always be true */+gotorestart;+}+}+}++acl->a_flags&=~(RICHACL_WRITE_THROUGH|RICHACL_MASKED);+}+EXPORT_SYMBOL_GPL(richacl_compute_max_masks);
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:47:05
Doing a chmod() sets the file mode, which includes the file permission
bits. When a file has a richacl, the permissions that the richacl
grants need to be limited to what the new file permission bits allow.
This is done by setting the file masks in the richacl to what the file
permission bits map to. The richacl access check algorithm takes the
file masks into account, which ensures that the richacl cannot grant too
many permissions.
It is possible to explicitly add permissions to the file masks which go
beyond what the file permission bits can grant (like the
RICHACE_WRITE_ACL permission). The POSIX.1 standard calls this an
alternate file access control mechanism. A subsequent chmod() would
ensure that those permissions are disabled again.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 72 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:47:07
When a new file is created, it can inherit an acl from its parent
directory; this is similar to how default acls work in POSIX ACLs.
As with POSIX ACLs, if a file inherits an acl from its parent directory,
the intersection between the create mode and the permissions granted by
the inherited acl determines the file masks and file permission bits,
and the umask is ignored.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/richacl.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 2 +
2 files changed, 153 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:49:51
Automatic Inheritance (AI) allows changes to the acl of a directory to
propagate down to children.
This is mostly implemented in user space: when a process changes the
permissions of a directory and Automatic Inheritance is enabled for that
directory, the process must propagate those changes to all children,
recursively.
The kernel enables this by keeping track of which permissions have been
inherited at create time. In addition, it makes sure that permission
propagation is turned off when the permissions are set explicitly (for
example, upon create or chmod).
Automatic Inheritance works as follows:
- When the RICHACL_AUTO_INHERIT flag in the acl of a file or directory
is not set, the file or directory is not affected by AI.
- When the RICHACL_AUTO_INHERIT flag in the acl of a directory is set
and a file or subdirectory is created in that directory, the
inherited acl will have the RICHACL_AUTO_INHERIT flag set, and all
inherited aces will have the RICHACE_INHERITED_ACE flag set. This
allows user space to distinguish between aces which have been
inherited and aces which have been explicitly added.
- When the RICHACL_PROTECTED acl flag in the acl of a file or directory
is set, AI will not modify the acl. This does not affect propagation
of permissions from the file to its children (if the file is a
directory).
Linux does not have a way of creating files or directories without setting the
file permission bits, so all files created inside a directory with
RICHACL_AUTO_INHERIT set will have the RICHACL_PROTECTED flag set. This
effectively disables Automatic Inheritance.
Protocols which support creating files without specifying permissions can
explicitly clear the RICHACL_PROTECTED flag after creating a file and reset the
file masks to "undo" applying the create mode; see richacl_compute_max_masks().
They should set the RICHACL_DEFAULTED flag. (A mechanism that would allow to
indicate to the kernel to ignore the create mode in the first place when there
are inherited permissions would be nice to have.)
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/richacl.c | 20 +++++++++++++++++++-
include/linux/richacl.h | 12 ++++++++++++
include/uapi/linux/richacl.h | 11 ++++++++++-
3 files changed, 41 insertions(+), 2 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:49:51
Cache richacls in struct inode so that this doesn't have to be done
individually in each filesystem. This is similar to POSIX ACLs.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/inode.c | 13 +++++---
fs/richacl.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 5 ++-
include/linux/richacl.h | 11 +++++++
4 files changed, 105 insertions(+), 5 deletions(-)
@@ -20,6 +20,87 @@#include<linux/slab.h>#include<linux/richacl.h>+voidset_cached_richacl(structinode*inode,structrichacl*acl)+{+structbase_acl*old;++old=xchg(&inode->i_acl,&richacl_get(acl)->a_base);+if(!is_uncached_acl(old))+base_acl_put(old);+}+EXPORT_SYMBOL_GPL(set_cached_richacl);++voidforget_cached_richacl(structinode*inode)+{+__forget_cached_acl(&inode->i_acl);+}+EXPORT_SYMBOL_GPL(forget_cached_richacl);++structrichacl*get_richacl(structinode*inode)+{+structbase_acl*sentinel,*base_acl;+structrichacl*acl;++if(!IS_RICHACL(inode))+returnNULL;++/*+*Thesentinelisusedtodetectwhenanotheroperationlike+*set_cached_richacl()orforget_cached_richacl()raceswith+*get_richacl().+*Itisguaranteedthatis_uncached_acl(sentinel)istrue.+*/++base_acl=__get_cached_acl(&inode->i_acl);+if(!is_uncached_acl(base_acl))+returnrichacl(base_acl);++sentinel=uncached_acl_sentinel(current);++/*+*IftheACLisn'tbeingreadyet,setoursentinel.Otherwise,the+*currentvalueoftheACLwillnotbeACL_NOT_CACHEDandsoourown+*sentinelwillnotbeset;anothertaskwillupdatethecache.We+*couldwaitforthatothertasktocompleteitsjob,butit'seasier+*tojustcall->get_acltofetchtheACLourself.(Thisisgoingto+*beanunlikelyrace.)+*/+if(cmpxchg(&inode->i_acl,ACL_NOT_CACHED,sentinel)!=ACL_NOT_CACHED)+/* fall through */;++/*+*Normally,theACLreturnedby->get_richaclwillbecached.+*Afilesystemcanpreventthatbycalling+*forget_cached_richacl(inode)in->get_richacl.+*+*Ifthefilesystemdoesn'thavea->get_richaclfunctionatall,+*we'lljustcreatethenegativecacheentry.+*/+if(!inode->i_op->get_richacl){+set_cached_richacl(inode,NULL);+returnNULL;+}++acl=inode->i_op->get_richacl(inode);+if(IS_ERR(acl)){+/*+*Removeoursentinelsothatwedon'tblockfutureattempts+*tocachetheACL.+*/+cmpxchg(&inode->i_acl,sentinel,ACL_NOT_CACHED);+returnacl;+}++/*+*Cachetheresult,butonlyifoursentinelisstillinplace.+*/+richacl_get(acl);+if(unlikely(cmpxchg(&inode->i_acl,sentinel,&acl->a_base)!=sentinel))+richacl_put(acl);+returnacl;+}+EXPORT_SYMBOL_GPL(get_richacl);+/***richacl_alloc-allocatearichacl*@count:numberofentries
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:49:51
ACLs are considered equivalent to file modes if they only consist of
owner@, group@, and everyone@ entries, the owner@ permissions do not
depend on whether the owner is a member in the owning group, and no
inheritance flags are set. This test is used to avoid storing richacls
if the acl can be computed from the file permission bits.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 105 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:49:51
We need to map from POSIX permissions to NFSv4 permissions when a
chmod() is done, from NFSv4 permissions to POSIX permissions when an acl
is set (which implicitly sets the file permission bits), and from the
MAY_READ/MAY_WRITE/MAY_EXEC/MAY_APPEND flags to NFSv4 permissions when
doing an access check in a richacl.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 118 +++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 3 ++
include/uapi/linux/richacl.h | 44 ++++++++++++++++
3 files changed, 165 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:49:51
Richacls support permissions that allow to take ownership of a file,
change the file permissions, and set the file timestamps. Support that
by introducing new permission mask flags and by checking for those mask
flags in inode_change_ok().
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Steve French <redacted>
---
fs/attr.c | 79 +++++++++++++++++++++++++++++++++++++++++++++---------
include/linux/fs.h | 3 +++
2 files changed, 70 insertions(+), 12 deletions(-)
@@ -47,22 +106,18 @@ int inode_change_ok(struct inode *inode, struct iattr *attr)return0;/* Make sure a caller can chown. */-if((ia_valid&ATTR_UID)&&-(!uid_eq(current_fsuid(),inode->i_uid)||-!uid_eq(attr->ia_uid,inode->i_uid))&&-!capable_wrt_inode_uidgid(inode,CAP_CHOWN))-return-EPERM;+if(ia_valid&ATTR_UID)+if(!inode_uid_change_ok(inode,attr->ia_uid))+return-EPERM;/* Make sure caller can chgrp. */-if((ia_valid&ATTR_GID)&&-(!uid_eq(current_fsuid(),inode->i_uid)||-(!in_group_p(attr->ia_gid)&&!gid_eq(attr->ia_gid,inode->i_gid)))&&-!capable_wrt_inode_uidgid(inode,CAP_CHOWN))-return-EPERM;+if(ia_valid&ATTR_GID)+if(!inode_gid_change_ok(inode,attr->ia_gid))+return-EPERM;/* Make sure a caller can chmod. */if(ia_valid&ATTR_MODE){-if(!inode_owner_or_capable(inode))+if(!inode_owner_permitted_or_capable(inode,MAY_CHMOD))return-EPERM;/* Also check the setgid bit! */if(!in_group_p((ia_valid&ATTR_GID)?attr->ia_gid:
@@ -73,7 +128,7 @@ int inode_change_ok(struct inode *inode, struct iattr *attr)/* Check for setting the inode time. */if(ia_valid&(ATTR_MTIME_SET|ATTR_ATIME_SET|ATTR_TIMES_SET)){-if(!inode_owner_or_capable(inode))+if(!inode_owner_permitted_or_capable(inode,MAY_SET_TIMES))return-EPERM;}
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:49:51
Richacls distinguish between creating non-directories and directories. To
support that, add an isdir parameter to may_create(). When checking
inode_permission() for create permission, pass in an additional
MAY_CREATE_FILE or MAY_CREATE_DIR mask flag.
Add may_replace() to allow checking for delete and create access when
replacing an existing file in vfs_rename().
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Andreas Dilger <redacted>
Reviewed-by: Steve French <redacted>
---
fs/namei.c | 49 +++++++++++++++++++++++++++++++++----------------
include/linux/fs.h | 2 ++
2 files changed, 35 insertions(+), 16 deletions(-)
@@ -2798,6 +2801,18 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)return0;}+staticintmay_delete(structinode*dir,structdentry*victim,boolisdir)+{+returnmay_delete_or_replace(dir,victim,isdir,MAY_WRITE|MAY_EXEC);+}++staticintmay_replace(structinode*dir,structdentry*victim,boolisdir)+{+intmask=isdir?MAY_CREATE_DIR:MAY_CREATE_FILE;++returnmay_delete_or_replace(dir,victim,isdir,mask|MAY_WRITE|MAY_EXEC);+}+/* Check whether we can create an object with dentry child in directory*dir.*1.Wecan'tdoitifchildalreadyexists(openhasspecialtreatmentfor
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:49:51
Hook the richacl permission checking function into the vfs.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/namei.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 52 insertions(+), 2 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:49:57
From: "Aneesh Kumar K.V" <redacted>
This feature flag selects richacl instead of POSIX ACL support on the
filesystem. When this feature is off, the "acl" and "noacl" mount options
control whether POSIX ACLs are enabled. When it is on, richacls are
automatically enabled and using the "noacl" mount option leads to an error.
Signed-off-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Andreas Dilger <redacted>
---
fs/ext4/ext4.h | 6 ++++--
fs/ext4/super.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
2 files changed, 44 insertions(+), 11 deletions(-)
@@ -1306,6 +1306,28 @@ static ext4_fsblk_t get_sb_block(void **data)returnsb_block;}+staticintenable_acl(structsuper_block*sb)+{+sb->s_flags&=~(MS_POSIXACL|MS_RICHACL);+if(test_opt(sb,ACL)){+if(EXT4_HAS_INCOMPAT_FEATURE(sb,+EXT4_FEATURE_INCOMPAT_RICHACL)){+#ifdef CONFIG_EXT4_FS_RICHACL+sb->s_flags|=MS_RICHACL;+#else+return-EOPNOTSUPP;+#endif+}else{+#ifdef CONFIG_EXT4_FS_POSIX_ACL+sb->s_flags|=MS_POSIXACL;+#else+return-EOPNOTSUPP;+#endif+}+}+return0;+}+#define DEFAULT_JOURNAL_IOPRIO (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, 3))staticchardeprecated_msg[]="Mount option \"%s\" will be removed by %s\n""Contact linux-ext4@vger.kernel.org if you think we should keep it.\n";
@@ -1502,6 +1524,13 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,#endifswitch(token){caseOpt_noacl:+#ifdef CONFIG_EXT4_FS_RICHACL+if(EXT4_HAS_INCOMPAT_FEATURE(sb,EXT4_FEATURE_INCOMPAT_RICHACL)){+ext4_msg(sb,KERN_ERR,"Mount option \"%s\" incompatible "+"with richacl feature",opt);+return-1;+}+#endifcaseOpt_nouser_xattr:ext4_msg(sb,KERN_WARNING,deprecated_msg,opt,"3.5");break;
@@ -3274,8 +3303,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)set_opt(sb,NO_UID32);/* xattr user namespace & acls are now defaulted on */set_opt(sb,XATTR_USER);-#ifdef CONFIG_EXT4_FS_POSIX_ACL-set_opt(sb,POSIX_ACL);+#if defined(CONFIG_EXT4_FS_POSIX_ACL) || defined(CONFIG_EXT4_FS_RICHACL)+set_opt(sb,ACL);#endif/* don't forget to enable journal_csum when metadata_csum is enabled. */if(ext4_has_metadata_csum(sb))
@@ -3358,8 +3387,9 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)sb->s_iflags|=SB_I_CGROUPWB;}-sb->s_flags=(sb->s_flags&~MS_POSIXACL)|-(test_opt(sb,POSIX_ACL)?MS_POSIXACL:0);+err=enable_acl(sb);+if(err)+gotofailed_mount;if(le32_to_cpu(es->s_rev_level)==EXT4_GOOD_OLD_REV&&(ext4_has_compat_features(sb)||
@@ -4673,8 +4703,9 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)if(sbi->s_mount_flags&EXT4_MF_FS_ABORTED)ext4_abort(sb,"Abort forced by user");-sb->s_flags=(sb->s_flags&~MS_POSIXACL)|-(test_opt(sb,POSIX_ACL)?MS_POSIXACL:0);+err=enable_acl(sb);+if(err)+gotorestore_opts;es=sbi->s_es;
--
2.5.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:56:19
Both XATTR_NAME_POSIX_ACL_ACCESS and XATTR_NAME_POSIX_ACL_DEFAULT have
the same XATTR_SYSTEM_PREFIX prefix; don't check for the same prefix
repeatedly.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Steve French <redacted>
---
fs/xattr.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:56:40
POSIX ACLs and richacls are both objects allocated by kmalloc() with a
reference count which are freed by kfree_rcu(). An inode can either
cache an access and a default POSIX ACL, or a richacl (richacls do not
have default acls). To allow an inode to cache either of the two kinds
of acls, introduce a new base_acl type and convert i_acl and
i_default_acl to that type. In most cases, the vfs then doesn't care which
kind of acl an inode caches (if any).
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Cc: Andreas Dilger <redacted>
---
drivers/staging/lustre/lustre/llite/llite_lib.c | 2 +-
fs/9p/acl.c | 8 +--
fs/f2fs/acl.c | 4 +-
fs/inode.c | 32 +++++++++++-
fs/jffs2/acl.c | 6 ++-
fs/namei.c | 33 ++++++------
fs/nfs/nfs3acl.c | 14 ++---
fs/posix_acl.c | 69 +++++++------------------
fs/richacl.c | 4 +-
include/linux/fs.h | 41 +++++++++++++--
include/linux/posix_acl.h | 21 ++++----
include/linux/richacl.h | 9 ++--
12 files changed, 139 insertions(+), 104 deletions(-)
@@ -16,28 +16,28 @@*cachingget_aclresultsinarace-freeway.Seefs/posix_acl.c:get_acl()*forexplanations.*/-staticvoidnfs3_prepare_get_acl(structposix_acl**p)+staticvoidnfs3_prepare_get_acl(structbase_acl**p){-structposix_acl*sentinel=uncached_acl_sentinel(current);+structbase_acl*sentinel=uncached_acl_sentinel(current);if(cmpxchg(p,ACL_NOT_CACHED,sentinel)!=ACL_NOT_CACHED){/* Not the first reader or sentinel already in place. */}}-staticvoidnfs3_complete_get_acl(structposix_acl**p,structposix_acl*acl)+staticvoidnfs3_complete_get_acl(structbase_acl**p,structposix_acl*acl){-structposix_acl*sentinel=uncached_acl_sentinel(current);+structbase_acl*sentinel=uncached_acl_sentinel(current);/* Only cache the ACL if our sentinel is still in place. */posix_acl_dup(acl);-if(cmpxchg(p,sentinel,acl)!=sentinel)+if(cmpxchg(p,sentinel,&acl->a_base)!=sentinel)posix_acl_release(acl);}-staticvoidnfs3_abort_get_acl(structposix_acl**p)+staticvoidnfs3_abort_get_acl(structbase_acl**p){-structposix_acl*sentinel=uncached_acl_sentinel(current);+structbase_acl*sentinel=uncached_acl_sentinel(current);/* Remove our sentinel upon failure. */cmpxchg(p,sentinel,ACL_NOT_CACHED);
@@ -43,10 +43,7 @@ struct posix_acl_entry {};structposix_acl{-union{-atomic_ta_refcount;-structrcu_heada_rcu;-};+structbase_acla_base;/* must be first, see posix_acl_release() */unsignedinta_count;structposix_acl_entrya_entries[0];};
@@ -31,7 +31,7 @@ struct richace {};structrichacl{-atomic_ta_refcount;+structbase_acla_base;/* must be first, see richacl_put() */unsignedinta_owner_mask;unsignedinta_group_mask;unsignedinta_other_mask;
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:56:40
These operations are similar to the get_acl and set_acl operations for
POSIX ACLs. The distinction between access and default ACLs doesn't exist
for richacls.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Steve French <redacted>
---
include/linux/fs.h | 2 ++
1 file changed, 2 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-06-30 13:57:51
A richacl roughly grants a requested access if the NFSv4 acl in the
richacl grants the requested permissions according to the NFSv4
permission check algorithm and the file mask that applies to the process
includes the requested permissions.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 129 insertions(+)
@@ -338,3 +338,131 @@ restart:acl->a_flags&=~(RICHACL_WRITE_THROUGH|RICHACL_MASKED);}EXPORT_SYMBOL_GPL(richacl_compute_max_masks);++/**+*richacl_permission-richaclpermissioncheckalgorithm+*@inode:inodetocheck+*@acl:richacloftheinode+*@want:requestedaccess(MAY_*flags)+*+*Checksifthecurrentprocessisgranted@maskflagsin@acl.+*/+int+richacl_permission(structinode*inode,conststructrichacl*acl,+intwant)+{+conststructrichace*ace;+unsignedintmask=richacl_want_to_mask(want);+unsignedintrequested=mask,denied=0;+intin_owning_group=in_group_p(inode->i_gid);+intin_owner_or_group_class=in_owning_group;++/*+*Aprocessis+*-intheownerfileclassifitownsthefile,+*-inthegroupfileclassifitisinthefile'sowninggroupor+*itmatchesanyoftheuserorgroupentries,and+*-intheotherfileclassotherwise.+*Thefileclassisonlyrelevantfordeterminingwhichfilemaskto+*apply,whichonlyhappensformaskedacls.+*/+if(acl->a_flags&RICHACL_MASKED){+if((acl->a_flags&RICHACL_WRITE_THROUGH)&&+uid_eq(current_fsuid(),inode->i_uid)){+denied=requested&~acl->a_owner_mask;+gotoout;+}+}else{+/*+*Whentheaclisnotmasked,thereisnoneedtodetermineif+*theprocessisinthegroupclassandwecanbreakout+*earlieroftheloopbelow.+*/+in_owner_or_group_class=1;+}++/*+*Checkiftheaclgrantstherequestedaccessanddeterminewhich+*fileclasstheprocessisin.+*/+richacl_for_each_entry(ace,acl){+unsignedintace_mask=ace->e_mask;++if(richace_is_inherit_only(ace))+continue;+if(richace_is_owner(ace)){+if(!uid_eq(current_fsuid(),inode->i_uid))+continue;+gotoentry_matches_owner;+}elseif(richace_is_group(ace)){+if(!in_owning_group)+continue;+}elseif(richace_is_unix_user(ace)){+if(!uid_eq(current_fsuid(),ace->e_id.uid))+continue;+if(uid_eq(current_fsuid(),inode->i_uid))+gotoentry_matches_owner;+}elseif(richace_is_unix_group(ace)){+if(!in_group_p(ace->e_id.gid))+continue;+}else+gotoentry_matches_everyone;++/*+*Applythegroupfilemasktoentriesotherthanowner@and+*everyone@oruserentriesmatchingtheowner.Thisensures+*thatwegrantthesamepermissionsastheaclcomputedby+*richacl_apply_masks().+*+*Withoutthisrestriction,thefollowingrichaclwouldgrant+*rwaccesstoprocesseswhichareboththeownerandinthe+*owninggroup,butnottootherusersintheowninggroup,+*whichcouldnotberepresentedwithoutmasks:+*+*owner:rw::mask+*group@:rw::allow+*/+if((acl->a_flags&RICHACL_MASKED)&&richace_is_allow(ace))+ace_mask&=acl->a_group_mask;++entry_matches_owner:+/* The process is in the owner or group file class. */+in_owner_or_group_class=1;++entry_matches_everyone:+/* Check which mask flags the ACE allows or denies. */+if(richace_is_deny(ace))+denied|=ace_mask&mask;+mask&=~ace_mask;++/*+*Keepgoinguntilweknowwhichfileclass+*theprocessisin.+*/+if(!mask&&in_owner_or_group_class)+break;+}+denied|=mask;++if(acl->a_flags&RICHACL_MASKED){+/*+*Thefileclassaprocessisindetermineswhichfilemask+*applies.Checkifthatfilemaskalsograntstherequested+*access.+*/+if(uid_eq(current_fsuid(),inode->i_uid))+denied|=requested&~acl->a_owner_mask;+elseif(in_owner_or_group_class)+denied|=requested&~acl->a_group_mask;+else{+if(acl->a_flags&RICHACL_WRITE_THROUGH)+denied=requested&~acl->a_other_mask;+else+denied|=requested&~acl->a_other_mask;+}+}++out:+returndenied?-EACCES:0;+}+EXPORT_SYMBOL_GPL(richacl_permission);
On Thu, Jun 30, 2016 at 03:46:51PM +0200, Andreas Gruenbacher wrote:
What more needs to be done to get this initial set of patches merged?
Bind Christoph to some tree in the Harz hills, so that he can't reject
them? :-)
Christoph, is there anything realistic that can be done to get over
your concerns?
Volker
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Jeff Layton <hidden> Date: 2016-07-05 11:00:13
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted hunk
The vfs does not apply the umask for file systems that support acls.
The test used for this used to be called IS_POSIXACL(). Switch to a new
IS_ACL() test to check for either posix acls or richacls instead. Add a
new MS_RICHACL flag and IS_RICHACL() test for richacls alone. The
IS_POSIXACL() test is still needed in some places like nfsd.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Andreas Dilger <redacted>
Reviewed-by: Steve French <redacted>
---
fs/Kconfig | 3 +++
fs/namei.c | 6 +++---
include/linux/fs.h | 12 ++++++++++++
include/uapi/linux/fs.h | 3 ++-
4 files changed, 20 insertions(+), 4 deletions(-)
* O_EXCL open we want to return EEXIST not EROFS).
*/
if (open_flag & O_CREAT) {
- if (!IS_POSIXACL(dir->d_inode))
+ if (!IS_ACL(dir->d_inode))
mode &= ~current_umask();
if (unlikely(!got_write)) {
create_error = -EROFS;
@@ -3709,7 +3709,7 @@ retry:
if (IS_ERR(dentry))
return PTR_ERR(dentry);
- if (!IS_POSIXACL(path.dentry->d_inode))
+ if (!IS_ACL(path.dentry->d_inode))
mode &= ~current_umask();
error = security_path_mknod(&path, dentry, mode, dev);
if (error)
@@ -3780,7 +3780,7 @@ retry:
if (IS_ERR(dentry))
return PTR_ERR(dentry);
- if (!IS_POSIXACL(path.dentry->d_inode))
+ if (!IS_ACL(path.dentry->d_inode))
mode &= ~current_umask();
error = security_path_mkdir(&path, dentry, mode);
if (!error)
(inode)->i_rdev == WHITEOUT_DEV)
/*
+ * IS_ACL() tells the VFS to not apply the umask
+ * and use check_acl for acl permission checks when defined.
+ */
+#define IS_ACL(inode) __IS_FLG(inode, MS_POSIXACL | MS_RICHACL)
+
+/*
* Inode state bits. Protected by inode->i_lock
*
* Three bits determine the dirty state of the inode, I_DIRTY_SYNC,
From: Jeff Layton <hidden> Date: 2016-07-05 11:07:55
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
Normally, deleting a file requires MAY_WRITE access to the parent
directory. With richacls, a file may be deleted with MAY_DELETE_CHILD access
to the parent directory or with MAY_DELETE_SELF access to the file.
To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
when checking for delete access inside a directory, and MAY_DELETE_SELF
when checking for delete access to a file itelf.
Minor misspelling in changelog above. It should be "itself".
quoted hunk
The MAY_DELETE_SELF permission overrides the sticky directory check.
Signed-off-by: Andreas Gruenbacher <redacted>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Steve French <steve.french-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
---
fs/namei.c | 20 ++++++++++++--------
include/linux/fs.h | 2 ++
2 files changed, 14 insertions(+), 8 deletions(-)
@@ -454,9 +454,9 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
* this, letting us set arbitrary permissions for filesystem access without
* changing the "normal" UIDs which are used for other things.
*
- * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE, or
- * MAY_CREATE_DIR are set. That way, file systems that don't support these
- * permissions will check for MAY_WRITE instead.
+ * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE,
+ * MAY_CREATE_DIR, or MAY_DELETE_CHILD are set. That way, file systems that
+ * don't support these permissions will check for MAY_WRITE instead.
*/
int inode_permission(struct inode *inode, int mask)
{
From: Jeff Layton <hidden> Date: 2016-07-05 11:10:34
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted hunk
Richacls distinguish between creating non-directories and directories. To
support that, add an isdir parameter to may_create(). When checking
inode_permission() for create permission, pass in an additional
MAY_CREATE_FILE or MAY_CREATE_DIR mask flag.
Add may_replace() to allow checking for delete and create access when
replacing an existing file in vfs_rename().
Signed-off-by: Andreas Gruenbacher <redacted>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Andreas Dilger <redacted>
Reviewed-by: Steve French <steve.french-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
---
fs/namei.c | 49 +++++++++++++++++++++++++++++++++----------------
include/linux/fs.h | 2 ++
2 files changed, 35 insertions(+), 16 deletions(-)
@@ -454,7 +454,9 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
* this, letting us set arbitrary permissions for filesystem access without
* changing the "normal" UIDs which are used for other things.
*
- * When checking for MAY_APPEND, MAY_WRITE must also be set in @mask.
+ * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE, or
+ * MAY_CREATE_DIR are set. That way, file systems that don't support these
+ * permissions will check for MAY_WRITE instead.
*/
int inode_permission(struct inode *inode, int mask)
{
* 3. We should have write and exec permissions on dir
* 4. We can't do it if dir is immutable (done in permission())
*/
-static inline int may_create(struct inode *dir, struct dentry *child)
+static inline int may_create(struct inode *dir, struct dentry *child, bool isdir)
{
+ int mask = isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE;
+
audit_inode_child(dir, child, AUDIT_TYPE_CHILD_CREATE);
if (child->d_inode)
return -EEXIST;
if (IS_DEADDIR(dir))
return -ENOENT;
- return inode_permission(dir, MAY_WRITE | MAY_EXEC);
+ return inode_permission(dir, MAY_WRITE | MAY_EXEC | mask);
}
/*
From: Jeff Layton <hidden> Date: 2016-07-05 11:12:14
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted hunk
We will need to call iop->permission and iop->get_acl from
inode_change_ok() for additional permission checks, and both take a
non-const inode.
Signed-off-by: Andreas Gruenbacher <redacted>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Andreas Dilger <redacted>
Reviewed-by: Steve French <steve.french-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
---
fs/attr.c | 2 +-
include/linux/fs.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
* Should be called as the first thing in ->setattr implementations,
* possibly after taking additional locks.
*/
-int inode_change_ok(const struct inode *inode, struct iattr *attr)
+int inode_change_ok(struct inode *inode, struct iattr *attr)
{
unsigned int ia_valid = attr->ia_valid;
From: Jeff Layton <hidden> Date: 2016-07-05 11:18:45
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted hunk
Richacls support permissions that allow to take ownership of a file,
change the file permissions, and set the file timestamps. Support that
by introducing new permission mask flags and by checking for those mask
flags in inode_change_ok().
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Steve French <redacted>
---
fs/attr.c | 79 +++++++++++++++++++++++++++++++++++++++++++++---------
include/linux/fs.h | 3 +++
2 files changed, 70 insertions(+), 12 deletions(-)
#define MAY_CREATE_DIR 0x00000200
#define MAY_DELETE_CHILD 0x00000400
#define MAY_DELETE_SELF 0x00000800
+#define MAY_TAKE_OWNERSHIP 0x00001000
+#define MAY_CHMOD 0x00002000
+#define MAY_SET_TIMES 0x00004000
/*
* flags in file.f_mode. Note that FMODE_READ and FMODE_WRITE must correspond
Reviewed-by: Jeff Layton <redacted>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Jeff Layton <hidden> Date: 2016-07-05 11:34:12
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted hunk
A richacl consists of an NFSv4 acl and an owner, group, and other mask.
These three masks correspond to the owner, group, and other file
permission bits, but they contain NFSv4 permissions instead of POSIX
permissions.
Each entry in the NFSv4 acl applies to the file owner (OWNER@), the
owning group (GROUP@), everyone (EVERYONE@), or to a specific uid or
gid.
As in the standard POSIX file permission model, each process is the
owner, group, or other file class. A richacl grants a requested access
only if the NFSv4 acl in the richacl grants the access (according to the
NFSv4 permission check algorithm), and the file mask that applies to the
process includes the requested permissions.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/Makefile | 1 +
fs/richacl.c | 65 ++++++++++++++++
include/linux/richacl.h | 179 +++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/richacl.h | 99 ++++++++++++++++++++++++
5 files changed, 345 insertions(+)
create mode 100644 fs/richacl.c
create mode 100644 include/linux/richacl.h
create mode 100644 include/uapi/linux/richacl.h
I imagine we could have a lot of these at any given time. It might be
nice to consider how to do this with dedicated slabcaches for better
packing, but I think that would add to the complexity, unfortunately.
Reviewed-by: Jeff Layton <redacted>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jeff Layton <hidden> Date: 2016-07-05 13:39:50
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted hunk
We need to map from POSIX permissions to NFSv4 permissions when a
chmod() is done, from NFSv4 permissions to POSIX permissions when an acl
is set (which implicitly sets the file permission bits), and from the
MAY_READ/MAY_WRITE/MAY_EXEC/MAY_APPEND flags to NFSv4 permissions when
doing an access check in a richacl.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 118 +++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 3 ++
include/uapi/linux/richacl.h | 44 ++++++++++++++++
3 files changed, 165 insertions(+)
{
memcpy(to, from, sizeof(struct richace));
}
+
+/*
+ * richacl_mask_to_mode - compute the file permission bits from mask
+ * @mask: %RICHACE_* permission mask
+ *
+ * Compute the file permission bits corresponding to a particular set of
+ * richacl permissions.
+ *
+ * See richacl_masks_to_mode().
+ */
+static int
+richacl_mask_to_mode(unsigned int mask)
+{
+ int mode = 0;
+
+ if (mask & RICHACE_POSIX_MODE_READ)
+ mode |= S_IROTH;
+ if (mask & RICHACE_POSIX_MODE_WRITE)
+ mode |= S_IWOTH;
+ if (mask & RICHACE_POSIX_MODE_EXEC)
+ mode |= S_IXOTH;
+
+ return mode;
+}
+
+/**
+ * richacl_masks_to_mode - compute file permission bits from file masks
+ *
+ * When setting a richacl, we set the file permission bits to indicate maximum
+ * permissions: for example, we set the Write permission when a mask contains
+ * RICHACE_APPEND_DATA even if it does not also contain RICHACE_WRITE_DATA.
+ *
+ * Permissions which are not in RICHACE_POSIX_MODE_READ,
+ * RICHACE_POSIX_MODE_WRITE, or RICHACE_POSIX_MODE_EXEC cannot be represented
+ * in the file permission bits. Such permissions can still be effective, but
+ * not for new files or after a chmod(); they must be explicitly enabled in the
+ * richacl.
+ */
+int
+richacl_masks_to_mode(const struct richacl *acl)
+{
+ return richacl_mask_to_mode(acl->a_owner_mask) << 6 |
+ richacl_mask_to_mode(acl->a_group_mask) << 3 |
+ richacl_mask_to_mode(acl->a_other_mask);
+}
+EXPORT_SYMBOL_GPL(richacl_masks_to_mode);
+
+/**
+ * richacl_mode_to_mask - compute a file mask from the lowest three mode bits
+ * @mode: mode to convert to richacl permissions
+ *
+ * When the file permission bits of a file are set with chmod(), this specifies
+ * the maximum permissions that processes will get. All permissions beyond
+ * that will be removed from the file masks, and become ineffective.
+ */
+unsigned int
+richacl_mode_to_mask(umode_t mode)
+{
+ unsigned int mask = 0;
+
+ if (mode & S_IROTH)
+ mask |= RICHACE_POSIX_MODE_READ;
+ if (mode & S_IWOTH)
+ mask |= RICHACE_POSIX_MODE_WRITE;
+ if (mode & S_IXOTH)
+ mask |= RICHACE_POSIX_MODE_EXEC;
+
+ return mask;
+}
+
+/**
+ * richacl_want_to_mask - convert the iop->permission want argument to a mask
+ * @want: @want argument of the permission inode operation
+ *
+ * When checking for append, @want is (MAY_WRITE | MAY_APPEND).
+ *
+ * Richacls use the iop->may_create and iop->may_delete hooks which are used
+ * for checking if creating and deleting files is allowed. These hooks do not
+ * use richacl_want_to_mask(), so we do not have to deal with mapping MAY_WRITE
+ * to RICHACE_ADD_FILE, RICHACE_ADD_SUBDIRECTORY, and RICHACE_DELETE_CHILD
+ * here.
+ */
This comment is confusing as I don't see any may_create or may_delete
iops in the final patchset. Do you mean may_create() and may_delete()
here?
quoted hunk
+unsigned int
+richacl_want_to_mask(unsigned int want)
+{
+ unsigned int mask = 0;
+
+ if (want & MAY_READ)
+ mask |= RICHACE_READ_DATA;
+ if (want & MAY_DELETE_SELF)
+ mask |= RICHACE_DELETE;
+ if (want & MAY_TAKE_OWNERSHIP)
+ mask |= RICHACE_WRITE_OWNER;
+ if (want & MAY_CHMOD)
+ mask |= RICHACE_WRITE_ACL;
+ if (want & MAY_SET_TIMES)
+ mask |= RICHACE_WRITE_ATTRIBUTES;
+ if (want & MAY_EXEC)
+ mask |= RICHACE_EXECUTE;
+ /*
+ * differentiate MAY_WRITE from these request
+ */
+ if (want & (MAY_APPEND |
+ MAY_CREATE_FILE | MAY_CREATE_DIR |
+ MAY_DELETE_CHILD)) {
+ if (want & MAY_APPEND)
+ mask |= RICHACE_APPEND_DATA;
+ if (want & MAY_CREATE_FILE)
+ mask |= RICHACE_ADD_FILE;
+ if (want & MAY_CREATE_DIR)
+ mask |= RICHACE_ADD_SUBDIRECTORY;
+ if (want & MAY_DELETE_CHILD)
+ mask |= RICHACE_DELETE_CHILD;
+ } else if (want & MAY_WRITE)
+ mask |= RICHACE_WRITE_DATA;
+ return mask;
+}
+EXPORT_SYMBOL_GPL(richacl_want_to_mask);
RICHACE_WRITE_OWNER | \
RICHACE_SYNCHRONIZE )
+/*
+ * The POSIX permissions are supersets of the following richacl permissions:
+ *
+ * - MAY_READ maps to READ_DATA or LIST_DIRECTORY, depending on the type
+ * of the file system object.
+ *
+ * - MAY_WRITE maps to WRITE_DATA or RICHACE_APPEND_DATA for files, and to
+ * ADD_FILE, RICHACE_ADD_SUBDIRECTORY, or RICHACE_DELETE_CHILD for directories.
+ *
+ * - MAY_EXECUTE maps to RICHACE_EXECUTE.
+ *
+ * (Some of these richacl permissions have the same bit values.)
+ */
+#define RICHACE_POSIX_MODE_READ ( \
+ RICHACE_READ_DATA | \
+ RICHACE_LIST_DIRECTORY)
+#define RICHACE_POSIX_MODE_WRITE ( \
+ RICHACE_WRITE_DATA | \
+ RICHACE_ADD_FILE | \
+ RICHACE_APPEND_DATA | \
+ RICHACE_ADD_SUBDIRECTORY | \
+ RICHACE_DELETE_CHILD)
+#define RICHACE_POSIX_MODE_EXEC RICHACE_EXECUTE
+#define RICHACE_POSIX_MODE_ALL ( \
+ RICHACE_POSIX_MODE_READ | \
+ RICHACE_POSIX_MODE_WRITE | \
+ RICHACE_POSIX_MODE_EXEC)
+
+/*
+ * These permissions are always allowed no matter what the acl says.
+ */
+#define RICHACE_POSIX_ALWAYS_ALLOWED ( \
+ RICHACE_SYNCHRONIZE | \
+ RICHACE_READ_ATTRIBUTES | \
+ RICHACE_READ_ACL)
+
+/*
+ * The owner is implicitly granted these permissions under POSIX.
+ */
+#define RICHACE_POSIX_OWNER_ALLOWED ( \
+ RICHACE_WRITE_ATTRIBUTES | \
+ RICHACE_WRITE_OWNER | \
+ RICHACE_WRITE_ACL)
+
#endif /* __UAPI_RICHACL_H */
Other than the confusing comment, this looks ok.
Reviewed-by: Jeff Layton <redacted>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jeff Layton <hidden> Date: 2016-07-05 14:22:21
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted hunk
Compute upper bound owner, group, and other file masks with as few
permissions as possible without denying any permissions that the NFSv4
acl in a richacl grants.
This algorithm is used when a file inherits an acl at create time and
when an acl is set via a mechanism that does not provide file masks
(such as setting an acl via nfsd). When user-space sets an acl via
setxattr, the extended attribute already includes the file masks.
Setting an acl also sets the file mode permission bits: they are
determined by the file masks; see richacl_masks_to_mode().
Signed-off-by: Andreas Gruenbacher <redacted>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 158 insertions(+)
@@ -181,3 +181,160 @@ richacl_want_to_mask(unsigned int want)
return mask;
}
EXPORT_SYMBOL_GPL(richacl_want_to_mask);
+
+/*
+ * Note: functions like richacl_allowed_to_who(), richacl_group_class_allowed(),
+ * and richacl_compute_max_masks() iterate through the entire acl in reverse
+ * order as an optimization.
+ *
+ * In the standard algorithm, aces are considered in forward order. When a
+ * process matches an ace, the permissions in the ace are either allowed or
+ * denied depending on the ace type. Once a permission has been allowed or
+ * denied, it is no longer considered in further aces.
+ *
+ * By iterating through the acl in reverse order, we can compute the same
+ * result without having to keep track of which permissions have been allowed
+ * and denied already.
+ */
Clever!
quoted hunk
+
+/**
+ * richacl_allowed_to_who - permissions allowed to a specific who value
+ *
+ * Compute the maximum mask values allowed to a specific who value, taking
+ * everyone@ aces into account.
+ */
+static unsigned int richacl_allowed_to_who(struct richacl *acl,
+ struct richace *who)
+{
+ struct richace *ace;
+ unsigned int allowed = 0;
+
+ richacl_for_each_entry_reverse(ace, acl) {
+ if (richace_is_inherit_only(ace))
+ continue;
+ if (richace_is_same_identifier(ace, who) ||
+ richace_is_everyone(ace)) {
+ if (richace_is_allow(ace))
+ allowed |= ace->e_mask;
+ else if (richace_is_deny(ace))
+ allowed &= ~ace->e_mask;
+ }
+ }
+ return allowed;
+}
+
+/**
+ * richacl_group_class_allowed - maximum permissions of the group class
+ *
+ * Compute the maximum mask values allowed to a process in the group class
+ * (i.e., a process which is not the owner but is in the owning group or
+ * matches a user or group acl entry). This includes permissions granted or
+ * denied by everyone@ aces.
+ *
+ * See richacl_compute_max_masks().
+ */
+static unsigned int richacl_group_class_allowed(struct richacl *acl)
+{
+ struct richace *ace;
+ unsigned int everyone_allowed = 0, group_class_allowed = 0;
+ int had_group_ace = 0;
+
+ richacl_for_each_entry_reverse(ace, acl) {
+ if (richace_is_inherit_only(ace) ||
+ richace_is_owner(ace))
+ continue;
+
+ if (richace_is_everyone(ace)) {
+ if (richace_is_allow(ace))
+ everyone_allowed |= ace->e_mask;
+ else if (richace_is_deny(ace))
+ everyone_allowed &= ~ace->e_mask;
+ } else {
+ group_class_allowed |=
+ richacl_allowed_to_who(acl, ace);
+
+ if (richace_is_group(ace))
+ had_group_ace = 1;
+ }
+ }
+ /*
+ * If the acl doesn't contain any group@ aces, richacl_allowed_to_who()
+ * wasn't called for the owning group. We could make that call now, but
+ * we already know the result (everyone_allowed).
+ */
+ if (!had_group_ace)
+ group_class_allowed |= everyone_allowed;
+ return group_class_allowed;
+}
+
+/**
+ * richacl_compute_max_masks - compute upper bound masks
+ *
+ * Computes upper bound owner, group, and other masks so that none of the
+ * permissions allowed by the acl are disabled.
+ *
+ * We don't make assumptions about who the owner is so that the owner can
+ * change with no effect on the file masks or file mode permission bits; this
+ * means that we must assume that all entries can match the owner.
+ */
+void richacl_compute_max_masks(struct richacl *acl)
+{
+ unsigned int gmask = ~0;
+ struct richace *ace;
+
+ /*
+ * @gmask contains all permissions which the group class is ever
+ * allowed. We use it to avoid adding permissions to the group mask
+ * from everyone@ allow aces which the group class is always denied
+ * through other aces. For example, the following acl would otherwise
+ * result in a group mask of rw:
+ *
+ * group@:w::deny
+ * everyone@:rw::allow
+ *
+ * Avoid computing @gmask for acls which do not include any group class
+ * deny aces: in such acls, the group class is never denied any
+ * permissions from everyone@ allow aces, and the group class cannot
+ * have fewer permissions than the other class.
+ */
+
+restart:
+ acl->a_owner_mask = 0;
+ acl->a_group_mask = 0;
+ acl->a_other_mask = 0;
+
+ richacl_for_each_entry_reverse(ace, acl) {
+ if (richace_is_inherit_only(ace))
+ continue;
+
+ if (richace_is_owner(ace)) {
+ if (richace_is_allow(ace))
+ acl->a_owner_mask |= ace->e_mask;
+ else if (richace_is_deny(ace))
+ acl->a_owner_mask &= ~ace->e_mask;
+ } else if (richace_is_everyone(ace)) {
+ if (richace_is_allow(ace)) {
+ acl->a_owner_mask |= ace->e_mask;
+ acl->a_group_mask |= ace->e_mask & gmask;
+ acl->a_other_mask |= ace->e_mask;
+ } else if (richace_is_deny(ace)) {
+ acl->a_owner_mask &= ~ace->e_mask;
+ acl->a_group_mask &= ~ace->e_mask;
+ acl->a_other_mask &= ~ace->e_mask;
+ }
+ } else {
+ if (richace_is_allow(ace)) {
+ acl->a_owner_mask |= ace->e_mask & gmask;
+ acl->a_group_mask |= ace->e_mask & gmask;
+ } else if (richace_is_deny(ace) && gmask == ~0) {
+ gmask = richacl_group_class_allowed(acl);
+ if (likely(gmask != ~0))
+ /* should always be true */
+ goto restart;
+ }
+ }
+ }
+
+ acl->a_flags &= ~(RICHACL_WRITE_THROUGH | RICHACL_MASKED);
+}
+EXPORT_SYMBOL_GPL(richacl_compute_max_masks);
extern int richacl_masks_to_mode(const struct richacl *);
extern unsigned int richacl_mode_to_mask(umode_t);
extern unsigned int richacl_want_to_mask(unsigned int);
+extern void richacl_compute_max_masks(struct richacl *);
#endif /* __RICHACL_H */
Reviewed-by: Jeff Layton <redacted>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jeff Layton <hidden> Date: 2016-07-05 14:59:10
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
A richacl roughly grants a requested access if the NFSv4 acl in the
richacl grants the requested permissions according to the NFSv4
permission check algorithm and the file mask that applies to the process
includes the requested permissions.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 129 insertions(+)
acl->a_flags &= ~(RICHACL_WRITE_THROUGH | RICHACL_MASKED);
}
EXPORT_SYMBOL_GPL(richacl_compute_max_masks);
+
+/**
+ * richacl_permission - richacl permission check algorithm
+ * @inode: inode to check
+ * @acl: rich acl of the inode
+ * @want: requested access (MAY_* flags)
+ *
+ * Checks if the current process is granted @mask flags in @acl.
+ */
nit: there is no @mask parm here. Do you mean @want ?
quoted hunk
+int
+richacl_permission(struct inode *inode, const struct richacl *acl,
+ int want)
+{
+ const struct richace *ace;
+ unsigned int mask = richacl_want_to_mask(want);
+ unsigned int requested = mask, denied = 0;
+ int in_owning_group = in_group_p(inode->i_gid);
+ int in_owner_or_group_class = in_owning_group;
+
+ /*
+ * A process is
+ * - in the owner file class if it owns the file,
+ * - in the group file class if it is in the file's owning group or
+ * it matches any of the user or group entries, and
+ * - in the other file class otherwise.
+ * The file class is only relevant for determining which file mask to
+ * apply, which only happens for masked acls.
+ */
+ if (acl->a_flags & RICHACL_MASKED) {
+ if ((acl->a_flags & RICHACL_WRITE_THROUGH) &&
+ uid_eq(current_fsuid(), inode->i_uid)) {
+ denied = requested & ~acl->a_owner_mask;
+ goto out;
+ }
+ } else {
+ /*
+ * When the acl is not masked, there is no need to determine if
+ * the process is in the group class and we can break out
+ * earlier of the loop below.
+ */
+ in_owner_or_group_class = 1;
+ }
+
+ /*
+ * Check if the acl grants the requested access and determine which
+ * file class the process is in.
+ */
+ richacl_for_each_entry(ace, acl) {
+ unsigned int ace_mask = ace->e_mask;
+
+ if (richace_is_inherit_only(ace))
+ continue;
+ if (richace_is_owner(ace)) {
+ if (!uid_eq(current_fsuid(), inode->i_uid))
+ continue;
+ goto entry_matches_owner;
+ } else if (richace_is_group(ace)) {
+ if (!in_owning_group)
+ continue;
+ } else if (richace_is_unix_user(ace)) {
+ if (!uid_eq(current_fsuid(), ace->e_id.uid))
+ continue;
+ if (uid_eq(current_fsuid(), inode->i_uid))
+ goto entry_matches_owner;
+ } else if (richace_is_unix_group(ace)) {
+ if (!in_group_p(ace->e_id.gid))
+ continue;
+ } else
+ goto entry_matches_everyone;
+
+ /*
+ * Apply the group file mask to entries other than owner@ and
+ * everyone@ or user entries matching the owner. This ensures
+ * that we grant the same permissions as the acl computed by
+ * richacl_apply_masks().
+ *
+ * Without this restriction, the following richacl would grant
+ * rw access to processes which are both the owner and in the
+ * owning group, but not to other users in the owning group,
+ * which could not be represented without masks:
+ *
+ * owner:rw::mask
+ * group@:rw::allow
+ */
+ if ((acl->a_flags & RICHACL_MASKED) && richace_is_allow(ace))
+ ace_mask &= acl->a_group_mask;
+
+entry_matches_owner:
+ /* The process is in the owner or group file class. */
+ in_owner_or_group_class = 1;
+
+entry_matches_everyone:
+ /* Check which mask flags the ACE allows or denies. */
+ if (richace_is_deny(ace))
+ denied |= ace_mask & mask;
+ mask &= ~ace_mask;
+
+ /*
+ * Keep going until we know which file class
+ * the process is in.
+ */
+ if (!mask && in_owner_or_group_class)
+ break;
+ }
+ denied |= mask;
+
+ if (acl->a_flags & RICHACL_MASKED) {
+ /*
+ * The file class a process is in determines which file mask
+ * applies. Check if that file mask also grants the requested
+ * access.
+ */
+ if (uid_eq(current_fsuid(), inode->i_uid))
+ denied |= requested & ~acl->a_owner_mask;
+ else if (in_owner_or_group_class)
+ denied |= requested & ~acl->a_group_mask;
+ else {
+ if (acl->a_flags & RICHACL_WRITE_THROUGH)
+ denied = requested & ~acl->a_other_mask;
+ else
+ denied |= requested & ~acl->a_other_mask;
+ }
+ }
+
+out:
+ return denied ? -EACCES : 0;
+}
+EXPORT_SYMBOL_GPL(richacl_permission);
@@ -179,5 +179,6 @@ extern int richacl_masks_to_mode(const struct richacl *);
extern unsigned int richacl_mode_to_mask(umode_t);
extern unsigned int richacl_want_to_mask(unsigned int);
extern void richacl_compute_max_masks(struct richacl *);
+extern int richacl_permission(struct inode *, const struct richacl *, int);
#endif /* __RICHACL_H */
Reviewed-by: Jeff Layton <redacted>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Jeff Layton <hidden> Date: 2016-07-05 15:38:53
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
Both XATTR_NAME_POSIX_ACL_ACCESS and XATTR_NAME_POSIX_ACL_DEFAULT have
the same XATTR_SYSTEM_PREFIX prefix; don't check for the same prefix
repeatedly.
Signed-off-by: Andreas Gruenbacher <redacted>
Reviewed-by: Steve French <steve.french-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
---
fs/xattr.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
Might be nice to move this one to the head of the queue. I don't see it
as being a controversial change, and it could go in before the rest of
the patches.
Reviewed-by: Jeff Layton <redacted>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jeff Layton <hidden> Date: 2016-07-05 15:56:08
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
POSIX ACLs and richacls are both objects allocated by kmalloc() with a
reference count which are freed by kfree_rcu(). An inode can either
cache an access and a default POSIX ACL, or a richacl (richacls do not
have default acls). To allow an inode to cache either of the two kinds
of acls, introduce a new base_acl type and convert i_acl and
i_default_acl to that type. In most cases, the vfs then doesn't care which
kind of acl an inode caches (if any).
Signed-off-by: Andreas Gruenbacher <redacted>
Cc: Andreas Dilger <redacted>
---
drivers/staging/lustre/lustre/llite/llite_lib.c | 2 +-
fs/9p/acl.c | 8 +--
fs/f2fs/acl.c | 4 +-
fs/inode.c | 32 +++++++++++-
fs/jffs2/acl.c | 6 ++-
fs/namei.c | 33 ++++++------
fs/nfs/nfs3acl.c | 14 ++---
fs/posix_acl.c | 69 +++++++------------------
fs/richacl.c | 4 +-
include/linux/fs.h | 41 +++++++++++++--
include/linux/posix_acl.h | 21 ++++----
include/linux/richacl.h | 9 ++--
12 files changed, 139 insertions(+), 104 deletions(-)
I know this is basically copied from the existing get_cached_acl
function, but I'm a little uneasy with the above (and also with the
existing code that does the same thing).
The ba_refcount and ba_rcu are unioned. Once you've done the final
base_acl_put on the object, the you'll end up calling kfree_rcu which
is going to clobber the ba_refcount. How is it then safe to rely on it
still being zero for the atomic_inc_not_zero? ISTM that it would be
safer to make those separate fields and not union them.
...or am I missing something that prevents that?
struct posix_acl *get_acl(struct inode *inode, int type)
{
- void *sentinel;
- struct posix_acl **p;
+ struct base_acl **p = acl_by_type(inode, type);
+ struct base_acl *sentinel, *base_acl;
struct posix_acl *acl;
+ if (!IS_POSIXACL(inode))
+ return NULL;
+
/*
* The sentinel is used to detect when another operation like
* set_cached_acl() or forget_cached_acl() races with get_acl().
* It is guaranteed that is_uncached_acl(sentinel) is true.
*/
- acl = get_cached_acl(inode, type);
- if (!is_uncached_acl(acl))
- return acl;
-
- if (!IS_POSIXACL(inode))
- return NULL;
+ base_acl = __get_cached_acl(p);
+ if (!is_uncached_acl(base_acl))
+ return posix_acl(base_acl);
sentinel = uncached_acl_sentinel(current);
- p = acl_by_type(inode, type);
/*
* If the ACL isn't being read yet, set our sentinel. Otherwise, the
* Cache the result, but only if our sentinel is still in place.
*/
posix_acl_dup(acl);
- if (unlikely(cmpxchg(p, sentinel, acl) != sentinel))
+ if (unlikely(cmpxchg(p, sentinel, &acl->a_base) != sentinel))
posix_acl_release(acl);
return acl;
}
};
struct richacl {
- atomic_t a_refcount;
+ struct base_acl a_base; /* must be first, see richacl_put() */
unsigned int a_owner_mask;
unsigned int a_group_mask;
unsigned int a_other_mask;
From: Frank Filz <hidden> Date: 2016-07-05 17:09:32
quoted
+ * Note: functions like richacl_allowed_to_who(),
+richacl_group_class_allowed(),
+ * and richacl_compute_max_masks() iterate through the entire acl in
+reverse
+ * order as an optimization.
+ *
+ * In the standard algorithm, aces are considered in forward order.
+When a
+ * process matches an ace, the permissions in the ace are either
+allowed or
+ * denied depending on the ace type. Once a permission has been
+allowed or
+ * denied, it is no longer considered in further aces.
+ *
+ * By iterating through the acl in reverse order, we can compute the
+same
+ * result without having to keep track of which permissions have been
+allowed
+ * and denied already.
+ */
Clever!
Hmm, but does that result in examining the whole ACL for most access checks, at least for files where most of the accesses are by the owner, or a member of a specific group (with perhaps a ton of special case users added on the end)?
Frank
quoted
+
+/**
+ * richacl_allowed_to_who - permissions allowed to a specific who
+value
+ *
+ * Compute the maximum mask values allowed to a specific who value,
+taking
+ * everyone@ aces into account.
+ */
+static unsigned int richacl_allowed_to_who(struct richacl *acl,
+ struct richace *who)
+{
+ struct richace *ace;
+ unsigned int allowed = 0;
+
+ richacl_for_each_entry_reverse(ace, acl) {
+ if (richace_is_inherit_only(ace))
+ continue;
+ if (richace_is_same_identifier(ace, who) ||
+ richace_is_everyone(ace)) {
+ if (richace_is_allow(ace))
+ allowed |= ace->e_mask;
+ else if (richace_is_deny(ace))
+ allowed &= ~ace->e_mask;
+ }
+ }
+ return allowed;
+}
+
+/**
+ * richacl_group_class_allowed - maximum permissions of the group
+class
+ *
+ * Compute the maximum mask values allowed to a process in the group
+class
+ * (i.e., a process which is not the owner but is in the owning group
+or
+ * matches a user or group acl entry). This includes permissions
+granted or
+ * denied by everyone@ aces.
+ *
+ * See richacl_compute_max_masks().
+ */
+static unsigned int richacl_group_class_allowed(struct richacl *acl)
+{
+ struct richace *ace;
+ unsigned int everyone_allowed = 0, group_class_allowed = 0;
+ int had_group_ace = 0;
+
+ richacl_for_each_entry_reverse(ace, acl) {
+ if (richace_is_inherit_only(ace) ||
+ richace_is_owner(ace))
+ continue;
+
+ if (richace_is_everyone(ace)) {
+ if (richace_is_allow(ace))
+ everyone_allowed |= ace->e_mask;
+ else if (richace_is_deny(ace))
+ everyone_allowed &= ~ace->e_mask;
+ } else {
+ group_class_allowed |=
+ richacl_allowed_to_who(acl, ace);
+
+ if (richace_is_group(ace))
+ had_group_ace = 1;
+ }
+ }
+ /*
+ * If the acl doesn't contain any group@ aces,
richacl_allowed_to_who()
quoted
+ * wasn't called for the owning group. We could make that call now,
but
quoted
+ * we already know the result (everyone_allowed).
+ */
+ if (!had_group_ace)
+ group_class_allowed |= everyone_allowed;
+ return group_class_allowed;
+}
+
+/**
+ * richacl_compute_max_masks - compute upper bound masks
+ *
+ * Computes upper bound owner, group, and other masks so that none of
+the
+ * permissions allowed by the acl are disabled.
+ *
+ * We don't make assumptions about who the owner is so that the owner
+can
+ * change with no effect on the file masks or file mode permission
+bits; this
+ * means that we must assume that all entries can match the owner.
+ */
+void richacl_compute_max_masks(struct richacl *acl) {
+ unsigned int gmask = ~0;
+ struct richace *ace;
+
+ /*
+ * @gmask contains all permissions which the group class is ever
+ * allowed. We use it to avoid adding permissions to the group mask
+ * from everyone@ allow aces which the group class is always denied
+ * through other aces. For example, the following acl would
otherwise
quoted
+ * result in a group mask of rw:
+ *
+ * group@:w::deny
+ * everyone@:rw::allow
+ *
+ * Avoid computing @gmask for acls which do not include any group
class
quoted
+ * deny aces: in such acls, the group class is never denied any
+ * permissions from everyone@ allow aces, and the group class
cannot
quoted
+ * have fewer permissions than the other class.
+ */
+
+restart:
+ acl->a_owner_mask = 0;
+ acl->a_group_mask = 0;
+ acl->a_other_mask = 0;
+
+ richacl_for_each_entry_reverse(ace, acl) {
+ if (richace_is_inherit_only(ace))
+ continue;
+
+ if (richace_is_owner(ace)) {
+ if (richace_is_allow(ace))
+ acl->a_owner_mask |= ace->e_mask;
+ else if (richace_is_deny(ace))
+ acl->a_owner_mask &= ~ace->e_mask;
+ } else if (richace_is_everyone(ace)) {
+ if (richace_is_allow(ace)) {
+ acl->a_owner_mask |= ace->e_mask;
+ acl->a_group_mask |= ace->e_mask &
struct richace *);
extern int richacl_masks_to_mode(const struct richacl *);
extern unsigned int richacl_mode_to_mask(umode_t);
extern unsigned int richacl_want_to_mask(unsigned int);
+extern void richacl_compute_max_masks(struct richacl *);
#endif /* __RICHACL_H */
Reviewed-by: Jeff Layton <redacted>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the
body of a message to majordomo@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html
From: Jeff Layton <hidden> Date: 2016-07-06 18:31:17
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
These operations are similar to the get_acl and set_acl operations for
POSIX ACLs. The distinction between access and default ACLs doesn't exist
for richacls.
Signed-off-by: Andreas Gruenbacher <redacted>
Reviewed-by: Steve French <steve.french-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
---
include/linux/fs.h | 2 ++
1 file changed, 2 insertions(+)
umode_t create_mode, int *opened);
int (*tmpfile) (struct inode *, struct dentry *, umode_t);
int (*set_acl)(struct inode *, struct posix_acl *, int);
+ int (*set_richacl)(struct inode *, struct richacl *);
} ____cacheline_aligned;
ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
Looks OK, though it's hard to review something like this without some
context. Maybe this patch should be folded in with the patch that
actually adds the calls for these operations?
Reviewed-by: Jeff Layton <redacted>
From: Jeff Layton <hidden> Date: 2016-07-06 18:57:05
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
Cache richacls in struct inode so that this doesn't have to be done
individually in each filesystem. This is similar to POSIX ACLs.
Signed-off-by: Andreas Gruenbacher <redacted>
---
fs/inode.c | 13 +++++---
fs/richacl.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 5 ++-
include/linux/richacl.h | 11 +++++++
4 files changed, 105 insertions(+), 5 deletions(-)
#include
#include
+void set_cached_richacl(struct inode *inode, struct richacl *acl)
+{
+ struct base_acl *old;
+
+ old = xchg(&inode->i_acl, &richacl_get(acl)->a_base);
+ if (!is_uncached_acl(old))
+ base_acl_put(old);
+}
+EXPORT_SYMBOL_GPL(set_cached_richacl);
+
+void forget_cached_richacl(struct inode *inode)
+{
+ __forget_cached_acl(&inode->i_acl);
+}
+EXPORT_SYMBOL_GPL(forget_cached_richacl);
+
+struct richacl *get_richacl(struct inode *inode)
+{
+ struct base_acl *sentinel, *base_acl;
+ struct richacl *acl;
+
+ if (!IS_RICHACL(inode))
+ return NULL;
+
+ /*
+ * The sentinel is used to detect when another operation like
+ * set_cached_richacl() or forget_cached_richacl() races with
+ * get_richacl().
+ * It is guaranteed that is_uncached_acl(sentinel) is true.
+ */
+
+ base_acl = __get_cached_acl(&inode->i_acl);
+ if (!is_uncached_acl(base_acl))
+ return richacl(base_acl);
+
+ sentinel = uncached_acl_sentinel(current);
+
+ /*
+ * If the ACL isn't being read yet, set our sentinel. Otherwise, the
+ * current value of the ACL will not be ACL_NOT_CACHED and so our own
+ * sentinel will not be set; another task will update the cache. We
+ * could wait for that other task to complete its job, but it's easier
+ * to just call ->get_acl to fetch the ACL ourself. (This is going to
+ * be an unlikely race.)
+ */
+ if (cmpxchg(&inode->i_acl, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED)
+ /* fall through */ ;
+
So you do the same thing regardless of the outcome of the above? Why
bother with the if at all here? Just do the cmpxchg and toss out the
result.
+ /*
+ * Normally, the ACL returned by ->get_richacl will be cached.
+ * A filesystem can prevent that by calling
+ * forget_cached_richacl(inode) in ->get_richacl.
+ *
+ * If the filesystem doesn't have a ->get_richacl function at all,
+ * we'll just create the negative cache entry.
+ */
+ if (!inode->i_op->get_richacl) {
+ set_cached_richacl(inode, NULL);
+ return NULL;
+ }
+
+ acl = inode->i_op->get_richacl(inode);
+ if (IS_ERR(acl)) {
+ /*
+ * Remove our sentinel so that we don't block future attempts
+ * to cache the ACL.
+ */
+ cmpxchg(&inode->i_acl, sentinel, ACL_NOT_CACHED);
+ return acl;
+ }
+
So this is sort of icky: only the task that sets the sentinel can set
the cached acl.
You could have one task set the sentinel, call get_richacl and fail, but then a concurrent task also issues a get_richacl and succeeds. That task will get its acl, but it doesn't end up getting cached and subsequent callers then have to reissue the request. It would be good if the first successful fetch of the acl sets it in the cache.
That said, getting the acl could be pretty expensive with some filesystems. NFS or CIFS are going to have to do an on the wire call to fetch them, for instance. I think it would be better to have concurrent callers wait for the first caller's result instead of issuing parallel get_richacl requests.
quoted hunk
+ /*
+ * Cache the result, but only if our sentinel is still in place.
+ */
+ richacl_get(acl);
+ if (unlikely(cmpxchg(&inode->i_acl, sentinel, &acl->a_base) != sentinel))
+ richacl_put(acl);
+ return acl;
+}
+EXPORT_SYMBOL_GPL(get_richacl);
+
/**
* richacl_alloc - allocate a richacl
* @count: number of entries
--
Jeff Layton [off-list ref]
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Howells <dhowells@redhat.com> Date: 2016-07-07 14:14:13
Jeff Layton [off-list ref] wrote:
quoted
+ if (cmpxchg(&inode->i_acl, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED)
+ /* fall through */ ;
+
So you do the same thing regardless of the outcome of the above? Why
bother with the if at all here? Just do the cmpxchg and toss out the
result.
gcc might complain if you don't check the result.
However, this does look like it's subject to a thundering herd problem. If
30000 processes all look at the ACL at the same time on a network fs, could
that cause 30000 RPC calls to be transmitted for the same thing?
David
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-07-11 10:11:56
On Tue, Jul 5, 2016 at 1:34 PM, Jeff Layton [off-list ref] wrote:
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted
A richacl consists of an NFSv4 acl and an owner, group, and other mask.
These three masks correspond to the owner, group, and other file
permission bits, but they contain NFSv4 permissions instead of POSIX
permissions.
Each entry in the NFSv4 acl applies to the file owner (OWNER@), the
owning group (GROUP@), everyone (EVERYONE@), or to a specific uid or
gid.
As in the standard POSIX file permission model, each process is the
owner, group, or other file class. A richacl grants a requested access
only if the NFSv4 acl in the richacl grants the access (according to the
NFSv4 permission check algorithm), and the file mask that applies to the
process includes the requested permissions.
Signed-off-by: Andreas Gruenbacher <redacted>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/Makefile | 1 +
fs/richacl.c | 65 ++++++++++++++++
include/linux/richacl.h | 179 +++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/richacl.h | 99 ++++++++++++++++++++++++
5 files changed, 345 insertions(+)
create mode 100644 fs/richacl.c
create mode 100644 include/linux/richacl.h
create mode 100644 include/uapi/linux/richacl.h
I imagine we could have a lot of these at any given time. It might be
nice to consider how to do this with dedicated slabcaches for better
packing, but I think that would add to the complexity, unfortunately.
The objects are variable in size, so that wouldn't work.
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-07-11 13:26:41
On Tue, Jul 5, 2016 at 3:39 PM, Jeff Layton [off-list ref] wrote:
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
quoted
We need to map from POSIX permissions to NFSv4 permissions when a
chmod() is done, from NFSv4 permissions to POSIX permissions when an acl
is set (which implicitly sets the file permission bits), and from the
MAY_READ/MAY_WRITE/MAY_EXEC/MAY_APPEND flags to NFSv4 permissions when
doing an access check in a richacl.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 118 +++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 3 ++
include/uapi/linux/richacl.h | 44 ++++++++++++++++
3 files changed, 165 insertions(+)
This comment is confusing as I don't see any may_create or may_delete
iops in the final patchset. Do you mean may_create() and may_delete()
here?
Since this is only called from richacl_permission, the comment doesn't
make much sense anymore; removing. Also, richacl_want_to_mask can be
turned into a static function.
quoted
+unsigned int
+richacl_want_to_mask(unsigned int want)
+{
+ unsigned int mask = 0;
+
+ if (want & MAY_READ)
+ mask |= RICHACE_READ_DATA;
+ if (want & MAY_DELETE_SELF)
+ mask |= RICHACE_DELETE;
+ if (want & MAY_TAKE_OWNERSHIP)
+ mask |= RICHACE_WRITE_OWNER;
+ if (want & MAY_CHMOD)
+ mask |= RICHACE_WRITE_ACL;
+ if (want & MAY_SET_TIMES)
+ mask |= RICHACE_WRITE_ATTRIBUTES;
+ if (want & MAY_EXEC)
+ mask |= RICHACE_EXECUTE;
+ /*
+ * differentiate MAY_WRITE from these request
+ */
+ if (want & (MAY_APPEND |
+ MAY_CREATE_FILE | MAY_CREATE_DIR |
+ MAY_DELETE_CHILD)) {
+ if (want & MAY_APPEND)
+ mask |= RICHACE_APPEND_DATA;
+ if (want & MAY_CREATE_FILE)
+ mask |= RICHACE_ADD_FILE;
+ if (want & MAY_CREATE_DIR)
+ mask |= RICHACE_ADD_SUBDIRECTORY;
+ if (want & MAY_DELETE_CHILD)
+ mask |= RICHACE_DELETE_CHILD;
+ } else if (want & MAY_WRITE)
+ mask |= RICHACE_WRITE_DATA;
+ return mask;
+}
+EXPORT_SYMBOL_GPL(richacl_want_to_mask);
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-07-11 13:28:41
On Tue, Jul 5, 2016 at 4:59 PM, Jeff Layton [off-list ref] wrote:
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted
A richacl roughly grants a requested access if the NFSv4 acl in the
richacl grants the requested permissions according to the NFSv4
permission check algorithm and the file mask that applies to the process
includes the requested permissions.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 129 insertions(+)
nit: there is no @mask parm here. Do you mean @want ?
Yes, thanks.
quoted
+int
+richacl_permission(struct inode *inode, const struct richacl *acl,
+ int want)
+{
+ const struct richace *ace;
+ unsigned int mask = richacl_want_to_mask(want);
+ unsigned int requested = mask, denied = 0;
+ int in_owning_group = in_group_p(inode->i_gid);
+ int in_owner_or_group_class = in_owning_group;
+
+ /*
+ * A process is
+ * - in the owner file class if it owns the file,
+ * - in the group file class if it is in the file's owning group or
+ * it matches any of the user or group entries, and
+ * - in the other file class otherwise.
+ * The file class is only relevant for determining which file mask to
+ * apply, which only happens for masked acls.
+ */
+ if (acl->a_flags & RICHACL_MASKED) {
+ if ((acl->a_flags & RICHACL_WRITE_THROUGH) &&
+ uid_eq(current_fsuid(), inode->i_uid)) {
+ denied = requested & ~acl->a_owner_mask;
+ goto out;
+ }
+ } else {
+ /*
+ * When the acl is not masked, there is no need to determine if
+ * the process is in the group class and we can break out
+ * earlier of the loop below.
+ */
+ in_owner_or_group_class = 1;
+ }
+
+ /*
+ * Check if the acl grants the requested access and determine which
+ * file class the process is in.
+ */
+ richacl_for_each_entry(ace, acl) {
+ unsigned int ace_mask = ace->e_mask;
+
+ if (richace_is_inherit_only(ace))
+ continue;
+ if (richace_is_owner(ace)) {
+ if (!uid_eq(current_fsuid(), inode->i_uid))
+ continue;
+ goto entry_matches_owner;
+ } else if (richace_is_group(ace)) {
+ if (!in_owning_group)
+ continue;
+ } else if (richace_is_unix_user(ace)) {
+ if (!uid_eq(current_fsuid(), ace->e_id.uid))
+ continue;
+ if (uid_eq(current_fsuid(), inode->i_uid))
+ goto entry_matches_owner;
+ } else if (richace_is_unix_group(ace)) {
+ if (!in_group_p(ace->e_id.gid))
+ continue;
+ } else
+ goto entry_matches_everyone;
+
+ /*
+ * Apply the group file mask to entries other than owner@ and
+ * everyone@ or user entries matching the owner. This ensures
+ * that we grant the same permissions as the acl computed by
+ * richacl_apply_masks().
+ *
+ * Without this restriction, the following richacl would grant
+ * rw access to processes which are both the owner and in the
+ * owning group, but not to other users in the owning group,
+ * which could not be represented without masks:
+ *
+ * owner:rw::mask
+ * group@:rw::allow
+ */
+ if ((acl->a_flags & RICHACL_MASKED) && richace_is_allow(ace))
+ ace_mask &= acl->a_group_mask;
+
+entry_matches_owner:
+ /* The process is in the owner or group file class. */
+ in_owner_or_group_class = 1;
+
+entry_matches_everyone:
+ /* Check which mask flags the ACE allows or denies. */
+ if (richace_is_deny(ace))
+ denied |= ace_mask & mask;
+ mask &= ~ace_mask;
+
+ /*
+ * Keep going until we know which file class
+ * the process is in.
+ */
+ if (!mask && in_owner_or_group_class)
+ break;
+ }
+ denied |= mask;
+
+ if (acl->a_flags & RICHACL_MASKED) {
+ /*
+ * The file class a process is in determines which file mask
+ * applies. Check if that file mask also grants the requested
+ * access.
+ */
+ if (uid_eq(current_fsuid(), inode->i_uid))
+ denied |= requested & ~acl->a_owner_mask;
+ else if (in_owner_or_group_class)
+ denied |= requested & ~acl->a_group_mask;
+ else {
+ if (acl->a_flags & RICHACL_WRITE_THROUGH)
+ denied = requested & ~acl->a_other_mask;
+ else
+ denied |= requested & ~acl->a_other_mask;
+ }
+ }
+
+out:
+ return denied ? -EACCES : 0;
+}
+EXPORT_SYMBOL_GPL(richacl_permission);
From: Jeff Layton <hidden> Date: 2016-07-12 11:37:00
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
Doing a chmod() sets the file mode, which includes the file permission
bits. When a file has a richacl, the permissions that the richacl
grants need to be limited to what the new file permission bits allow.
This is done by setting the file masks in the richacl to what the file
permission bits map to. The richacl access check algorithm takes the
file masks into account, which ensures that the richacl cannot grant too
many permissions.
It is possible to explicitly add permissions to the file masks which go
beyond what the file permission bits can grant (like the
RICHACE_WRITE_ACL permission). The POSIX.1 standard calls this an
alternate file access control mechanism. A subsequent chmod() would
ensure that those permissions are disabled again.
Signed-off-by: Andreas Gruenbacher <redacted>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 72 insertions(+)
From: Jeff Layton <hidden> Date: 2016-07-12 11:39:39
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
ACLs are considered equivalent to file modes if they only consist of
owner@, group@, and everyone@ entries, the owner@ permissions do not
depend on whether the owner is a member in the owning group, and no
inheritance flags are set. This test is used to avoid storing richacls
if the acl can be computed from the file permission bits.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
---
fs/richacl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 105 insertions(+)
Reviewed-by: Jeff Layton <redacted>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jeff Layton <hidden> Date: 2016-07-12 11:41:55
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
When a new file is created, it can inherit an acl from its parent
directory; this is similar to how default acls work in POSIX ACLs.
As with POSIX ACLs, if a file inherits an acl from its parent directory,
the intersection between the create mode and the permissions granted by
the inherited acl determines the file masks and file permission bits,
and the umask is ignored.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/richacl.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 2 +
2 files changed, 153 insertions(+)
Reviewed-by: Jeff Layton <redacted>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Jeff Layton <hidden> Date: 2016-07-12 11:56:00
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
Automatic Inheritance (AI) allows changes to the acl of a directory to
propagate down to children.
This is mostly implemented in user space: when a process changes the
permissions of a directory and Automatic Inheritance is enabled for that
directory, the process must propagate those changes to all children,
recursively.
The kernel enables this by keeping track of which permissions have been
inherited at create time. In addition, it makes sure that permission
propagation is turned off when the permissions are set explicitly (for
example, upon create or chmod).
Automatic Inheritance works as follows:
- When the RICHACL_AUTO_INHERIT flag in the acl of a file or directory
is not set, the file or directory is not affected by AI.
- When the RICHACL_AUTO_INHERIT flag in the acl of a directory is set
and a file or subdirectory is created in that directory, the
inherited acl will have the RICHACL_AUTO_INHERIT flag set, and all
inherited aces will have the RICHACE_INHERITED_ACE flag set. This
allows user space to distinguish between aces which have been
inherited and aces which have been explicitly added.
- When the RICHACL_PROTECTED acl flag in the acl of a file or directory
is set, AI will not modify the acl. This does not affect propagation
of permissions from the file to its children (if the file is a
directory).
Linux does not have a way of creating files or directories without setting the
file permission bits, so all files created inside a directory with
RICHACL_AUTO_INHERIT set will have the RICHACL_PROTECTED flag set. This
effectively disables Automatic Inheritance.
Protocols which support creating files without specifying permissions can
explicitly clear the RICHACL_PROTECTED flag after creating a file and reset the
file masks to "undo" applying the create mode; see richacl_compute_max_masks().
They should set the RICHACL_DEFAULTED flag. (A mechanism that would allow to
indicate to the kernel to ignore the create mode in the first place when there
are inherited permissions would be nice to have.)
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/richacl.c | 20 +++++++++++++++++++-
include/linux/richacl.h | 12 ++++++++++++
include/uapi/linux/richacl.h | 11 ++++++++++-
3 files changed, 41 insertions(+), 2 deletions(-)
richacl_put(acl);
acl = NULL;
} else {
+ /*
+ * We need to set RICHACL_PROTECTED because we are
+ * doing an implicit chmod
+ */
+ if (richacl_is_auto_inherit(acl))
+ acl->a_flags |= RICHACL_PROTECTED;
+
richacl_compute_max_masks(acl);
/*
* Ensure that the acl will not grant any permissions
Barf. AI seems like a trainwreck waiting to happen. What are the
chances that userland is going to get this right?
Still, I do applaud the fact that you're just doing the bare minimum in
kernel to support userland apps that want this. Thanks for not trying
to push the propagation of the changed ACEs into the kernel.
Reviewed-by: Jeff Layton <redacted>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Fair enough. I do wonder a bit whether we might be better served with a
new set of syscalls for this instead of using xattrs (as I think
Christoph has suggested). What _is_ the rationale for doing this with
xattrs, btw?
Regardless, this patch looks fine to me, assuming that we really do
want to do this xattrs:
Reviewed-by: Jeff Layton <redacted>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
EXPORT_SYMBOL_GPL(richacl_permission);
/**
+ * set_richacl - set the richacl of an inode
+ * @inode: inode whose richacl to set
+ * @acl: access control list
+ */
+int
+set_richacl(struct inode *inode, struct richacl *acl)
+{
+ if (!IS_RICHACL(inode))
+ return -EOPNOTSUPP;
+ if (!inode->i_op->set_richacl)
+ return -EOPNOTSUPP;
+
+ if (!uid_eq(current_fsuid(), inode->i_uid) &&
+ inode_permission(inode, MAY_CHMOD) &&
+ !capable(CAP_FOWNER))
+ return -EPERM;
+
+ return inode->i_op->set_richacl(inode, acl);
+}
+EXPORT_SYMBOL(set_richacl);
+
+/**
* __richacl_chmod - update the file masks to reflect the new mode
* @acl: access control list
* @mode: new file permission bits including the file type
From: Jeff Layton <hidden> Date: 2016-07-12 12:13:57
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted hunk
Hook the richacl permission checking function into the vfs.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/namei.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 52 insertions(+), 2 deletions(-)
__putname(name);
}
-static int check_acl(struct inode *inode, int mask)
+static int check_richacl(struct inode *inode, int mask)
+{
+#ifdef CONFIG_FS_RICHACL
+ if (mask & MAY_NOT_BLOCK) {
+ struct base_acl *base_acl;
+
+ base_acl = rcu_dereference(inode->i_acl);
+ if (!base_acl)
+ goto no_acl;
+ /* no ->get_richacl() calls in RCU mode... */
+ if (is_uncached_acl(base_acl))
+ return -ECHILD;
+ return richacl_permission(inode, richacl(base_acl),
+ mask & ~MAY_NOT_BLOCK);
+ } else {
+ struct richacl *acl;
+
+ acl = get_richacl(inode);
+ if (IS_ERR(acl))
+ return PTR_ERR(acl);
+ if (acl) {
+ int error = richacl_permission(inode, acl, mask);
+ richacl_put(acl);
+ return error;
+ }
+ }
+no_acl:
+#endif
nit: Can you move the above to a static inline or something that becomes a noop when the config var is turned off?
quoted hunk
+ if (mask & (MAY_DELETE_SELF | MAY_TAKE_OWNERSHIP |
+ MAY_CHMOD | MAY_SET_TIMES)) {
+ /* File permission bits cannot grant this. */
+ return -EACCES;
+ }
+ return -EAGAIN;
+}
+
+static int check_posix_acl(struct inode *inode, int mask)
{
#ifdef CONFIG_FS_POSIX_ACL
if (mask & MAY_NOT_BLOCK) {
@@ -294,11 +331,24 @@ static int acl_permission_check(struct inode *inode, int mask)
{
unsigned int mode = inode->i_mode;
+ /*
+ * With POSIX ACLs, the (mode & S_IRWXU) bits exactly match the owner
+ * permissions, and we can skip checking posix acls for the owner.
+ * With richacls, the owner may be granted fewer permissions than the
+ * mode bits seem to suggest (for example, append but not write), and
+ * we always need to check the richacl.
+ */
+
+ if (IS_RICHACL(inode)) {
+ int error = check_richacl(inode, mask);
+ if (error != -EAGAIN)
+ return error;
+ }
if (likely(uid_eq(current_fsuid(), inode->i_uid)))
mode >>= 6;
else {
if (IS_POSIXACL(inode) && (mode & S_IRWXG)) {
- int error = check_acl(inode, mask);
+ int error = check_posix_acl(inode, mask);
if (error != -EAGAIN)
return error;
}
Looks fine other than the nit above:
Reviewed-by: Jeff Layton <redacted>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: J. Bruce Fields <hidden> Date: 2016-07-12 19:11:44
On Tue, Jul 12, 2016 at 07:56:00AM -0400, Jeff Layton wrote:
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted
Automatic Inheritance (AI) allows changes to the acl of a directory to
propagate down to children.
This is mostly implemented in user space: when a process changes the
permissions of a directory and Automatic Inheritance is enabled for that
directory, the process must propagate those changes to all children,
recursively.
The kernel enables this by keeping track of which permissions have been
inherited at create time. In addition, it makes sure that permission
propagation is turned off when the permissions are set explicitly (for
example, upon create or chmod).
Automatic Inheritance works as follows:
- When the RICHACL_AUTO_INHERIT flag in the acl of a file or directory
is not set, the file or directory is not affected by AI.
- When the RICHACL_AUTO_INHERIT flag in the acl of a directory is set
and a file or subdirectory is created in that directory, the
inherited acl will have the RICHACL_AUTO_INHERIT flag set, and all
inherited aces will have the RICHACE_INHERITED_ACE flag set. This
allows user space to distinguish between aces which have been
inherited and aces which have been explicitly added.
- When the RICHACL_PROTECTED acl flag in the acl of a file or directory
is set, AI will not modify the acl. This does not affect propagation
of permissions from the file to its children (if the file is a
directory).
Linux does not have a way of creating files or directories without setting the
file permission bits, so all files created inside a directory with
RICHACL_AUTO_INHERIT set will have the RICHACL_PROTECTED flag set. This
effectively disables Automatic Inheritance.
Protocols which support creating files without specifying permissions can
explicitly clear the RICHACL_PROTECTED flag after creating a file and reset the
file masks to "undo" applying the create mode; see richacl_compute_max_masks().
They should set the RICHACL_DEFAULTED flag. (A mechanism that would allow to
indicate to the kernel to ignore the create mode in the first place when there
are inherited permissions would be nice to have.)
Signed-off-by: Andreas Gruenbacher <redacted>
---
fs/richacl.c | 20 +++++++++++++++++++-
include/linux/richacl.h | 12 ++++++++++++
include/uapi/linux/richacl.h | 11 ++++++++++-
3 files changed, 41 insertions(+), 2 deletions(-)
richacl_put(acl);
acl = NULL;
} else {
+ /*
+ * We need to set RICHACL_PROTECTED because we are
+ * doing an implicit chmod
+ */
+ if (richacl_is_auto_inherit(acl))
+ acl->a_flags |= RICHACL_PROTECTED;
+
richacl_compute_max_masks(acl);
/*
* Ensure that the acl will not grant any permissions
Barf. AI seems like a trainwreck waiting to happen. What are the
chances that userland is going to get this right?
This is just taken from Windows, so presumably Samba should do the right
thing (with one workaround required--see the discussion of PROTECTED in
the changelog).
Still, I do applaud the fact that you're just doing the bare minimum in
kernel to support userland apps that want this. Thanks for not trying
to push the propagation of the changed ACEs into the kernel.
Agreed. But, again, this is just what Windows does. (I wonder whether
there's an OS that's actually tried do any sort of atomic propagation.)
--b.
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-07-12 20:28:20
On Tue, Jul 12, 2016 at 9:11 PM, J. Bruce Fields [off-list ref] wrote:
On Tue, Jul 12, 2016 at 07:56:00AM -0400, Jeff Layton wrote:
quoted
Barf. AI seems like a trainwreck waiting to happen. What are the
chances that userland is going to get this right?
This is just taken from Windows, so presumably Samba should do the right
thing (with one workaround required--see the discussion of PROTECTED in
the changelog).
quoted
Still, I do applaud the fact that you're just doing the bare minimum in
kernel to support userland apps that want this. Thanks for not trying
to push the propagation of the changed ACEs into the kernel.
Agreed. But, again, this is just what Windows does. (I wonder whether
there's an OS that's actually tried do any sort of atomic propagation.)
NetWare's NSS filesystem did have automatic propagation, but I think
that was implemented by composing the permissions along the path to a
file at access time.
Andreas
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-07-13 12:35:02
Frank,
On Tue, Jul 5, 2016 at 7:08 PM, Frank Filz [off-list ref] wrote:
quoted
quoted
+ * Note: functions like richacl_allowed_to_who(),
+richacl_group_class_allowed(),
+ * and richacl_compute_max_masks() iterate through the entire acl in
+reverse
+ * order as an optimization.
+ *
+ * In the standard algorithm, aces are considered in forward order.
+When a
+ * process matches an ace, the permissions in the ace are either
+allowed or
+ * denied depending on the ace type. Once a permission has been
+allowed or
+ * denied, it is no longer considered in further aces.
+ *
+ * By iterating through the acl in reverse order, we can compute the
+same
+ * result without having to keep track of which permissions have been
+allowed
+ * and denied already.
+ */
Clever!
Hmm, but does that result in examining the whole ACL for most access checks, at least for files where most of the accesses are by the owner, or a member of a specific group (with perhaps a ton of special case users added on the end)?
I don't understand -- what does this algorithm have to do with access checks?
Thanks,
Andreas
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Frank Filz <hidden> Date: 2016-07-13 19:39:53
quoted
Hmm, but does that result in examining the whole ACL for most access
checks, at least for files where most of the accesses are by the owner, or a
member of a specific group (with perhaps a ton of special case users added
on the end)?
I don't understand -- what does this algorithm have to do with access checks?
Oh, sorry, misread the patch... got caught up looking at a tree and not seeing the forest...
Frank
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-07-14 20:02:04
On Wed, Jul 6, 2016 at 8:57 PM, Jeff Layton [off-list ref] wrote:
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted
Cache richacls in struct inode so that this doesn't have to be done
individually in each filesystem. This is similar to POSIX ACLs.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/inode.c | 13 +++++---
fs/richacl.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 5 ++-
include/linux/richacl.h | 11 +++++++
4 files changed, 105 insertions(+), 5 deletions(-)
@@ -20,6 +20,87 @@#include#include+voidset_cached_richacl(structinode*inode,structrichacl*acl)+{+structbase_acl*old;++old=xchg(&inode->i_acl,&richacl_get(acl)->a_base);+if(!is_uncached_acl(old))+base_acl_put(old);+}+EXPORT_SYMBOL_GPL(set_cached_richacl);++voidforget_cached_richacl(structinode*inode)+{+__forget_cached_acl(&inode->i_acl);+}+EXPORT_SYMBOL_GPL(forget_cached_richacl);++structrichacl*get_richacl(structinode*inode)+{+structbase_acl*sentinel,*base_acl;+structrichacl*acl;++if(!IS_RICHACL(inode))+returnNULL;++/*+*Thesentinelisusedtodetectwhenanotheroperationlike+*set_cached_richacl()orforget_cached_richacl()raceswith+*get_richacl().+*Itisguaranteedthatis_uncached_acl(sentinel)istrue.+*/++base_acl=__get_cached_acl(&inode->i_acl);+if(!is_uncached_acl(base_acl))+returnrichacl(base_acl);++sentinel=uncached_acl_sentinel(current);++/*+*IftheACLisn'tbeingreadyet,setoursentinel.Otherwise,the+*currentvalueoftheACLwillnotbeACL_NOT_CACHEDandsoourown+*sentinelwillnotbeset;anothertaskwillupdatethecache.We+*couldwaitforthatothertasktocompleteitsjob,butit'seasier+*tojustcall->get_acltofetchtheACLourself.(Thisisgoingto+*beanunlikelyrace.)+*/+if(cmpxchg(&inode->i_acl,ACL_NOT_CACHED,sentinel)!=ACL_NOT_CACHED)+/* fall through */;+
So you do the same thing regardless of the outcome of the above? Why
bother with the if at all here? Just do the cmpxchg and toss out the
result.
This is to document explicitly that we don't care either way ...
quoted
+ /*
+ * Normally, the ACL returned by ->get_richacl will be cached.
+ * A filesystem can prevent that by calling
+ * forget_cached_richacl(inode) in ->get_richacl.
+ *
+ * If the filesystem doesn't have a ->get_richacl function at all,
+ * we'll just create the negative cache entry.
+ */
+ if (!inode->i_op->get_richacl) {
+ set_cached_richacl(inode, NULL);
+ return NULL;
+ }
+
+ acl = inode->i_op->get_richacl(inode);
+ if (IS_ERR(acl)) {
+ /*
+ * Remove our sentinel so that we don't block future attempts
+ * to cache the ACL.
+ */
+ cmpxchg(&inode->i_acl, sentinel, ACL_NOT_CACHED);
+ return acl;
+ }
+
So this is sort of icky: only the task that sets the sentinel can set
the cached acl.
You could have one task set the sentinel, call get_richacl and fail, but then a concurrent task also issues a get_richacl and succeeds. That task will get its acl, but it doesn't end up getting cached and subsequent callers then have to reissue the request. It would be good if the first successful fetch of the acl sets it in the cache.
That said, getting the acl could be pretty expensive with some filesystems. NFS or CIFS are going to have to do an on the wire call to fetch them, for instance. I think it would be better to have concurrent callers wait for the first caller's result instead of issuing parallel get_richacl requests.
Making successive callers wait on the first caller should certainly be
possible, for POSIX ACLs as well as for Richacls. For network
filesystems, an improvement should be measurable. I'm thinking of an
approach similar to __wait_on_freeing_inode which uses a hash table of
wait queues instead of per-inode ones. It's a bit tricky to get this
right, though.
quoted
+ /*
+ * Cache the result, but only if our sentinel is still in place.
+ */
+ richacl_get(acl);
+ if (unlikely(cmpxchg(&inode->i_acl, sentinel, &acl->a_base) != sentinel))
+ richacl_put(acl);
+ return acl;
+}
+EXPORT_SYMBOL_GPL(get_richacl);
+
/**
* richacl_alloc - allocate a richacl
* @count: number of entries
Fair enough. I do wonder a bit whether we might be better served with a
new set of syscalls for this instead of using xattrs (as I think
Christoph has suggested). What _is_ the rationale for doing this with
xattrs, btw?
Xattrs are intended for exactly this kind of auxiliary file
information. They are used for POSIX ACLs as well, and they are not
going to go away. A number of utilities now include support for
xattrs. By using xattrs for Richacls, those tools will automatically
have at least basic Richacl support (for backup/restore and similar
purposes, for example). In the kernel, supporting a new set of xattrs
is pretty cheap and easy. So I really don't see the point of inventing
yet another interface for basically the same type of information.
Regardless, this patch looks fine to me, assuming that we really do
want to do this xattrs:
Reviewed-by: Jeff Layton <redacted>
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-07-14 20:59:16
On Tue, Jul 12, 2016 at 2:13 PM, Jeff Layton [off-list ref] wrote:
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
quoted
Hook the richacl permission checking function into the vfs.
Signed-off-by: Andreas Gruenbacher <redacted>
---
fs/namei.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 52 insertions(+), 2 deletions(-)
@@ -256,7 +257,43 @@ void putname(struct filename *name)__putname(name);}-staticintcheck_acl(structinode*inode,intmask)+staticintcheck_richacl(structinode*inode,intmask)+{+#ifdef CONFIG_FS_RICHACL+if(mask&MAY_NOT_BLOCK){+structbase_acl*base_acl;++base_acl=rcu_dereference(inode->i_acl);+if(!base_acl)+gotono_acl;+/* no ->get_richacl() calls in RCU mode... */+if(is_uncached_acl(base_acl))+return-ECHILD;+returnrichacl_permission(inode,richacl(base_acl),+mask&~MAY_NOT_BLOCK);+}else{+structrichacl*acl;++acl=get_richacl(inode);+if(IS_ERR(acl))+returnPTR_ERR(acl);+if(acl){+interror=richacl_permission(inode,acl,mask);+richacl_put(acl);+returnerror;+}+}+no_acl:+#endif
nit: Can you move the above to a static inline or something that becomes a noop when the config var is turned off?
We could move check_richacl into richacl.c and check_posix_acl into
posix_acl.c. Given that those functions are currently only called once
in namei.c, that's a very small improvement at most though.
quoted
+ if (mask & (MAY_DELETE_SELF | MAY_TAKE_OWNERSHIP |
+ MAY_CHMOD | MAY_SET_TIMES)) {
+ /* File permission bits cannot grant this. */
+ return -EACCES;
+ }
+ return -EAGAIN;
+}
+
+static int check_posix_acl(struct inode *inode, int mask)
{
#ifdef CONFIG_FS_POSIX_ACL
if (mask & MAY_NOT_BLOCK) {
@@ -294,11 +331,24 @@ static int acl_permission_check(struct inode *inode, int mask) { unsigned int mode = inode->i_mode;+ /*+ * With POSIX ACLs, the (mode & S_IRWXU) bits exactly match the owner+ * permissions, and we can skip checking posix acls for the owner.+ * With richacls, the owner may be granted fewer permissions than the+ * mode bits seem to suggest (for example, append but not write), and+ * we always need to check the richacl.+ */++ if (IS_RICHACL(inode)) {+ int error = check_richacl(inode, mask);+ if (error != -EAGAIN)+ return error;+ } if (likely(uid_eq(current_fsuid(), inode->i_uid))) mode >>= 6; else { if (IS_POSIXACL(inode) && (mode & S_IRWXG)) {- int error = check_acl(inode, mask);+ int error = check_posix_acl(inode, mask); if (error != -EAGAIN) return error; }
Looks fine other than the nit above:
Reviewed-by: Jeff Layton <redacted>