[PATCH 1/5] xfs: Propagate dentry down to inode_change_ok()

[Jan Kara]

On Fri 27-05-16 07:53:04, Dave Chinner wrote:
> On Thu, May 26, 2016 at 06:19:56PM +0200, Jan Kara wrote:
> > To avoid clearing of capabilities or security related extended
> > attributes too early, inode_change_ok() will need to take dentry instead
> > of inode. Propagate dentry down to functions calling inode_change_ok().
> > This is rather straightforward except for xfs_set_mode() function which
> > does not have dentry easily available. Luckily that function does not
> > call inode_change_ok() anyway so we just have to do a little dance with
> > function prototypes.
> 
> The idea behind the change is good, but I think the little dance
> could be improved as it makes the layering of the code seem weirdly
> unbalanced to me. e.g.
> 
> xfs_vn_setattr()
>   xfs_vn_setattr_size()		<<<< inode_change_ok() here
> 
> xfs_vn_setattr()
>   xfs_vn_setattr_nonsize()	<<<< inode_change_ok() here
>     xfs_setattr_nonsize()
> 
> xfs_vn_setattr()
>   xfs_vn_setattr_size()
>     xfs_vn_setattr_nonsize()	<<<< inode_change_ok() here
>       xfs_setattr_nonsize()
> 
> And to be more confusing, the externally callable functions for the
> rest of the XFS code are now xfs_vn_setattr_size() and
> xfs_setattr_nonsize() which now have different calling context
> limitations.
> 
> I think adding a little symmetric make sense. i.e:
> 
> xfs_vn_change_ok(dentry, iattr)
> {
> +	if (mp->m_flags & XFS_MOUNT_RDONLY)
> +		return -EROFS;
> +
> +	if (XFS_FORCED_SHUTDOWN(mp))
> +		return -EIO;
> +
> +	error = inode_change_ok(inode, iattr);
> +	if (error)
> +		return error;
> +
> }
> 
> xfs_vn_setattr_size(d, i)
> {
> 	xfs_vn_change_ok(d, i)
> 	xfs_setattr_size(ip, i)
> }
> 
> xfs_vn_setattr_nonsize(d, i)
> {
> 	xfs_vn_change_ok(d, i)
> 	xfs_setattr_nonsize(ip, i)
> }
> 
> xfs_vn_setattr(d, i)
> {
> 	xfs_vn_change_ok(d, i)
> 	<rest of xfs_vn_setattr unchanged>
> }
> 
> And remove the inode_change_ok() code from xfs_setattr_size and
> xfs_setattr_nonsize() completely.  You've already done this with
> xfs_vn_setattr_nonsize() - it just needs to be made symmetric to
> keep a clean layering between VFS interfaces and internal XFS
> interfaces...

Ok, something like attached patch?

								Honza
-- 
Jan Kara <jack at suse.com>
SUSE Labs, CR
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-xfs-Propagate-dentry-down-to-inode_change_ok.patch
Type: text/x-patch
Size: 6336 bytes
Desc: not available
URL: <http://oss.sgi.com/pipermail/xfs/attachments/20160527/c8201387/attachment.patch>