* [PATCH v2 0/4] tweak fs mapping helpers
@ 2021-03-20 12:26 Christian Brauner
2021-03-20 12:26 ` [PATCH v2 1/4] fs: document " Christian Brauner
` (3 more replies)
0 siblings, 4 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
Hey,
/* v2 */
Add some kernel docs to helpers as suggested by Christoph.
Switch to Al's naming proposal for these helpers.
(Added Acks.)
This little series tries to improve naming and developer friendliness of
fs idmapping helpers triggered by a request/comment from Vivek.
Let's remove the two open-coded checks for whether there's a mapping for
fsuid/fsgid in the s_user_ns of the underlying filesystem. Instead move them
into a tiny helper, getting rid of redundancy and making sure that if we ever
change something it's changed in all places. Also add two helpers to initialize
and inode's i_uid and i_gid fields taking into account idmapped mounts making
it easier for fs developers.
The xfstests I sent out all pass for both xfs and ext4:
#### xfs
1. Detached mount propagation
ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check generic/631
FSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021
MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1
MOUNT_OPTIONS -- /dev/loop1 /mnt/scratch
generic/631 9s ... 11s
Ran: generic/631
Passed all 1 tests
2. Idmapped mounts test-suite
ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check generic/632
FSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021
MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1
MOUNT_OPTIONS -- /dev/loop1 /mnt/scratch
generic/632 13s ... 14s
Ran: generic/632
Passed all 1 tests
3. Testing xfs quotas can't be exceeded/work correctly from idmapped mounts
ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check xfs/529
FSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021
MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1
MOUNT_OPTIONS -- /dev/loop1 /mnt/scratch
xfs/529 42s ... 44s
Ran: xfs/529
Passed all 1 tests
4. Testing xfs qutoas on idmapped mounts
ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check xfs/530
hFSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021
MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1
MOUNT_OPTIONS -- /dev/loop1 /mnt/scratch
xfs/530 20s ... 20s
Ran: xfs/530
Passed all 1 tests
#### ext4
1. Detached mount propagation
ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check generic/631
FSTYP -- ext4
PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021
MKFS_OPTIONS -- /dev/loop1
MOUNT_OPTIONS -- -o acl,user_xattr /dev/loop1 /mnt/scratch
generic/631 11s ... 8s
Ran: generic/631
Passed all 1 tests
2. Idmapped mounts test-suite
ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check generic/632
FSTYP -- ext4
PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021
MKFS_OPTIONS -- /dev/loop1
MOUNT_OPTIONS -- -o acl,user_xattr /dev/loop1 /mnt/scratch
generic/632 14s ... 10s
Ran: generic/632
Passed all 1 tests
3. Testing xfs quotas can't be exceeded/work correctly from idmapped mounts
ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check xfs/529
FSTYP -- ext4
PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021
MKFS_OPTIONS -- /dev/loop1
MOUNT_OPTIONS -- -o acl,user_xattr /dev/loop1 /mnt/scratch
xfs/529 44s ... [not run] not suitable for this filesystem type: ext4
Ran: xfs/529
Not run: xfs/529
Passed all 1 tests
4. Testing xfs qutoas on idmapped mounts
ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check xfs/530
FSTYP -- ext4
PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021
MKFS_OPTIONS -- /dev/loop1
MOUNT_OPTIONS -- -o acl,user_xattr /dev/loop1 /mnt/scratch
xfs/530 20s ... [not run] not suitable for this filesystem type: ext4
Ran: xfs/530
Not run: xfs/530
Passed all 1 tests
Thanks!
Christian
Christian Brauner (4):
fs: document mapping helpers
fs: document and rename fsid helpers
fs: introduce fsuidgid_has_mapping() helper
fs: introduce two inode i_{u,g}id initialization helpers
fs/ext4/ialloc.c | 2 +-
fs/inode.c | 4 +-
fs/namei.c | 11 ++--
fs/xfs/xfs_inode.c | 10 ++--
fs/xfs/xfs_symlink.c | 4 +-
include/linux/fs.h | 124 ++++++++++++++++++++++++++++++++++++++++++-
6 files changed, 135 insertions(+), 20 deletions(-)
base-commit: 8b12a62a4e3ed4ae99c715034f557eb391d6b196
--
2.27.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [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
* [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
* [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
* 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 2/4] fs: document and rename fsid helpers
2021-03-20 12:26 ` [PATCH v2 2/4] fs: document and rename fsid helpers 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
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [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
* Re: [PATCH v2 4/4] fs: introduce two inode i_{u,g}id initialization helpers
2021-03-20 12:26 ` [PATCH v2 4/4] fs: introduce two inode i_{u,g}id initialization helpers Christian Brauner
@ 2021-03-22 7:05 ` Christoph Hellwig
0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2021-03-22 7:05 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
end of thread, other threads:[~2021-03-22 8:51 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-22 7:03 ` Christoph Hellwig
2021-03-22 7:35 ` Matthew Wilcox
2021-03-22 8:50 ` Christian Brauner
2021-03-20 12:26 ` [PATCH v2 2/4] fs: document and rename fsid helpers 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-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
2021-03-22 7:05 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.