* [PATCH 0/11 v1] Fix inheritance of SGID in presence of default ACLs @ 2017-06-22 13:31 Jan Kara 2017-06-22 13:31 ` [PATCH 11/11] xfs: Don't clear SGID when inheriting ACLs Jan Kara 0 siblings, 1 reply; 3+ messages in thread From: Jan Kara @ 2017-06-22 13:31 UTC (permalink / raw) To: linux-fsdevel Cc: Andreas Gruenbacher, Jan Kara, Darrick J . Wong, linux-xfs, reiserfs-devel, Mike Marshall, pvfs2-developers, Joel Becker, ocfs2-devel, Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson, linux-btrfs, David Sterba, linux-ext4, Theodore Ts'o Hello, this patch set fixes a problem introduced by commit 073931017b49 "posix_acl: Clear SGID bit when setting file permissions". The problem is that when new directory 'DIR1' is created in a directory 'DIR0' with SGID bit set, DIR1 is expected to have SGID bit set (and owning group equal to the owning group of 'DIR0'). However when 'DIR0' also has some default ACLs that 'DIR1' inherits, setting these ACLs will result in SGID bit on 'DIR1' to get cleared if user is not member of the owning group. The problem is fixed by moving posix_acl_update_mode() so that it does not get called when default ACLs are inherited. I have created new generic/441 test for this and verified that generic/314, generic/375, and generic/441 pass for ext2, ext4, btrfs, xfs, ocfs2, reiserfs. All patches in this series are completely independent so fs maintainers please pick them up as soon as they get reviewed. I'm leaving for three weeks of vacation at the end of the week so I won't be able to push this further for some time. Honza ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 11/11] xfs: Don't clear SGID when inheriting ACLs 2017-06-22 13:31 [PATCH 0/11 v1] Fix inheritance of SGID in presence of default ACLs Jan Kara @ 2017-06-22 13:31 ` Jan Kara 2017-06-26 15:52 ` Darrick J. Wong 0 siblings, 1 reply; 3+ messages in thread From: Jan Kara @ 2017-06-22 13:31 UTC (permalink / raw) To: linux-fsdevel Cc: Andreas Gruenbacher, Jan Kara, stable, Darrick J . Wong, linux-xfs When new directory 'DIR1' is created in a directory 'DIR0' with SGID bit set, DIR1 is expected to have SGID bit set (and owning group equal to the owning group of 'DIR0'). However when 'DIR0' also has some default ACLs that 'DIR1' inherits, setting these ACLs will result in SGID bit on 'DIR1' to get cleared if user is not member of the owning group. Fix the problem by calling __xfs_set_acl() instead of xfs_set_acl() when setting up inode in xfs_generic_create(). That prevents SGID bit clearing and mode is properly set by posix_acl_create() anyway. We also reorder arguments of __xfs_set_acl() to match the ordering of xfs_set_acl() to make things consistent. Fixes: 073931017b49d9458aa351605b43a7e34598caef CC: stable@vger.kernel.org CC: Darrick J. Wong <darrick.wong@oracle.com> CC: linux-xfs@vger.kernel.org Signed-off-by: Jan Kara <jack@suse.cz> --- fs/xfs/xfs_acl.c | 6 +++--- fs/xfs/xfs_acl.h | 1 + fs/xfs/xfs_iops.c | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c index b468e041f207..7034e17535de 100644 --- a/fs/xfs/xfs_acl.c +++ b/fs/xfs/xfs_acl.c @@ -170,8 +170,8 @@ xfs_get_acl(struct inode *inode, int type) return acl; } -STATIC int -__xfs_set_acl(struct inode *inode, int type, struct posix_acl *acl) +int +__xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type) { struct xfs_inode *ip = XFS_I(inode); unsigned char *ea_name; @@ -268,5 +268,5 @@ xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type) } set_acl: - return __xfs_set_acl(inode, type, acl); + return __xfs_set_acl(inode, acl, type); } diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h index 286fa89217f5..04327318ef67 100644 --- a/fs/xfs/xfs_acl.h +++ b/fs/xfs/xfs_acl.h @@ -24,6 +24,7 @@ struct posix_acl; #ifdef CONFIG_XFS_POSIX_ACL extern struct posix_acl *xfs_get_acl(struct inode *inode, int type); extern int xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type); +extern int __xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type); #else static inline struct posix_acl *xfs_get_acl(struct inode *inode, int type) { diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index ebfc13350f9a..077e2b2ac773 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -190,12 +190,12 @@ xfs_generic_create( #ifdef CONFIG_XFS_POSIX_ACL if (default_acl) { - error = xfs_set_acl(inode, default_acl, ACL_TYPE_DEFAULT); + error = __xfs_set_acl(inode, default_acl, ACL_TYPE_DEFAULT); if (error) goto out_cleanup_inode; } if (acl) { - error = xfs_set_acl(inode, acl, ACL_TYPE_ACCESS); + error = __xfs_set_acl(inode, acl, ACL_TYPE_ACCESS); if (error) goto out_cleanup_inode; } -- 2.12.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 11/11] xfs: Don't clear SGID when inheriting ACLs 2017-06-22 13:31 ` [PATCH 11/11] xfs: Don't clear SGID when inheriting ACLs Jan Kara @ 2017-06-26 15:52 ` Darrick J. Wong 0 siblings, 0 replies; 3+ messages in thread From: Darrick J. Wong @ 2017-06-26 15:52 UTC (permalink / raw) To: Jan Kara; +Cc: linux-fsdevel, Andreas Gruenbacher, stable, linux-xfs On Thu, Jun 22, 2017 at 03:31:15PM +0200, Jan Kara wrote: > When new directory 'DIR1' is created in a directory 'DIR0' with SGID bit > set, DIR1 is expected to have SGID bit set (and owning group equal to > the owning group of 'DIR0'). However when 'DIR0' also has some default > ACLs that 'DIR1' inherits, setting these ACLs will result in SGID bit on > 'DIR1' to get cleared if user is not member of the owning group. > > Fix the problem by calling __xfs_set_acl() instead of xfs_set_acl() when > setting up inode in xfs_generic_create(). That prevents SGID bit > clearing and mode is properly set by posix_acl_create() anyway. We also > reorder arguments of __xfs_set_acl() to match the ordering of > xfs_set_acl() to make things consistent. > > Fixes: 073931017b49d9458aa351605b43a7e34598caef > CC: stable@vger.kernel.org > CC: Darrick J. Wong <darrick.wong@oracle.com> > CC: linux-xfs@vger.kernel.org > Signed-off-by: Jan Kara <jack@suse.cz> Looks ok, will pull into xfs tree... Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_acl.c | 6 +++--- > fs/xfs/xfs_acl.h | 1 + > fs/xfs/xfs_iops.c | 4 ++-- > 3 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c > index b468e041f207..7034e17535de 100644 > --- a/fs/xfs/xfs_acl.c > +++ b/fs/xfs/xfs_acl.c > @@ -170,8 +170,8 @@ xfs_get_acl(struct inode *inode, int type) > return acl; > } > > -STATIC int > -__xfs_set_acl(struct inode *inode, int type, struct posix_acl *acl) > +int > +__xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type) > { > struct xfs_inode *ip = XFS_I(inode); > unsigned char *ea_name; > @@ -268,5 +268,5 @@ xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type) > } > > set_acl: > - return __xfs_set_acl(inode, type, acl); > + return __xfs_set_acl(inode, acl, type); > } > diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h > index 286fa89217f5..04327318ef67 100644 > --- a/fs/xfs/xfs_acl.h > +++ b/fs/xfs/xfs_acl.h > @@ -24,6 +24,7 @@ struct posix_acl; > #ifdef CONFIG_XFS_POSIX_ACL > extern struct posix_acl *xfs_get_acl(struct inode *inode, int type); > extern int xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type); > +extern int __xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type); > #else > static inline struct posix_acl *xfs_get_acl(struct inode *inode, int type) > { > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > index ebfc13350f9a..077e2b2ac773 100644 > --- a/fs/xfs/xfs_iops.c > +++ b/fs/xfs/xfs_iops.c > @@ -190,12 +190,12 @@ xfs_generic_create( > > #ifdef CONFIG_XFS_POSIX_ACL > if (default_acl) { > - error = xfs_set_acl(inode, default_acl, ACL_TYPE_DEFAULT); > + error = __xfs_set_acl(inode, default_acl, ACL_TYPE_DEFAULT); > if (error) > goto out_cleanup_inode; > } > if (acl) { > - error = xfs_set_acl(inode, acl, ACL_TYPE_ACCESS); > + error = __xfs_set_acl(inode, acl, ACL_TYPE_ACCESS); > if (error) > goto out_cleanup_inode; > } > -- > 2.12.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-26 15:52 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-22 13:31 [PATCH 0/11 v1] Fix inheritance of SGID in presence of default ACLs Jan Kara 2017-06-22 13:31 ` [PATCH 11/11] xfs: Don't clear SGID when inheriting ACLs Jan Kara 2017-06-26 15:52 ` Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).