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@xxxxxx>
> >
> > 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@xxxxxx>
> ....
> > @@ -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
