From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:33
Al,
could you please make sure you are happy with the current version of the
richacl patch queue for the next merge window?
Changes since the last posting (https://lwn.net/Articles/671398/):
* Some combinations of ACL entry flags were not computed correctly when
ACL entries were inherited from a directory to files and directories
created inside that directory. This is fixed now, with a test case
covering all the flag combinations in the richacl package.
* Rebases on top of v4.5-rc5.
The complete patch queue is available here:
git://git.kernel.org/pub/scm/linux/kernel/git/agruen/linux-richacl.git \
richacl-2016-02-22
The richacl user-space utilitites, man pages, and test suite are available
here:
https://github.com/andreas-gruenbacher/richacl
Changes to other user-space packages for richacl:
https://github.com/andreas-gruenbacher/coreutilshttps://github.com/andreas-gruenbacher/e2fsprogshttps://github.com/andreas-gruenbacher/xfsprogs-devhttps://github.com/andreas-gruenbacher/nfs-utils
Please see the richacl homepage for more information:
http://www.bestbits.at/richacl/
Thanks,
Andreas
Andreas Gruenbacher (20):
vfs: Add IS_ACL() and IS_RICHACL() tests
vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags
vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
vfs: Make the inode passed to inode_change_ok non-const
vfs: Add permission flags for setting file attributes
richacl: In-memory representation and helper functions
richacl: Permission mapping functions
richacl: Compute maximum file masks from an acl
richacl: Permission check algorithm
posix_acl: Unexport acl_by_type and make it static
vfs: Cache base_acl objects in inodes
vfs: Add get_richacl and set_richacl inode operations
vfs: Cache richacl in struct inode
richacl: Update the file masks in chmod()
richacl: Check if an acl is equivalent to a file mode
richacl: Create-time inheritance
richacl: Automatic Inheritance
richacl: xattr mapping functions
richacl: Add richacl xattr handler
vfs: Add richacl permission checking
Aneesh Kumar K.V (2):
ext4: Add richacl support
ext4: Add richacl feature flag
drivers/staging/lustre/lustre/llite/llite_lib.c | 2 +-
fs/Kconfig | 3 +
fs/Makefile | 2 +
fs/attr.c | 81 +++-
fs/ext4/Kconfig | 11 +
fs/ext4/Makefile | 1 +
fs/ext4/ext4.h | 6 +-
fs/ext4/file.c | 3 +
fs/ext4/ialloc.c | 11 +-
fs/ext4/inode.c | 12 +-
fs/ext4/namei.c | 5 +
fs/ext4/richacl.c | 142 ++++++
fs/ext4/richacl.h | 40 ++
fs/ext4/super.c | 49 +-
fs/ext4/xattr.c | 7 +
fs/f2fs/acl.c | 4 +-
fs/inode.c | 15 +-
fs/jffs2/acl.c | 10 +-
fs/namei.c | 118 ++++-
fs/posix_acl.c | 50 +-
fs/richacl_base.c | 580 ++++++++++++++++++++++++
fs/richacl_inode.c | 333 ++++++++++++++
fs/richacl_xattr.c | 235 ++++++++++
fs/xattr.c | 29 +-
include/linux/fs.h | 60 ++-
include/linux/posix_acl.h | 13 +-
include/linux/richacl.h | 208 +++++++++
include/linux/richacl_xattr.h | 31 ++
include/uapi/linux/Kbuild | 2 +
include/uapi/linux/fs.h | 3 +-
include/uapi/linux/richacl.h | 152 +++++++
include/uapi/linux/richacl_xattr.h | 44 ++
include/uapi/linux/xattr.h | 2 +
33 files changed, 2157 insertions(+), 107 deletions(-)
create mode 100644 fs/ext4/richacl.c
create mode 100644 fs/ext4/richacl.h
create mode 100644 fs/richacl_base.c
create mode 100644 fs/richacl_inode.c
create mode 100644 fs/richacl_xattr.c
create mode 100644 include/linux/richacl.h
create mode 100644 include/linux/richacl_xattr.h
create mode 100644 include/uapi/linux/richacl.h
create mode 100644 include/uapi/linux/richacl_xattr.h
--
2.4.3
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:08
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 <redacted>
Reviewed-by: J. Bruce Fields <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-02-29 08:17:09
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>
---
fs/attr.c | 2 +-
include/linux/fs.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:12
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_base.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-02-29 08:17:13
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_base.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 158 insertions(+)
@@ -183,3 +183,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-02-29 08:17:14
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/Makefile | 2 +-
fs/richacl_inode.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 3 +
3 files changed, 153 insertions(+), 1 deletion(-)
create mode 100644 fs/richacl_inode.c
@@ -0,0 +1,149 @@+/*+*Copyright(C)2010Novell,Inc.+*Copyright(C)2015RedHat,Inc.+*WrittenbyAndreasGruenbacher<agruenba@redhat.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseaspublishedbythe+*FreeSoftwareFoundation;eitherversion2,or(atyouroption)any+*laterversion.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,but+*WITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.SeetheGNU+*GeneralPublicLicenseformoredetails.+*/++#include<linux/sched.h>+#include<linux/module.h>+#include<linux/fs.h>+#include<linux/slab.h>+#include<linux/richacl.h>++/**+*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);
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:15
acl_by_type(inode, type) returns a pointer to either inode->i_acl or
inode->i_default_acl depending on type. This is useful in
fs/posix_acl.c, but should never have been visible outside that file.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/posix_acl.c | 3 +--
include/linux/posix_acl.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:20
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_base.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 1 +
2 files changed, 105 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:22
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_base.c | 13 ++++++++++++-
fs/richacl_inode.c | 7 +++++++
include/linux/richacl.h | 12 ++++++++++++
include/uapi/linux/richacl.h | 11 ++++++++++-
4 files changed, 41 insertions(+), 2 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:36
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 by nfsd.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <redacted>
Reviewed-by: Andreas Dilger <redacted>
---
fs/Kconfig | 3 +++
fs/namei.c | 8 ++++----
include/linux/fs.h | 12 ++++++++++++
include/uapi/linux/fs.h | 3 ++-
4 files changed, 21 insertions(+), 5 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:39
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>
---
fs/namei.c | 49 +++++++++++++++++++++++++++++++++----------------
include/linux/fs.h | 2 ++
2 files changed, 35 insertions(+), 16 deletions(-)
@@ -2646,6 +2649,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-02-29 08:17:53
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 | 2 +
fs/richacl_base.c | 67 ++++++++++++++++
include/linux/richacl.h | 179 +++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/richacl.h | 99 ++++++++++++++++++++++++
5 files changed, 348 insertions(+)
create mode 100644 fs/richacl_base.c
create mode 100644 include/linux/richacl.h
create mode 100644 include/uapi/linux/richacl.h
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:17:55
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>
---
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-02-29 08:18:12
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>
Reviewed-by: Andreas Dilger <redacted>
---
drivers/staging/lustre/lustre/llite/llite_lib.c | 2 +-
fs/f2fs/acl.c | 4 +--
fs/inode.c | 4 +--
fs/jffs2/acl.c | 10 ++++--
fs/posix_acl.c | 41 +++++++++++++------------
fs/richacl_base.c | 4 +--
include/linux/fs.h | 34 ++++++++++++++++++--
include/linux/posix_acl.h | 12 +++-----
include/linux/richacl.h | 9 +++---
9 files changed, 75 insertions(+), 45 deletions(-)
@@ -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-02-29 08:18:16
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>
---
include/linux/fs.h | 2 ++
1 file changed, 2 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:18:21
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 | 11 +++++--
fs/posix_acl.c | 2 +-
fs/richacl_inode.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 5 +++-
include/linux/richacl.h | 6 ++++
5 files changed, 96 insertions(+), 5 deletions(-)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:18:24
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_base.c | 42 ++++++++++++++++++++++++++++++++++++++++++
fs/richacl_inode.c | 30 ++++++++++++++++++++++++++++++
include/linux/richacl.h | 2 ++
3 files changed, 74 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:18:31
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_base.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
fs/richacl_inode.c | 70 ++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl.h | 2 ++
3 files changed, 153 insertions(+)
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-02-29 08:18:52
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(-)
@@ -1305,6 +1305,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";
@@ -1501,6 +1523,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;
@@ -3267,8 +3296,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))
@@ -3351,8 +3380,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)||
@@ -4668,8 +4698,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;
From: Christoph Hellwig <hch@infradead.org> Date: 2016-03-11 14:01:34
On Mon, Feb 29, 2016 at 09:17:05AM +0100, Andreas Gruenbacher wrote:
Al,
could you please make sure you are happy with the current version of the
richacl patch queue for the next merge window?
I'm still not happy.
For one I still see no reason to merge this broken ACL model at all.
It provides our actualy Linux users no benefit at all, while breaking
a lot of assumptions, especially by adding allow and deny ACE at the
same sime.
It also doesn't help with the issue that the main thing it's trying
to be compatible with (Windows) actually uses a fundamentally different
identifier to apply the ACLs to - as long as you're still limited
to users and groups and not guids we'll still have that mapping problem
anyway.
But besides that fundamental question on the purpose of it I also
don't think the code is suitable, more in the individual patches.
--
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: Christoph Hellwig <hch@infradead.org> Date: 2016-03-11 14:07:49
On Mon, Feb 29, 2016 at 09:17:16AM +0100, Andreas Gruenbacher wrote:
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).
This base_acl object is pointless. I've asked in the past to have
a proper container for the ACLs in common code, but a union
of a refcount and a rcu head doesn't really fit that category.
But this points out that the f2fs folks really need a couple of
slaps on their hands. Not if generic funtionality doesn't
fit your needs you are not going to blindly copy and paste it,
please talk to find a solution instead of duplicating it.
Folks, please come up with a suggestion to get rid of f2fs_acl_clone,
f2fs_acl_create_masq and f2fs_acl_create ASAP.
--
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: J. Bruce Fields <hidden> Date: 2016-03-11 14:07:59
On Fri, Mar 11, 2016 at 06:01:34AM -0800, Christoph Hellwig wrote:
On Mon, Feb 29, 2016 at 09:17:05AM +0100, Andreas Gruenbacher wrote:
quoted
Al,
could you please make sure you are happy with the current version of the
richacl patch queue for the next merge window?
I'm still not happy.
For one I still see no reason to merge this broken ACL model at all.
It provides our actualy Linux users no benefit at all, while breaking
a lot of assumptions, especially by adding allow and deny ACE at the
same sime.
Could you explain what you mean by "adding allow and deny ACE at the
same time"?
It also doesn't help with the issue that the main thing it's trying
to be compatible with (Windows) actually uses a fundamentally different
identifier to apply the ACLs to - as long as you're still limited
to users and groups and not guids we'll still have that mapping problem
anyway.
Agreed, but, one step at a time? My impression is that the Samba people
still consider this a step forward for Linux compatibility.
--b.
But besides that fundamental question on the purpose of it I also
don't think the code is suitable, more in the individual patches.
What's the point of a tiny separate file here? All richacls files
together are still small, and it would be much preferably to have all
that code together.
From: Christoph Hellwig <hch@infradead.org> Date: 2016-03-11 14:17:35
On Mon, Feb 29, 2016 at 09:17:24AM +0100, Andreas Gruenbacher wrote:
Add richacl xattr handler implementing the xattr operations based on the
get_richacl and set_richacl inode operations.
Given all the issues with Posix ACLs and selinux attributes these really
should be proper syscalls instead of abusing the xattr interface.
--
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
Thi isn't ext4-specific and potentially duplicated in every caller.
Please provide this as a common helper.
Also while we're at it, the mode argument is ignore and the function
always uses inode->i_mode instead.
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-03-11 16:11:52
On Fri, Mar 11, 2016 at 3:01 PM, Christoph Hellwig [off-list ref] wrote:
On Mon, Feb 29, 2016 at 09:17:05AM +0100, Andreas Gruenbacher wrote:
quoted
Al,
could you please make sure you are happy with the current version of the
richacl patch queue for the next merge window?
I'm still not happy.
For one I still see no reason to merge this broken ACL model at all.
It provides our actualy Linux users no benefit at all,
This permission model is useful in mixed environments that involve
UNIX and Windows machines. Think of NAS boxes with Linux and Windows
clients, for example. It also fits the NFSv4 ACL model very well. If
you're not a user dealing with such environments, then the model
likely won't provide any benefits to *you*, and you're better off with
a less complicated permission model. That doesn't say anything about
other users, though.
while breaking a lot of assumptions,
The model is designed specifically to be compliant with the POSIX
permission model. What assumptions are you talking about?
especially by adding allow and deny ACE at the same time.
I remember from past discussions that a permission model like the
POSIX ACL model that doesn't have DENY ACEs would be more to your
liking. This argument is dead from the start though: NFSv4 ACLs
without DENY ACEs cannot represent basic file permissions like 0604
where the owning group has fewer permissions than others, for example
(see the richaclex(7) man page). We would end up with a permission
model that isn't even compatible with the traditional POSIX file
permission model, one which nobody else implements or cares about.
It also doesn't help with the issue that the main thing it's trying
to be compatible with (Windows) actually uses a fundamentally different
identifier to apply the ACLs to - as long as you're still limited
to users and groups and not guids we'll still have that mapping problem
anyway.
Samba has been dealing with mapping between SIDs and UIDs/GIDs for a
long time, and it's working acceptably well.
We could store SIDs in ACEs, but that wouldn't make the actual
problems go away: Files on Linux have an owner and an owning group
which are identitifed by UID/GID, whereas a file is owned by a SID
which can be either a user or a group in a SID world. Also, processes
on Linux have an owner and a list of groups which are identified by
UID/GID, so any SIDs stored in filesystems would never match a
process, anyway.
(NFSv4 refers to users and groups as opposed to SIDs, and so it
doesn't have this problem.)
Thanks,
Andreas
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-03-11 16:24:46
On Fri, Mar 11, 2016 at 3:07 PM, Christoph Hellwig [off-list ref] wrote:
On Mon, Feb 29, 2016 at 09:17:16AM +0100, Andreas Gruenbacher wrote:
quoted
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).
This base_acl object is pointless. I've asked in the past to have
a proper container for the ACLs in common code, but a union
of a refcount and a rcu head doesn't really fit that category.
POSIX ACLs and RichACLs are different objects, with different members
and different algorithms operating on them. The only commonality is
that they are both kmalloc()ed, reference counted objects, and when an
inode is destroyed, both kinds of ACLs can be put in the same way,
avoiding an unnecessary if. What kind of common-code container beyond
that are you still dreaming about?
Thanks,
Andreas
From: Steve French <smfrench@gmail.com> Date: 2016-03-11 20:05:37
On Fri, Mar 11, 2016 at 10:11 AM, Andreas Gruenbacher
[off-list ref] wrote:
On Fri, Mar 11, 2016 at 3:01 PM, Christoph Hellwig [off-list ref] w
The model is designed specifically to be compliant with the POSIX
permission model. What assumptions are you talking about?
quoted
especially by adding allow and deny ACE at the same time.
I remember from past discussions that a permission model like the
POSIX ACL model that doesn't have DENY ACEs would be more to your
liking. This argument is dead from the start though: NFSv4 ACLs
without DENY ACEs cannot represent basic file permissions like 0604
where the owning group has fewer permissions than others, for example
(see the richaclex(7) man page). We would end up with a permission
model that isn't even compatible with the traditional POSIX file
permission model, one which nobody else implements or cares about.
NFSv4.1 (and later) and Samba's (and cifs.ko and NTFS-3g) ACL model are close
enough that doing a common approach that helps all three seems
very reasonable.
A loosely related question is what can be done for tools around existing
interfaces for ACLs. I recently found out NTFS-3g has this xattr:
static const char nf_ns_xattr_ntfs_acl[] = "system.ntfs_acl";
which allows you to query system.ntfs_acl xattr to get their full ACL
(I hope) from NTFS but it hard to read without tools to parse
the blobs better. I was prototyping adding this to cifs.ko for
the most current (SMB3 and later) protocol dialects at least
to allow backup and debug tools to use this to get the actual
ACL. cifs.ko ACL should match almost exactly to NTFS-3g's but
I wish I could find some tools that use this xattr so I could try
comparing this with cacls.exe output and smbcacls (samba tool)
for display detailed ACL information. Any idea of disk management tools
for dumping/viewing/editing ntfs ACLs on Linux for comparison?
quoted
It also doesn't help with the issue that the main thing it's trying
to be compatible with (Windows) actually uses a fundamentally different
identifier to apply the ACLs to - as long as you're still limited
to users and groups and not guids we'll still have that mapping problem
anyway.
Samba has been dealing with mapping between SIDs and UIDs/GIDs for a
long time, and it's working acceptably well.
We could store SIDs in ACEs, but that wouldn't make the actual
problems go away: Files on Linux have an owner and an owning group
which are identitifed by UID/GID, whereas a file is owned by a SID
which can be either a user or a group in a SID world. Also, processes
on Linux have an owner and a list of groups which are identified by
UID/GID, so any SIDs stored in filesystems would never match a
process, anyway.
Samba's SID<->Username and SID<->UID mapping does work
acceptably well, if a bit "over-configurable" ie with
many choices for how it is done.
(the related RFC2307 mapping needed for mapping usernames
to uids across an enterprise, which Samba's winbind can
also do is helpful much more broadly - since, unlike a uid
which is too small, the username in an NFS ACLs
are basically a one to one mapping for Samba to SIDs (there
are may samba vfs modules for different OS that already
do this, including a one for an earlier version of Linux RichACLs)
Sounds like I need to quickly rework the SMB3 ACL helper functions
for cifs.ko
Also do you know where is the current version of the corresponding
vfs_richacl for
Samba which works with the current RichACL format?
--
Thanks,
Steve
From: Jeremy Allison <hidden> Date: 2016-03-11 23:02:54
On Fri, Mar 11, 2016 at 02:05:16PM -0600, Steve French wrote:
Sounds like I need to quickly rework the SMB3 ACL helper functions
for cifs.ko
Also do you know where is the current version of the corresponding
vfs_richacl for
Samba which works with the current RichACL format?
I have a patch for a new vfs_richacl somewhere. I remember
sending it to Andreas for testing...
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
On Fri, 2016-03-11 at 09:07 -0500, J. Bruce Fields wrote:
On Fri, Mar 11, 2016 at 06:01:34AM -0800, Christoph Hellwig wrote:
quoted
On Mon, Feb 29, 2016 at 09:17:05AM +0100, Andreas Gruenbacher
wrote:
quoted
Al,
could you please make sure you are happy with the current version
of the
richacl patch queue for the next merge window?
I'm still not happy.
For one I still see no reason to merge this broken ACL model at
all.
It provides our actualy Linux users no benefit at all, while
breaking
a lot of assumptions, especially by adding allow and deny ACE at
the
same sime.
Could you explain what you mean by "adding allow and deny ACE at the
same time"?
quoted
It also doesn't help with the issue that the main thing it's trying
to be compatible with (Windows) actually uses a fundamentally
different
identifier to apply the ACLs to - as long as you're still limited
to users and groups and not guids we'll still have that mapping
problem
anyway.
Agreed, but, one step at a time? My impression is that the Samba
people
still consider this a step forward for Linux compatibility.
It is a step forward, but being able to store SIDs in the ACL, would be
a much better one.
Simo.
--b.
quoted
But besides that fundamental question on the purpose of it I also
don't think the code is suitable, more in the individual patches.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs"
in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-03-13 23:02:14
On Sat, Mar 12, 2016 at 12:02 AM, Jeremy Allison [off-list ref] wrote:
On Fri, Mar 11, 2016 at 02:05:16PM -0600, Steve French wrote:
quoted
Sounds like I need to quickly rework the SMB3 ACL helper functions
for cifs.ko
Also do you know where is the current version of the corresponding
vfs_richacl for
Samba which works with the current RichACL format?
I have a patch for a new vfs_richacl somewhere. I remember
sending it to Andreas for testing...
Ah, the patch was for testing, not resting ... how could I get that mixed up.
I've applied your patch to the latest master branch, made it compile
again, and fixed a few obvious problems. The results I get with
smbcacls look reasonable now.
The code is here:
https://github.com/andreas-gruenbacher/samba richacl
I've used the following smb.conf:
[richacl]
comment = Richacl directory
path = /mnt/ext4
vfs objects = richacl
writeable = yes
browseable = yes
Is there a particular reason why you didn't make vfs_richacl a
dynamically loadable module?
Thanks,
Andreas
Thi isn't ext4-specific and potentially duplicated in every caller.
Please provide this as a common helper.
Also while we're at it, the mode argument is ignore and the function
always uses inode->i_mode instead.
Shouldn't richacl_from_xattr return the error pointer that ->get_richacl
callers expect?
The xattr representation is the same on disk and at the xattr syscall
layer, and so richacl_from_xattr is used for converting into the
in-memory representation in both cases. The error codes are not the
same when a user supplies an invalid value via setxattr or NFS and
when an invalid xattr is read from disk though. I'll add a parameter
to richacl_from_xattr to make this more explicit.
Should this check for a NULL acl instead of special casing that
in ext4_set_richacl?
I'm not sure I understand what you mean. When iop->set_richacl is
called with a richacl that is mode-equivalent, the file permission
bits need to be updated and any existing acl needs to be removed.
Doing this at the vfs level would result in two calls, iop->setattr
and iop->set_richacl, which can cause problems. To remove an existing
acl without setting the mode, set_richacl is called with a NULL
richacl.
__ext4_set_richacl() was split into __ext4_set_richacl() and
__ext4_remove_richacl() to align with the xfs code due to the
following comment from Dave Chinner:
http://oss.sgi.com/archives/xfs/2015-10/msg00354.html
Diff here:
https://git.kernel.org/cgit/linux/kernel/git/agruen/linux-richacl.git/diff/fs/ext4/richacl.c?id=richacl-2015-10-16&id2=richacl-2015-10-12
Shouldn't richacl_create return NULL if the ACL is equivalent to the
mode bits instead of letting every filesystem figure that out on it's
own?
Hm, that's what it does?
Thanks,
Andreas
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
Thi isn't ext4-specific and potentially duplicated in every caller.
Please provide this as a common helper.
This can go in neither fs.h nor posix_acl.h nor richacl.h unless we
turn it into a macro, and I don't think we want to add a new header
file for such extreme trivia.
Also while we're at it, the mode argument is ignore and the function
always uses inode->i_mode instead.
From: Christoph Hellwig <hch@infradead.org> Date: 2016-03-15 07:11:06
On Fri, Mar 11, 2016 at 09:19:05AM -0500, J. Bruce Fields wrote:
On Fri, Mar 11, 2016 at 06:17:35AM -0800, Christoph Hellwig wrote:
quoted
On Mon, Feb 29, 2016 at 09:17:24AM +0100, Andreas Gruenbacher wrote:
quoted
Add richacl xattr handler implementing the xattr operations based on the
get_richacl and set_richacl inode operations.
Given all the issues with Posix ACLs and selinux attributes these really
should be proper syscalls instead of abusing the xattr interface.
What are those problems exactly?
That people get confused between the attr used by the xattr syscall
interface and the attr used to store things on disk or the protocol.
This has happened every time we have non-native support, e.g. XFS, NFS,
CIFS, ntfs, etc. And it's only going to become worse.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Christoph Hellwig <hch@infradead.org> Date: 2016-03-15 07:11:53
On Fri, Mar 11, 2016 at 05:11:51PM +0100, Andreas Gruenbacher wrote:
quoted
while breaking a lot of assumptions,
The model is designed specifically to be compliant with the POSIX
permission model. What assumptions are you talking about?
People have long learned that we only have 'alloc' permissions. Any
model that mixes allow and deny ACE is a mistake.
quoted
especially by adding allow and deny ACE at the same time.
I remember from past discussions that a permission model like the
POSIX ACL model that doesn't have DENY ACEs would be more to your
liking. This argument is dead from the start though: NFSv4 ACLs
without DENY ACEs cannot represent basic file permissions like 0604
where the owning group has fewer permissions than others, for example
(see the richaclex(7) man page). We would end up with a permission
model that isn't even compatible with the traditional POSIX file
permission model, one which nobody else implements or cares about.
So let's stick to the model that we already have.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Christoph Hellwig <hch@infradead.org> Date: 2016-03-15 07:13:48
On Fri, Mar 11, 2016 at 05:24:45PM +0100, Andreas Gruenbacher wrote:
POSIX ACLs and RichACLs are different objects, with different members
and different algorithms operating on them. The only commonality is
that they are both kmalloc()ed, reference counted objects, and when an
inode is destroyed, both kinds of ACLs can be put in the same way,
avoiding an unnecessary if. What kind of common-code container beyond
that are you still dreaming about?
We still have a main object that is simply a list of ACEs. But if that
doesn't work out (I suspect it should) I don't think the common base
object is a good idea. It just leads to a lot of crazy container_of
calls. If the common object abstraction doesn't work out we'll need
a procedural one instead that has common acl_* calls that decide what
do to based on the file system acl flag.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Christoph Hellwig <hch@infradead.org> Date: 2016-03-15 07:14:39
On Fri, Mar 11, 2016 at 02:05:16PM -0600, Steve French wrote:
A loosely related question is what can be done for tools around existing
interfaces for ACLs. I recently found out NTFS-3g has this xattr:
static const char nf_ns_xattr_ntfs_acl[] = "system.ntfs_acl";
which allows you to query system.ntfs_acl xattr to get their full ACL
Bah. Filesystems really have no business exposing random system xattrs,
and we really need to add a filter to fs/xattr.c to not expose
arbitrary attrs ouside the user.* prefix.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Christoph Hellwig <hch@infradead.org> Date: 2016-03-15 07:17:07
On Mon, Mar 14, 2016 at 12:08:31AM +0100, Andreas Gruenbacher wrote:
The xattr representation is the same on disk and at the xattr syscall
layer, and so richacl_from_xattr is used for converting into the
in-memory representation in both cases. The error codes are not the
same when a user supplies an invalid value via setxattr or NFS and
when an invalid xattr is read from disk though. I'll add a parameter
to richacl_from_xattr to make this more explicit.
Should this check for a NULL acl instead of special casing that
in ext4_set_richacl?
I'm not sure I understand what you mean. When the
ext4_set_richacl checks for a NULL acl pointer and then calls into
__ext4_remove_richacl. I'd rather have that special casing in one
place.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
Thi isn't ext4-specific and potentially duplicated in every caller.
Please provide this as a common helper.
This can go in neither fs.h nor posix_acl.h nor richacl.h unless we
turn it into a macro, and I don't think we want to add a new header
file for such extreme trivia.
I'd expect us to grow a few more of thos helper if we get the sharing
right (either a real common base object, or wrappers for anything
dealing with the acl pointers in the inode), so a new linux/acl.h
should be fine.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: J. Bruce Fields <hidden> Date: 2016-03-15 21:05:28
On Tue, Mar 15, 2016 at 12:10:14AM -0700, Christoph Hellwig wrote:
On Fri, Mar 11, 2016 at 09:19:05AM -0500, J. Bruce Fields wrote:
quoted
On Fri, Mar 11, 2016 at 06:17:35AM -0800, Christoph Hellwig wrote:
quoted
On Mon, Feb 29, 2016 at 09:17:24AM +0100, Andreas Gruenbacher wrote:
quoted
Add richacl xattr handler implementing the xattr operations based on the
get_richacl and set_richacl inode operations.
Given all the issues with Posix ACLs and selinux attributes these really
should be proper syscalls instead of abusing the xattr interface.
What are those problems exactly?
That people get confused between the attr used by the xattr syscall
interface and the attr used to store things on disk or the protocol.
This has happened every time we have non-native support, e.g. XFS, NFS,
CIFS, ntfs, etc. And it's only going to become worse.
How has that confusion caused problems in practice?
--b.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Steve French <smfrench@gmail.com> Date: 2016-03-16 03:40:00
On Tue, Mar 15, 2016 at 2:14 AM, Christoph Hellwig [off-list ref] wrote:
On Fri, Mar 11, 2016 at 02:05:16PM -0600, Steve French wrote:
quoted
A loosely related question is what can be done for tools around existing
interfaces for ACLs. I recently found out NTFS-3g has this xattr:
static const char nf_ns_xattr_ntfs_acl[] = "system.ntfs_acl";
which allows you to query system.ntfs_acl xattr to get their full ACL
Bah. Filesystems really have no business exposing random system xattrs,
and we really need to add a filter to fs/xattr.c to not expose
arbitrary attrs ouside the user.* prefix.
Hopefully we don't consider them random system xattrs, it is
plausible that ntfs uses these for user space tools that I don't
have.
At least for cifs.ko a similar subset (querying ACLs, streams and
reparse info e.g.)
to the ntfs set would be very helpful. For example,
Being able to query the actual ACL over the wire, is important for backup
and for debug, the only question is whether to do it via an xattr (possibly
being able to have some synergy with existing ntfs-3g tools) or an ioctl
(since adding an NTFS specific syscall for a couple fs doesn't make sense).
For the specific example of the odd ntfs.streams.list xattr, I can see why
they have it. I would have mixed feelings about having no way to tell
streams and EAs from each other
since NTFS-3g displaying streams as xattrs and also Extended
Attributes (EAs) as xattrs
(and if they didn't have an additional xattr to list streams)
without a way to tell the difference (at least a system xattr to list
the alternate
data streams is useful). There is useful information in alternate data streams
that backup (and debugging) programs need for some workloads,
for example the origin (where internet explorer downloaded a file from)
and file classification information.
--
Thanks,
Steve
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-03-16 22:31:38
On Tue, Mar 15, 2016 at 8:12 AM, Christoph Hellwig [off-list ref] wrote:
On Fri, Mar 11, 2016 at 05:24:45PM +0100, Andreas Gruenbacher wrote:
quoted
POSIX ACLs and RichACLs are different objects, with different members
and different algorithms operating on them. The only commonality is
that they are both kmalloc()ed, reference counted objects, and when an
inode is destroyed, both kinds of ACLs can be put in the same way,
avoiding an unnecessary if. What kind of common-code container beyond
that are you still dreaming about?
We still have a main object that is simply a list of ACEs. But if that
doesn't work out (I suspect it should) I don't think the common base
object is a good idea. It just leads to a lot of crazy container_of
calls.
There are two such container_of calls for POSIX ACLs in fs/jffs2/acl.c
[which could be replaced by get_acl()], two in fs/posix_acl.c for
POSIX ACLs, and two in fs/richacl.c for RichACLs. That's it.
If the common object abstraction doesn't work out we'll need
a procedural one instead that has common acl_* calls that decide what
do to based on the file system acl flag.
I've already made such abstractions where it made sense; if you can
find more, I don't see why we shouldn't add them.
Thanks,
Andreas
From: Andreas Gruenbacher <agruenba@redhat.com> Date: 2016-03-16 22:38:33
On Tue, Mar 15, 2016 at 8:17 AM, Christoph Hellwig [off-list ref] wrote:
On Mon, Mar 14, 2016 at 12:08:31AM +0100, Andreas Gruenbacher wrote:
quoted
The xattr representation is the same on disk and at the xattr syscall
layer, and so richacl_from_xattr is used for converting into the
in-memory representation in both cases. The error codes are not the
same when a user supplies an invalid value via setxattr or NFS and
when an invalid xattr is read from disk though. I'll add a parameter
to richacl_from_xattr to make this more explicit.
Should this check for a NULL acl instead of special casing that
in ext4_set_richacl?
I'm not sure I understand what you mean. When the
ext4_set_richacl checks for a NULL acl pointer and then calls into
__ext4_remove_richacl. I'd rather have that special casing in one
place.
Those are two different cases: the first is where ext4_set_richacl is
called with a NULL acl to remove an existing ACL; the second is where
ext4_set_richacl is called with a mode-equivalent ACL to set the mode
and remove any existing ACL.
The check for mode-equivalent ACLs is in __ext4_set_richacl and not in
ext4_set_richacl because an inherited ACL (ext4_init_acl) can also be
mode-equivalent.
Andreas
--
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: Christoph Hellwig <hch@infradead.org> Date: 2016-03-21 16:09:22
On Tue, Mar 15, 2016 at 05:05:26PM -0400, J. Bruce Fields wrote:
quoted
That people get confused between the attr used by the xattr syscall
interface and the attr used to store things on disk or the protocol.
This has happened every time we have non-native support, e.g. XFS, NFS,
CIFS, ntfs, etc. And it's only going to become worse.
How has that confusion caused problems in practice?
We had all kinds of bugs in this area that were only slowly uncovered.
We also had all kind of privilegue escalations with (non-ACLs) xattrs
as people never grasped the way different free-form namespaces have
different permission checking.