XFS removes sgid bits of subdirectories created under a directory containing a default acl. When a default acl is set, it implies xfs to call xfs_setattr_nonsize() in its code path. Such function is shared among mkdir and chmod system calls, and does some checks unneeded by mkdir (calling inode_change_ok()). Such checks remove sgid bit from the inode after it has been granted. So, this patch wraps up these checks to be used only in chmod path. Also, chmod should not remove sgid bit from an inode if this is a directory, so, it adds a check if S_ISDIR is set in the inode mode, into xfs_setattr_nonsize() Done that, xfs_setattr_mode() doesn't need to re-check for group id and capabilities permissions, this only implies in another try to remove sgid bit from the directories. Such check is already done either on inode_change_ok() or xfs_setattr_nonsize(). This addresses SGI bug 280: http://oss.sgi.com/bugzilla/show_bug.cgi?id=280 Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx> --- fs/xfs/xfs_iops.c | 51 ++++++++++++++++++++++++++++++++++----------------- 1 files changed, 34 insertions(+), 17 deletions(-) diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index ca9ecaa..5c9c505 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -467,9 +467,6 @@ xfs_setattr_mode( ASSERT(tp); ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); - if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) - mode &= ~S_ISGID; - ip->i_d.di_mode &= S_IFMT; ip->i_d.di_mode |= mode & ~S_IFMT; @@ -477,23 +474,20 @@ xfs_setattr_mode( inode->i_mode |= mode & ~S_IFMT; } +/* Check inode permissions and filesystem errors + * + * Wrapper to some pre-checks needed while changing + * inode attributes + */ int -xfs_setattr_nonsize( +xfs_setattr_setup( struct xfs_inode *ip, struct iattr *iattr, int flags) { xfs_mount_t *mp = ip->i_mount; struct inode *inode = VFS_I(ip); - int mask = iattr->ia_valid; - xfs_trans_t *tp; int error; - uid_t uid = 0, iuid = 0; - gid_t gid = 0, igid = 0; - struct xfs_dquot *udqp = NULL, *gdqp = NULL; - struct xfs_dquot *olddquot1 = NULL, *olddquot2 = NULL; - - trace_xfs_setattr(ip); if (mp->m_flags & XFS_MOUNT_RDONLY) return XFS_ERROR(EROFS); @@ -505,6 +499,27 @@ xfs_setattr_nonsize( if (error) return XFS_ERROR(error); + return xfs_setattr_nonsize(ip, iattr, flags); +} + +int +xfs_setattr_nonsize( + struct xfs_inode *ip, + struct iattr *iattr, + int flags) +{ + xfs_mount_t *mp = ip->i_mount; + struct inode *inode = VFS_I(ip); + int mask = iattr->ia_valid; + xfs_trans_t *tp; + int error; + uid_t uid = 0, iuid = 0; + gid_t gid = 0, igid = 0; + struct xfs_dquot *udqp = NULL, *gdqp = NULL; + struct xfs_dquot *olddquot1 = NULL, *olddquot2 = NULL; + + trace_xfs_setattr(ip); + ASSERT((mask & ATTR_SIZE) == 0); /* @@ -592,11 +607,13 @@ xfs_setattr_nonsize( * CAP_FSETID overrides the following restrictions: * * The set-user-ID and set-group-ID bits of a file will be - * cleared upon successful return from chown() + * cleared upon successful return from chown(). + * Of a directory, these bits should be kept. */ - if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) && - !capable(CAP_FSETID)) - ip->i_d.di_mode &= ~(S_ISUID|S_ISGID); + if (!S_ISDIR(inode->i_mode)) + if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) && + !capable(CAP_FSETID)) + ip->i_d.di_mode &= ~(S_ISUID|S_ISGID); /* * Change the ownerships and register quota modifications @@ -915,7 +932,7 @@ xfs_vn_setattr( { if (iattr->ia_valid & ATTR_SIZE) return -xfs_setattr_size(XFS_I(dentry->d_inode), iattr, 0); - return -xfs_setattr_nonsize(XFS_I(dentry->d_inode), iattr, 0); + return -xfs_setattr_setup(XFS_I(dentry->d_inode), iattr, 0); } STATIC int -- 1.7.1 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
