[PATCH v5 37/42] xfs: support idmapped mounts

[Christian Brauner]

On Fri, Jan 15, 2021 at 07:51:54AM +1100, Dave Chinner wrote:
> On Tue, Jan 12, 2021 at 11:01:19PM +0100, Christian Brauner wrote:
> > From: Christoph Hellwig <hch at lst.de>
> > 
> > Enable idmapped mounts for xfs. This basically just means passing down
> > the user_namespace argument from the VFS methods down to where it is
> > passed to helper.
> > 
> > Signed-off-by: Christoph Hellwig <hch at lst.de>
> ....
> > @@ -654,6 +658,7 @@ xfs_vn_change_ok(
> >   */
> >  static int
> >  xfs_setattr_nonsize(
> > +	struct user_namespace	*mnt_userns,
> >  	struct xfs_inode	*ip,
> >  	struct iattr		*iattr)
> >  {
> > @@ -813,7 +818,7 @@ xfs_setattr_nonsize(
> >  	 * 	     Posix ACL code seems to care about this issue either.
> >  	 */
> >  	if (mask & ATTR_MODE) {
> > -		error = posix_acl_chmod(&init_user_ns, inode, inode->i_mode);
> > +		error = posix_acl_chmod(mnt_userns, inode, inode->i_mode);
> >  		if (error)
> >  			return error;
> >  	}
> > @@ -868,7 +873,7 @@ xfs_setattr_size(
> >  		 * Use the regular setattr path to update the timestamps.
> >  		 */
> >  		iattr->ia_valid &= ~ATTR_SIZE;
> > -		return xfs_setattr_nonsize(ip, iattr);
> > +		return xfs_setattr_nonsize(&init_user_ns, ip, iattr);
> 
> Shouldn't that be passing mnt_userns?

Hey Dave,

Thanks for taking a look.

This is the time updating codepath.

xfs_setattr_size();
-> xfs_setattr_nonsize(&init_user_ns);

The xfs_setattr_size() helper will assert:

ASSERT((iattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET|
	ATTR_MTIME_SET|ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0);

While the

xfs_setattr_nonsize() helper will further assert:

ASSERT((mask & ATTR_SIZE) == 0);

so xfs_setattr_nonsize() in this callpath is only used to update the

if (!(iattr->ia_valid & (ATTR_CTIME|ATTR_MTIME)))
	return 0;

so there's no interactions with idmappings in any way. Simply passing
mnt_userns might be clearer though.

But if this would be using the wrong idmapping the xfstest suite I added
would've immediately caught that and failed.

But this specific codepath can also be reliably hit from userspace by
doing ftruncate(fd, 0) so just to be extra sure I added truncate tests
to the xfstests now for both the idmapped and non-idmapped case.

Christian