| Message ID | 161472410230.3421449.11155796770029064636.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: small fixes for 5.12 | expand |
On Tue, Mar 02, 2021 at 02:28:22PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Nowadays, we indirectly use the idmap-aware helper functions in the VFS > to set the initial uid and gid of a file being created. Unfortunately, > we didn't convert the quota code, which means we attach the wrong dquots > to files created on an idmapped mount. > > Fixes: f736d93d76d3 ("xfs: support idmapped mounts") > Signed-off-by: Darrick J. Wong <djwong@kernel.org> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue, Mar 02, 2021 at 02:28:22PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Nowadays, we indirectly use the idmap-aware helper functions in the VFS > to set the initial uid and gid of a file being created. Unfortunately, > we didn't convert the quota code, which means we attach the wrong dquots > to files created on an idmapped mount. > > Fixes: f736d93d76d3 ("xfs: support idmapped mounts") > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- This looks good but it misses one change, I think. The xfs_ioctl_setattr() needs to take the mapping into account as well: diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 99dfe89a8d08..067366bf9a59 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -1460,8 +1460,8 @@ xfs_ioctl_setattr( * because the i_*dquot fields will get updated anyway. */ if (XFS_IS_QUOTA_ON(mp)) { - error = xfs_qm_vop_dqalloc(ip, VFS_I(ip)->i_uid, - VFS_I(ip)->i_gid, fa->fsx_projid, + error = xfs_qm_vop_dqalloc(ip, i_uid_into_mnt(mnt_userns, VFS_I(ip)), + i_gid_into_mnt(mnt_userns, VFS_I(ip)), fa->fsx_projid, XFS_QMOPT_PQUOTA, NULL, NULL, &pdqp); if (error) return error; This should cover all callsites. Acked-by: Christian Brauner <christian.brauner@ubuntu.com> > fs/xfs/xfs_inode.c | 14 ++++++++------ > fs/xfs/xfs_symlink.c | 3 ++- > 2 files changed, 10 insertions(+), 7 deletions(-) > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index 46a861d55e48..f93370bd7b1e 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -1007,9 +1007,10 @@ xfs_create( > /* > * Make sure that we have allocated dquot(s) on disk. > */ > - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, > - XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > - &udqp, &gdqp, &pdqp); > + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), > + fsgid_into_mnt(mnt_userns), prid, > + XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > + &udqp, &gdqp, &pdqp); > if (error) > return error; > > @@ -1157,9 +1158,10 @@ xfs_create_tmpfile( > /* > * Make sure that we have allocated dquot(s) on disk. > */ > - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, > - XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > - &udqp, &gdqp, &pdqp); > + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), > + fsgid_into_mnt(mnt_userns), prid, > + XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > + &udqp, &gdqp, &pdqp); > if (error) > return error; > > diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c > index 1379013d74b8..7f368b10ded1 100644 > --- a/fs/xfs/xfs_symlink.c > +++ b/fs/xfs/xfs_symlink.c > @@ -182,7 +182,8 @@ xfs_symlink( > /* > * Make sure that we have allocated dquot(s) on disk. > */ > - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, > + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), > + fsgid_into_mnt(mnt_userns), prid, > XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > &udqp, &gdqp, &pdqp); > if (error) >
On Wed, Mar 03, 2021 at 02:44:01PM +0000, Christian Brauner wrote: > On Tue, Mar 02, 2021 at 02:28:22PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Nowadays, we indirectly use the idmap-aware helper functions in the VFS > > to set the initial uid and gid of a file being created. Unfortunately, > > we didn't convert the quota code, which means we attach the wrong dquots > > to files created on an idmapped mount. > > > > Fixes: f736d93d76d3 ("xfs: support idmapped mounts") > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > This looks good but it misses one change, I think. The > xfs_ioctl_setattr() needs to take the mapping into account as well: > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index 99dfe89a8d08..067366bf9a59 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -1460,8 +1460,8 @@ xfs_ioctl_setattr( > * because the i_*dquot fields will get updated anyway. > */ > if (XFS_IS_QUOTA_ON(mp)) { > - error = xfs_qm_vop_dqalloc(ip, VFS_I(ip)->i_uid, > - VFS_I(ip)->i_gid, fa->fsx_projid, > + error = xfs_qm_vop_dqalloc(ip, i_uid_into_mnt(mnt_userns, VFS_I(ip)), > + i_gid_into_mnt(mnt_userns, VFS_I(ip)), fa->fsx_projid, > XFS_QMOPT_PQUOTA, NULL, NULL, &pdqp); The uid/gid parameters here don't matter because the "XFS_QMOPT_PQUOTA" flag by itself here means that _dqalloc only looks at the project id argument. > if (error) > return error; > > This should cover all callsites. > > Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Thanks! --D > > fs/xfs/xfs_inode.c | 14 ++++++++------ > > fs/xfs/xfs_symlink.c | 3 ++- > > 2 files changed, 10 insertions(+), 7 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > > index 46a861d55e48..f93370bd7b1e 100644 > > --- a/fs/xfs/xfs_inode.c > > +++ b/fs/xfs/xfs_inode.c > > @@ -1007,9 +1007,10 @@ xfs_create( > > /* > > * Make sure that we have allocated dquot(s) on disk. > > */ > > - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, > > - XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > > - &udqp, &gdqp, &pdqp); > > + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), > > + fsgid_into_mnt(mnt_userns), prid, > > + XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > > + &udqp, &gdqp, &pdqp); > > if (error) > > return error; > > > > @@ -1157,9 +1158,10 @@ xfs_create_tmpfile( > > /* > > * Make sure that we have allocated dquot(s) on disk. > > */ > > - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, > > - XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > > - &udqp, &gdqp, &pdqp); > > + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), > > + fsgid_into_mnt(mnt_userns), prid, > > + XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > > + &udqp, &gdqp, &pdqp); > > if (error) > > return error; > > > > diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c > > index 1379013d74b8..7f368b10ded1 100644 > > --- a/fs/xfs/xfs_symlink.c > > +++ b/fs/xfs/xfs_symlink.c > > @@ -182,7 +182,8 @@ xfs_symlink( > > /* > > * Make sure that we have allocated dquot(s) on disk. > > */ > > - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, > > + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), > > + fsgid_into_mnt(mnt_userns), prid, > > XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, > > &udqp, &gdqp, &pdqp); > > if (error) > >
On Wed, Mar 03, 2021 at 10:33:52AM -0800, Darrick J. Wong wrote: > On Wed, Mar 03, 2021 at 02:44:01PM +0000, Christian Brauner wrote: > > On Tue, Mar 02, 2021 at 02:28:22PM -0800, Darrick J. Wong wrote: > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > Nowadays, we indirectly use the idmap-aware helper functions in the VFS > > > to set the initial uid and gid of a file being created. Unfortunately, > > > we didn't convert the quota code, which means we attach the wrong dquots > > > to files created on an idmapped mount. > > > > > > Fixes: f736d93d76d3 ("xfs: support idmapped mounts") > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > --- > > > > This looks good but it misses one change, I think. The > > xfs_ioctl_setattr() needs to take the mapping into account as well: > > > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > > index 99dfe89a8d08..067366bf9a59 100644 > > --- a/fs/xfs/xfs_ioctl.c > > +++ b/fs/xfs/xfs_ioctl.c > > @@ -1460,8 +1460,8 @@ xfs_ioctl_setattr( > > * because the i_*dquot fields will get updated anyway. > > */ > > if (XFS_IS_QUOTA_ON(mp)) { > > - error = xfs_qm_vop_dqalloc(ip, VFS_I(ip)->i_uid, > > - VFS_I(ip)->i_gid, fa->fsx_projid, > > + error = xfs_qm_vop_dqalloc(ip, i_uid_into_mnt(mnt_userns, VFS_I(ip)), > > + i_gid_into_mnt(mnt_userns, VFS_I(ip)), fa->fsx_projid, > > XFS_QMOPT_PQUOTA, NULL, NULL, &pdqp); > > The uid/gid parameters here don't matter because the "XFS_QMOPT_PQUOTA" > flag by itself here means that _dqalloc only looks at the project id > argument. Ah, thanks! (Maybe a comment would be good or just idmaped them anyway.) Thanks! Christian
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 46a861d55e48..f93370bd7b1e 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -1007,9 +1007,10 @@ xfs_create( /* * Make sure that we have allocated dquot(s) on disk. */ - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, - XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, - &udqp, &gdqp, &pdqp); + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), + fsgid_into_mnt(mnt_userns), prid, + XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, + &udqp, &gdqp, &pdqp); if (error) return error; @@ -1157,9 +1158,10 @@ xfs_create_tmpfile( /* * Make sure that we have allocated dquot(s) on disk. */ - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, - XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, - &udqp, &gdqp, &pdqp); + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), + fsgid_into_mnt(mnt_userns), prid, + XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, + &udqp, &gdqp, &pdqp); if (error) return error; diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c index 1379013d74b8..7f368b10ded1 100644 --- a/fs/xfs/xfs_symlink.c +++ b/fs/xfs/xfs_symlink.c @@ -182,7 +182,8 @@ xfs_symlink( /* * Make sure that we have allocated dquot(s) on disk. */ - error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid, + error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), + fsgid_into_mnt(mnt_userns), prid, XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp, &pdqp); if (error)