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

Dave Chinner
2021-01-17T21:07:41Z

On Thu, Jan 14, 2021 at 11:10:48PM +0100, Christian Brauner wrote:
> 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@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@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.

Yes, I understand the code path, that's why I asked the question and
commented that it's a landmine. That is, if in future we ever need
to do anything that is is in any way namespace related in the
truncate path, the wrong thing will happen because we are passing
the wrong namespace into that function.

Please just pass down the correct namespace for the operation even
though we don't currently require it for the operations being
performed in that path.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com