Hi, any comments on this one? On Fri, Jun 21, 2013 at 02:45:53PM -0300, Carlos Maiolino wrote: > XFS removes sgid bits of subdirectories 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. > > With this patch, we extend the meaning of XFS_ATTR_NOACL flag to avoid these > checks when acls are being inherited (thanks hch). > > Also, 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(). > > Changelog: > > V2: Extends the meaning of XFS_ATTR_NOACL instead of wrap the tests into another > function > > V3: Remove S_ISDIR check in xfs_setattr_nonsize() from the patch > > Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx> > --- > fs/xfs/xfs_iops.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > index ca9ecaa..2e5aca8 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; > > @@ -495,15 +492,18 @@ xfs_setattr_nonsize( > > trace_xfs_setattr(ip); > > - if (mp->m_flags & XFS_MOUNT_RDONLY) > - return XFS_ERROR(EROFS); > + /* If acls are being inherited, we already have this checked */ > + if (!(flags & XFS_ATTR_NOACL)) { > + if (mp->m_flags & XFS_MOUNT_RDONLY) > + return XFS_ERROR(EROFS); > > - if (XFS_FORCED_SHUTDOWN(mp)) > - return XFS_ERROR(EIO); > + if (XFS_FORCED_SHUTDOWN(mp)) > + return XFS_ERROR(EIO); > > - error = -inode_change_ok(inode, iattr); > - if (error) > - return XFS_ERROR(error); > + error = -inode_change_ok(inode, iattr); > + if (error) > + return XFS_ERROR(error); > + } > > ASSERT((mask & ATTR_SIZE) == 0); > > -- > 1.8.1.4 > > _______________________________________________ > xfs mailing list > xfs@xxxxxxxxxxx > http://oss.sgi.com/mailman/listinfo/xfs -- Carlos _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs