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

Christoph Hellwig
2021-01-15T16:24:21Z

On Fri, Jan 15, 2021 at 07:51:54AM +1100, Dave Chinner wrote:
> > @@ -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?

As Christian already explained we an't hit this with anything related
to uids/gids, the only thing that will be updated are the timestamps,
as also mentioned in the comment that only makes it partially into the
diff context.

> > trace_xfs_setattr(ip);
> >
> > - error = xfs_vn_change_ok(dentry, iattr);
> > + error = xfs_vn_change_ok(mnt_userns, dentry, iattr);
> > if (error)
> > return error;
> > return xfs_setattr_size(ip, iattr);
>
> And this passing mnt_userns down into xfs_setattr_size()? Seems
> like a bit of a landmine...

That being said we could just pass down the argument, even if it doesn't
make much sense for the size update.