Hey Carlos, On Wed, Jul 03, 2013 at 12:24:12PM -0300, Carlos Maiolino wrote: > Hi, any comments on this one? This goes with generic/313, right? http://oss.sgi.com/archives/xfs/2013-05/msg00957.html Thanks, Ben > 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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
