| Message ID | 20210315145419.2612537-3-christian.brauner@ubuntu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tweak idmap helpers | expand |
> -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns) > +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns) > { > return kuid_from_mnt(mnt_userns, current_fsuid()); > } > > -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns) > +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns) > { > return kgid_from_mnt(mnt_userns, current_fsgid()); > } I'm not sure the naming is an improvement. I always think of identity mapped when reading it, which couldn't be further from what it does.. But either way comments describing what these helpers do would be very useful.
On Thu, Mar 18, 2021 at 07:27:23AM +0100, Christoph Hellwig wrote: > > -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns) > > +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns) > > { > > return kuid_from_mnt(mnt_userns, current_fsuid()); > > } > > > > -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns) > > +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns) > > { > > return kgid_from_mnt(mnt_userns, current_fsgid()); > > } > > I'm not sure the naming is an improvement. I always think of > identity mapped when reading it, which couldn't be further from what > it does.. But either way comments describing what these helpers do > would be very useful. s/idmapped/mapped/?
On Thu, Mar 18, 2021 at 06:30:50AM +0000, Al Viro wrote: > On Thu, Mar 18, 2021 at 07:27:23AM +0100, Christoph Hellwig wrote: > > > -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns) > > > +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns) > > > { > > > return kuid_from_mnt(mnt_userns, current_fsuid()); > > > } > > > > > > -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns) > > > +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns) > > > { > > > return kgid_from_mnt(mnt_userns, current_fsgid()); > > > } > > > > I'm not sure the naming is an improvement. I always think of > > identity mapped when reading it, which couldn't be further from what > > it does.. But either way comments describing what these helpers do > > would be very useful. > > s/idmapped/mapped/? Yeah, I think that's a good idea. I'll also add comments.
On Thu, Mar 18, 2021 at 07:27:23AM +0100, Christoph Hellwig wrote: > > -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns) > > +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns) > > { > > return kuid_from_mnt(mnt_userns, current_fsuid()); > > } > > > > -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns) > > +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns) > > { > > return kgid_from_mnt(mnt_userns, current_fsgid()); > > } > > I'm not sure the naming is an improvement. I always think of > identity mapped when reading it, which couldn't be further from what > it does.. But either way comments describing what these helpers do > would be very useful. Good point. I'll add comments!
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index f93370bd7b1e..8703408bd1aa 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -1007,8 +1007,8 @@ xfs_create( /* * Make sure that we have allocated dquot(s) on disk. */ - error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), - fsgid_into_mnt(mnt_userns), prid, + error = xfs_qm_vop_dqalloc(dp, idmapped_fsuid(mnt_userns), + idmapped_fsgid(mnt_userns), prid, XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp, &pdqp); if (error) @@ -1158,8 +1158,8 @@ xfs_create_tmpfile( /* * Make sure that we have allocated dquot(s) on disk. */ - error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), - fsgid_into_mnt(mnt_userns), prid, + error = xfs_qm_vop_dqalloc(dp, idmapped_fsuid(mnt_userns), + idmapped_fsgid(mnt_userns), prid, XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp, &pdqp); if (error) diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c index 7f368b10ded1..669e8517e2e1 100644 --- a/fs/xfs/xfs_symlink.c +++ b/fs/xfs/xfs_symlink.c @@ -182,8 +182,8 @@ xfs_symlink( /* * Make sure that we have allocated dquot(s) on disk. */ - error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns), - fsgid_into_mnt(mnt_userns), prid, + error = xfs_qm_vop_dqalloc(dp, idmapped_fsuid(mnt_userns), + idmapped_fsgid(mnt_userns), prid, XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp, &pdqp); if (error) diff --git a/include/linux/fs.h b/include/linux/fs.h index edcb1aa99fd6..189673721726 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1610,12 +1610,12 @@ static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns, return KGIDT_INIT(from_kgid(mnt_userns, kgid)); } -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns) +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns) { return kuid_from_mnt(mnt_userns, current_fsuid()); } -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns) +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns) { return kgid_from_mnt(mnt_userns, current_fsgid()); }
Vivek pointed out that the current naming scheme can be misleading as it conflicts with some of the other helpers naming. So get rid of the confusion by simply calling those helpers idmapped_fs{u,g}id() that make it very clear that and idmapped fsuid/fsgid is used. xfs needs to use them directly in the quota allocation codepaths. Inspired-by: Vivek Goyal <vgoyal@redhat.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Darrick J. Wong <djwong@kernel.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- fs/xfs/xfs_inode.c | 8 ++++---- fs/xfs/xfs_symlink.c | 4 ++-- include/linux/fs.h | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-)