* [PATCH v2 1/4] fs: document mapping helpers
2021-03-20 12:26 [PATCH v2 0/4] tweak fs mapping helpers Christian Brauner
@ 2021-03-20 12:26 ` Christian Brauner
2021-03-22 7:03 ` Christoph Hellwig
2021-03-22 7:35 ` Matthew Wilcox
2021-03-20 12:26 ` [PATCH v2 2/4] fs: document and rename fsid helpers Christian Brauner
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Christian Brauner @ 2021-03-20 12:26 UTC (permalink / raw)
To: Christoph Hellwig, Al Viro
Cc: Vivek Goyal, Darrick J . Wong, linux-fsdevel, linux-xfs,
Christian Brauner
Document new helpers we introduced this cycle.
Suggested-by: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
patch introduced
- Christoph Hellwig <hch@lst.de>:
- Add kernel docs to helpers.
---
include/linux/fs.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ec8f3ddf4a6a..4795a632ef0d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1574,36 +1574,84 @@ static inline void i_gid_write(struct inode *inode, gid_t gid)
inode->i_gid = make_kgid(inode->i_sb->s_user_ns, gid);
}
+/**
+ * kuid_into_mnt - map a kuid down into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kuid: kuid to be mapped
+ *
+ * Return @kuid mapped according to @mnt_userns.
+ * If @kuid has no mapping INVALID_UID is returned.
+ */
static inline kuid_t kuid_into_mnt(struct user_namespace *mnt_userns,
kuid_t kuid)
{
return make_kuid(mnt_userns, __kuid_val(kuid));
}
+/**
+ * kgid_into_mnt - map a kgid down into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kgid: kgid to be mapped
+ *
+ * Return @kgid mapped according to @mnt_userns.
+ * If @kgid has no mapping INVALID_GID is returned.
+ */
static inline kgid_t kgid_into_mnt(struct user_namespace *mnt_userns,
kgid_t kgid)
{
return make_kgid(mnt_userns, __kgid_val(kgid));
}
+/**
+ * i_uid_into_mnt - map an inode's i_uid down into a mnt_userns
+ * @mnt_userns: user namespace of the mount the inode was found from
+ * @inode: inode to map
+ *
+ * Return the inode's i_uid mapped down according to @mnt_userns.
+ * If the inode's i_uid has no mapping INVALID_UID is returned.
+ */
static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
const struct inode *inode)
{
return kuid_into_mnt(mnt_userns, inode->i_uid);
}
+/**
+ * i_gid_into_mnt - map an inode's i_gid down into a mnt_userns
+ * @mnt_userns: user namespace of the mount the inode was found from
+ * @inode: inode to map
+ *
+ * Return the inode's i_gid mapped down according to @mnt_userns.
+ * If the inode's i_gid has no mapping INVALID_GID is returned.
+ */
static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
const struct inode *inode)
{
return kgid_into_mnt(mnt_userns, inode->i_gid);
}
+/**
+ * kuid_from_mnt - map a kuid up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kuid: kuid to be mapped
+ *
+ * Return @kuid mapped up according to @mnt_userns.
+ * If @kuid has no mapping INVALID_UID is returned.
+ */
static inline kuid_t kuid_from_mnt(struct user_namespace *mnt_userns,
kuid_t kuid)
{
return KUIDT_INIT(from_kuid(mnt_userns, kuid));
}
+/**
+ * kgid_from_mnt - map a kgid up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kgid: kgid to be mapped
+ *
+ * Return @kgid mapped up according to @mnt_userns.
+ * If @kgid has no mapping INVALID_GID is returned.
+ */
static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
kgid_t kgid)
{
--
2.27.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/4] fs: document mapping helpers
2021-03-20 12:26 ` [PATCH v2 1/4] fs: document " Christian Brauner
@ 2021-03-22 7:03 ` Christoph Hellwig
2021-03-22 7:35 ` Matthew Wilcox
1 sibling, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2021-03-22 7:03 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Al Viro, Vivek Goyal, Darrick J . Wong,
linux-fsdevel, linux-xfs
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] fs: document mapping helpers
2021-03-20 12:26 ` [PATCH v2 1/4] fs: document " Christian Brauner
2021-03-22 7:03 ` Christoph Hellwig
@ 2021-03-22 7:35 ` Matthew Wilcox
2021-03-22 8:50 ` Christian Brauner
1 sibling, 1 reply; 11+ messages in thread
From: Matthew Wilcox @ 2021-03-22 7:35 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Al Viro, Vivek Goyal, Darrick J . Wong,
linux-fsdevel, linux-xfs
On Sat, Mar 20, 2021 at 01:26:21PM +0100, Christian Brauner wrote:
> +/**
> + * kuid_into_mnt - map a kuid down into a mnt_userns
> + * @mnt_userns: user namespace of the relevant mount
> + * @kuid: kuid to be mapped
> + *
> + * Return @kuid mapped according to @mnt_userns.
> + * If @kuid has no mapping INVALID_UID is returned.
> + */
If you could just put the ':' after 'Return', htmldoc would put this into
a nice section for you.
I also like to include a Context: section which lists whether the
function takes locks / requires locks to be held / can be called in
hard or soft interrupt context / may sleep / requires refcounts be held /
... Generally, what do you expect from your callers, and what your callers
can expect from you.
I don't understand the thing you're documenting, so it may not make sense
to talk about interrupt context, for example.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] fs: document mapping helpers
2021-03-22 7:35 ` Matthew Wilcox
@ 2021-03-22 8:50 ` Christian Brauner
0 siblings, 0 replies; 11+ messages in thread
From: Christian Brauner @ 2021-03-22 8:50 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Christoph Hellwig, Al Viro, Vivek Goyal, Darrick J . Wong,
linux-fsdevel, linux-xfs
On Mon, Mar 22, 2021 at 07:35:46AM +0000, Matthew Wilcox wrote:
> On Sat, Mar 20, 2021 at 01:26:21PM +0100, Christian Brauner wrote:
> > +/**
> > + * kuid_into_mnt - map a kuid down into a mnt_userns
> > + * @mnt_userns: user namespace of the relevant mount
> > + * @kuid: kuid to be mapped
> > + *
> > + * Return @kuid mapped according to @mnt_userns.
> > + * If @kuid has no mapping INVALID_UID is returned.
> > + */
>
> If you could just put the ':' after 'Return', htmldoc would put this into
> a nice section for you.
I'll fix that up in my tree. Thanks!
>
> I also like to include a Context: section which lists whether the
> function takes locks / requires locks to be held / can be called in
> hard or soft interrupt context / may sleep / requires refcounts be held /
> ... Generally, what do you expect from your callers, and what your callers
> can expect from you.
Thanks for the hint about "Context:". The functions don't take any
locks, they don't require locks to be held and they don't sleep and
don't manipulate refcounts (The lifetime of @mnt_userns is tied to the
respective mnt it's from. It can't be altered anymore once a non-initial
@mnt_userns has been attached to the mnt so it can't go away behind the
caller's back.). Internally only smp_rmb and smp_wmb are used and so
they should be fine to call from soft and hard irq.
Because of that it seemed not explicitly mentioning all that is more
correct then describing all of that. I always thought "Context:"
sections are "Here's things to keep in mind." less then "Here's how it
behaves in all those contexts.", i.e. point out pitfalls, not describe
regular behavior. I might be wrong though or it's a matter of
preference.
(Should we also aim for all other fs.h helpers to have similar comments?)
Christian
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] fs: document and rename fsid helpers
2021-03-20 12:26 [PATCH v2 0/4] tweak fs mapping helpers Christian Brauner
2021-03-20 12:26 ` [PATCH v2 1/4] fs: document " Christian Brauner
@ 2021-03-20 12:26 ` Christian Brauner
2021-03-22 7:04 ` Christoph Hellwig
2021-03-20 12:26 ` [PATCH v2 3/4] fs: introduce fsuidgid_has_mapping() helper Christian Brauner
2021-03-20 12:26 ` [PATCH v2 4/4] fs: introduce two inode i_{u,g}id initialization helpers Christian Brauner
3 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2021-03-20 12:26 UTC (permalink / raw)
To: Christoph Hellwig, Al Viro
Cc: Vivek Goyal, Darrick J . Wong, linux-fsdevel, linux-xfs,
Christian Brauner
Vivek pointed out that the fs{g,u}id_into_mnt() naming scheme can be
misleading as it could be understood as implying they do the exact same
thing as i_{g,u}id_into_mnt(). The original motivation for this naming
scheme was to signal to callers that the helpers will always take care
to map the k{g,u}id such that the ownership is expressed in terms of the
mnt_users.
Get rid of the confusion by renaming those helpers to something more
sensible. Al suggested mapped_fs{g,u}id() which seems a really good fit.
Usually filesystems don't need to bother with these helpers directly
only in some cases where they allocate objects that carry {g,u}ids which
are either filesystem specific (e.g. xfs quota objects) or don't have a
clean set of helpers as inodes have.
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
Inspired-by: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christoph Hellwig <hch@lst.de>:
- Add kernel docs to helpers.
- Al Viro <viro@zeniv.linux.org.uk>:
- s/idmapped_{fsuid,fsgid}/mapped_{fsuid,fsgid}/g
---
fs/ext4/ialloc.c | 2 +-
fs/inode.c | 4 ++--
fs/namei.c | 8 ++++----
fs/xfs/xfs_inode.c | 10 +++++-----
fs/xfs/xfs_symlink.c | 4 ++--
include/linux/fs.h | 28 ++++++++++++++++++++++++++--
6 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 633ae7becd61..d0dc12197346 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -970,7 +970,7 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns,
i_gid_write(inode, owner[1]);
} else if (test_opt(sb, GRPID)) {
inode->i_mode = mode;
- inode->i_uid = fsuid_into_mnt(mnt_userns);
+ inode->i_uid = mapped_fsuid(mnt_userns);
inode->i_gid = dir->i_gid;
} else
inode_init_owner(mnt_userns, inode, dir, mode);
diff --git a/fs/inode.c b/fs/inode.c
index a047ab306f9a..81a6a59b7dd3 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2148,7 +2148,7 @@ EXPORT_SYMBOL(init_special_inode);
void inode_init_owner(struct user_namespace *mnt_userns, struct inode *inode,
const struct inode *dir, umode_t mode)
{
- inode->i_uid = fsuid_into_mnt(mnt_userns);
+ inode->i_uid = mapped_fsuid(mnt_userns);
if (dir && dir->i_mode & S_ISGID) {
inode->i_gid = dir->i_gid;
@@ -2160,7 +2160,7 @@ void inode_init_owner(struct user_namespace *mnt_userns, struct inode *inode,
!capable_wrt_inode_uidgid(mnt_userns, dir, CAP_FSETID))
mode &= ~S_ISGID;
} else
- inode->i_gid = fsgid_into_mnt(mnt_userns);
+ inode->i_gid = mapped_fsgid(mnt_userns);
inode->i_mode = mode;
}
EXPORT_SYMBOL(inode_init_owner);
diff --git a/fs/namei.c b/fs/namei.c
index 216f16e74351..6b5424d34962 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2830,8 +2830,8 @@ static inline int may_create(struct user_namespace *mnt_userns,
if (IS_DEADDIR(dir))
return -ENOENT;
s_user_ns = dir->i_sb->s_user_ns;
- if (!kuid_has_mapping(s_user_ns, fsuid_into_mnt(mnt_userns)) ||
- !kgid_has_mapping(s_user_ns, fsgid_into_mnt(mnt_userns)))
+ if (!kuid_has_mapping(s_user_ns, mapped_fsuid(mnt_userns)) ||
+ !kgid_has_mapping(s_user_ns, mapped_fsgid(mnt_userns)))
return -EOVERFLOW;
return inode_permission(mnt_userns, dir, MAY_WRITE | MAY_EXEC);
}
@@ -3040,8 +3040,8 @@ static int may_o_create(struct user_namespace *mnt_userns,
return error;
s_user_ns = dir->dentry->d_sb->s_user_ns;
- if (!kuid_has_mapping(s_user_ns, fsuid_into_mnt(mnt_userns)) ||
- !kgid_has_mapping(s_user_ns, fsgid_into_mnt(mnt_userns)))
+ if (!kuid_has_mapping(s_user_ns, mapped_fsuid(mnt_userns)) ||
+ !kgid_has_mapping(s_user_ns, mapped_fsgid(mnt_userns)))
return -EOVERFLOW;
error = inode_permission(mnt_userns, dir->dentry->d_inode,
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index f93370bd7b1e..dc91f8c34d35 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -812,7 +812,7 @@ xfs_init_new_inode(
if (dir && !(dir->i_mode & S_ISGID) &&
(mp->m_flags & XFS_MOUNT_GRPID)) {
- inode->i_uid = fsuid_into_mnt(mnt_userns);
+ inode->i_uid = mapped_fsuid(mnt_userns);
inode->i_gid = dir->i_gid;
inode->i_mode = mode;
} else {
@@ -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, mapped_fsuid(mnt_userns),
+ mapped_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, mapped_fsuid(mnt_userns),
+ mapped_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..63edb4dbed4a 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, mapped_fsuid(mnt_userns),
+ mapped_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 4795a632ef0d..c8603969d21f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1658,12 +1658,36 @@ 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)
+/**
+ * mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ *
+ * Return the caller's current fsuid mapped up according to @mnt_userns.
+ *
+ * Use this helper to initialize a new vfs or filesystem object based on
+ * the caller's fsuid. A common example is initializing the i_uid field of
+ * a newly allocated inode triggered by a creation event such as mkdir or
+ * O_CREAT. Other examples include the allocation of quotas for a specific
+ * user.
+ */
+static inline kuid_t mapped_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)
+/**
+ * mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ *
+ * Return the caller's current fsgid mapped up according to @mnt_userns.
+ *
+ * Use this helper to initialize a new vfs or filesystem object based on
+ * the caller's fsgid. A common example is initializing the i_gid field of
+ * a newly allocated inode triggered by a creation event such as mkdir or
+ * O_CREAT. Other examples include the allocation of quotas for a specific
+ * user.
+ */
+static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
{
return kgid_from_mnt(mnt_userns, current_fsgid());
}
--
2.27.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 3/4] fs: introduce fsuidgid_has_mapping() helper
2021-03-20 12:26 [PATCH v2 0/4] tweak fs mapping helpers Christian Brauner
2021-03-20 12:26 ` [PATCH v2 1/4] fs: document " Christian Brauner
2021-03-20 12:26 ` [PATCH v2 2/4] fs: document and rename fsid helpers Christian Brauner
@ 2021-03-20 12:26 ` Christian Brauner
2021-03-22 7:04 ` Christoph Hellwig
2021-03-20 12:26 ` [PATCH v2 4/4] fs: introduce two inode i_{u,g}id initialization helpers Christian Brauner
3 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2021-03-20 12:26 UTC (permalink / raw)
To: Christoph Hellwig, Al Viro
Cc: Vivek Goyal, Darrick J . Wong, linux-fsdevel, linux-xfs,
Christian Brauner
Don't open-code the checks and instead move them into a clean little
helper we can call. This also reduces the risk that if we ever change
something we forget to change all locations.
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Inspired-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christoph Hellwig <hch@lst.de>:
- Add kernel docs to helpers.
---
fs/namei.c | 11 +++--------
include/linux/fs.h | 20 ++++++++++++++++++++
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 6b5424d34962..bc03cbc37ba7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2823,16 +2823,14 @@ static int may_delete(struct user_namespace *mnt_userns, struct inode *dir,
static inline int may_create(struct user_namespace *mnt_userns,
struct inode *dir, struct dentry *child)
{
- struct user_namespace *s_user_ns;
audit_inode_child(dir, child, AUDIT_TYPE_CHILD_CREATE);
if (child->d_inode)
return -EEXIST;
if (IS_DEADDIR(dir))
return -ENOENT;
- s_user_ns = dir->i_sb->s_user_ns;
- if (!kuid_has_mapping(s_user_ns, mapped_fsuid(mnt_userns)) ||
- !kgid_has_mapping(s_user_ns, mapped_fsgid(mnt_userns)))
+ if (!fsuidgid_has_mapping(dir->i_sb, mnt_userns))
return -EOVERFLOW;
+
return inode_permission(mnt_userns, dir, MAY_WRITE | MAY_EXEC);
}
@@ -3034,14 +3032,11 @@ static int may_o_create(struct user_namespace *mnt_userns,
const struct path *dir, struct dentry *dentry,
umode_t mode)
{
- struct user_namespace *s_user_ns;
int error = security_path_mknod(dir, dentry, mode, 0);
if (error)
return error;
- s_user_ns = dir->dentry->d_sb->s_user_ns;
- if (!kuid_has_mapping(s_user_ns, mapped_fsuid(mnt_userns)) ||
- !kgid_has_mapping(s_user_ns, mapped_fsgid(mnt_userns)))
+ if (!fsuidgid_has_mapping(dir->dentry->d_sb, mnt_userns))
return -EOVERFLOW;
error = inode_permission(mnt_userns, dir->dentry->d_inode,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c8603969d21f..0e2ce21b2552 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1692,6 +1692,26 @@ static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
return kgid_from_mnt(mnt_userns, current_fsgid());
}
+/**
+ * fsuidgid_has_mapping() - check whether caller's fsuid/fsgid is mapped
+ * @sb: the superblock we want a mapping in
+ * @mnt_userns: user namespace of the relevant mount
+ *
+ * Check whether the caller's fsuid and fsgid have a valid mapping in the
+ * s_user_ns of the superblock @sb. If the caller is on an idmapped mount map
+ * the caller's fsuid and fsgid according to the @mnt_userns first.
+ *
+ * Returns true if fsuid and fsgid is mapped, false if not.
+ */
+static inline bool fsuidgid_has_mapping(struct super_block *sb,
+ struct user_namespace *mnt_userns)
+{
+ struct user_namespace *s_user_ns = sb->s_user_ns;
+
+ return kuid_has_mapping(s_user_ns, mapped_fsuid(mnt_userns)) &&
+ kgid_has_mapping(s_user_ns, mapped_fsgid(mnt_userns));
+}
+
extern struct timespec64 current_time(struct inode *inode);
/*
--
2.27.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 3/4] fs: introduce fsuidgid_has_mapping() helper
2021-03-20 12:26 ` [PATCH v2 3/4] fs: introduce fsuidgid_has_mapping() helper Christian Brauner
@ 2021-03-22 7:04 ` Christoph Hellwig
0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2021-03-22 7:04 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Al Viro, Vivek Goyal, Darrick J . Wong,
linux-fsdevel, linux-xfs
On Sat, Mar 20, 2021 at 01:26:23PM +0100, Christian Brauner wrote:
> Don't open-code the checks and instead move them into a clean little
> helper we can call. This also reduces the risk that if we ever change
> something we forget to change all locations.
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] fs: introduce two inode i_{u,g}id initialization helpers
2021-03-20 12:26 [PATCH v2 0/4] tweak fs mapping helpers Christian Brauner
` (2 preceding siblings ...)
2021-03-20 12:26 ` [PATCH v2 3/4] fs: introduce fsuidgid_has_mapping() helper Christian Brauner
@ 2021-03-20 12:26 ` Christian Brauner
2021-03-22 7:05 ` Christoph Hellwig
3 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2021-03-20 12:26 UTC (permalink / raw)
To: Christoph Hellwig, Al Viro
Cc: Vivek Goyal, Darrick J . Wong, linux-fsdevel, linux-xfs,
Christian Brauner
Give filesystem two little helpers that do the right thing when
initializing the i_uid and i_gid fields on idmapped and non-idmapped
mounts. Filesystems shouldn't have to be concerned with too many
details.
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Inspired-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- Add kernel docs to helpers.
---
fs/ext4/ialloc.c | 2 +-
fs/inode.c | 4 ++--
fs/xfs/xfs_inode.c | 2 +-
include/linux/fs.h | 28 ++++++++++++++++++++++++++++
4 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index d0dc12197346..755a68bb7e22 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -970,7 +970,7 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns,
i_gid_write(inode, owner[1]);
} else if (test_opt(sb, GRPID)) {
inode->i_mode = mode;
- inode->i_uid = mapped_fsuid(mnt_userns);
+ inode_fsuid_set(inode, mnt_userns);
inode->i_gid = dir->i_gid;
} else
inode_init_owner(mnt_userns, inode, dir, mode);
diff --git a/fs/inode.c b/fs/inode.c
index 81a6a59b7dd3..21c5a620ca89 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2148,7 +2148,7 @@ EXPORT_SYMBOL(init_special_inode);
void inode_init_owner(struct user_namespace *mnt_userns, struct inode *inode,
const struct inode *dir, umode_t mode)
{
- inode->i_uid = mapped_fsuid(mnt_userns);
+ inode_fsuid_set(inode, mnt_userns);
if (dir && dir->i_mode & S_ISGID) {
inode->i_gid = dir->i_gid;
@@ -2160,7 +2160,7 @@ void inode_init_owner(struct user_namespace *mnt_userns, struct inode *inode,
!capable_wrt_inode_uidgid(mnt_userns, dir, CAP_FSETID))
mode &= ~S_ISGID;
} else
- inode->i_gid = mapped_fsgid(mnt_userns);
+ inode_fsgid_set(inode, mnt_userns);
inode->i_mode = mode;
}
EXPORT_SYMBOL(inode_init_owner);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index dc91f8c34d35..2a8bdf33e6c4 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -812,7 +812,7 @@ xfs_init_new_inode(
if (dir && !(dir->i_mode & S_ISGID) &&
(mp->m_flags & XFS_MOUNT_GRPID)) {
- inode->i_uid = mapped_fsuid(mnt_userns);
+ inode_fsuid_set(inode, mnt_userns);
inode->i_gid = dir->i_gid;
inode->i_mode = mode;
} else {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 0e2ce21b2552..4a4af6c26a01 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1692,6 +1692,34 @@ static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
return kgid_from_mnt(mnt_userns, current_fsgid());
}
+/**
+ * inode_fsuid_set - initialize inode's i_uid field with callers fsuid
+ * @inode: inode to initialize
+ * @mnt_userns: user namespace of the mount the inode was found from
+ *
+ * Initialize the i_uid field of @inode. If the inode was found/created via
+ * an idmapped mount map the caller's fsuid according to @mnt_users.
+ */
+static inline void inode_fsuid_set(struct inode *inode,
+ struct user_namespace *mnt_userns)
+{
+ inode->i_uid = mapped_fsuid(mnt_userns);
+}
+
+/**
+ * inode_fsgid_set - initialize inode's i_gid field with callers fsgid
+ * @inode: inode to initialize
+ * @mnt_userns: user namespace of the mount the inode was found from
+ *
+ * Initialize the i_gid field of @inode. If the inode was found/created via
+ * an idmapped mount map the caller's fsgid according to @mnt_users.
+ */
+static inline void inode_fsgid_set(struct inode *inode,
+ struct user_namespace *mnt_userns)
+{
+ inode->i_gid = mapped_fsgid(mnt_userns);
+}
+
/**
* fsuidgid_has_mapping() - check whether caller's fsuid/fsgid is mapped
* @sb: the superblock we want a mapping in
--
2.27.0
^ permalink raw reply related [flat|nested] 11+ messages in thread