* [PATCH 1/3] fs: introduce fsuidgid_has_mapping() helper
2021-03-15 14:54 [PATCH 0/3] tweak idmap helpers Christian Brauner
@ 2021-03-15 14:54 ` Christian Brauner
2021-03-18 6:26 ` Christoph Hellwig
2021-03-15 14:54 ` [PATCH 2/3] fs: improve naming for fsid helpers Christian Brauner
2021-03-15 14:54 ` [PATCH 3/3] fs: introduce two little fs{u,g}id inode initialization helpers Christian Brauner
2 siblings, 1 reply; 10+ messages in thread
From: Christian Brauner @ 2021-03-15 14:54 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.
Inspired-by: Vivek Goyal <vgoyal@redhat.com>
Cc: 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>
---
fs/namei.c | 11 +++--------
include/linux/fs.h | 11 +++++++++++
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 216f16e74351..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, fsuid_into_mnt(mnt_userns)) ||
- !kgid_has_mapping(s_user_ns, fsgid_into_mnt(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, fsuid_into_mnt(mnt_userns)) ||
- !kgid_has_mapping(s_user_ns, fsgid_into_mnt(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 ec8f3ddf4a6a..edcb1aa99fd6 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1620,6 +1620,17 @@ static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns)
return kgid_from_mnt(mnt_userns, current_fsgid());
}
+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,
+ kuid_from_mnt(mnt_userns, current_fsuid())) &&
+ kgid_has_mapping(s_user_ns,
+ kgid_from_mnt(mnt_userns, current_fsgid()));
+}
+
extern struct timespec64 current_time(struct inode *inode);
/*
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] fs: introduce fsuidgid_has_mapping() helper
2021-03-15 14:54 ` [PATCH 1/3] fs: introduce fsuidgid_has_mapping() helper Christian Brauner
@ 2021-03-18 6:26 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2021-03-18 6:26 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Al Viro, Vivek Goyal, Darrick J . Wong,
linux-fsdevel, linux-xfs
On Mon, Mar 15, 2021 at 03:54:17PM +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] 10+ messages in thread
* [PATCH 2/3] fs: improve naming for fsid helpers
2021-03-15 14:54 [PATCH 0/3] tweak idmap helpers Christian Brauner
2021-03-15 14:54 ` [PATCH 1/3] fs: introduce fsuidgid_has_mapping() helper Christian Brauner
@ 2021-03-15 14:54 ` Christian Brauner
2021-03-18 6:27 ` Christoph Hellwig
2021-03-15 14:54 ` [PATCH 3/3] fs: introduce two little fs{u,g}id inode initialization helpers Christian Brauner
2 siblings, 1 reply; 10+ messages in thread
From: Christian Brauner @ 2021-03-15 14:54 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 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(-)
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());
}
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: improve naming for fsid helpers
2021-03-15 14:54 ` [PATCH 2/3] fs: improve naming for fsid helpers Christian Brauner
@ 2021-03-18 6:27 ` Christoph Hellwig
2021-03-18 6:30 ` Al Viro
2021-03-18 11:40 ` Christian Brauner
0 siblings, 2 replies; 10+ messages in thread
From: Christoph Hellwig @ 2021-03-18 6:27 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Al Viro, Vivek Goyal, Darrick J . Wong,
linux-fsdevel, linux-xfs
> -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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: improve naming for fsid helpers
2021-03-18 6:27 ` Christoph Hellwig
@ 2021-03-18 6:30 ` Al Viro
2021-03-18 11:40 ` Christian Brauner
2021-03-18 11:40 ` Christian Brauner
1 sibling, 1 reply; 10+ messages in thread
From: Al Viro @ 2021-03-18 6:30 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Christian Brauner, Vivek Goyal, Darrick J . Wong, linux-fsdevel,
linux-xfs
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/?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: improve naming for fsid helpers
2021-03-18 6:30 ` Al Viro
@ 2021-03-18 11:40 ` Christian Brauner
0 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2021-03-18 11:40 UTC (permalink / raw)
To: Al Viro
Cc: Christoph Hellwig, Vivek Goyal, Darrick J . Wong, linux-fsdevel,
linux-xfs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: improve naming for fsid helpers
2021-03-18 6:27 ` Christoph Hellwig
2021-03-18 6:30 ` Al Viro
@ 2021-03-18 11:40 ` Christian Brauner
1 sibling, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2021-03-18 11:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Al Viro, Vivek Goyal, Darrick J . Wong, linux-fsdevel, linux-xfs
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!
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] fs: introduce two little fs{u,g}id inode initialization helpers
2021-03-15 14:54 [PATCH 0/3] tweak idmap helpers Christian Brauner
2021-03-15 14:54 ` [PATCH 1/3] fs: introduce fsuidgid_has_mapping() helper Christian Brauner
2021-03-15 14:54 ` [PATCH 2/3] fs: improve naming for fsid helpers Christian Brauner
@ 2021-03-15 14:54 ` Christian Brauner
2021-03-18 6:27 ` Christoph Hellwig
2 siblings, 1 reply; 10+ messages in thread
From: Christian Brauner @ 2021-03-15 14:54 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 so that do the right thing when
initializing the i_uid and i_gid fields on idmapped and non-idmapped
mounts. Filesystemd don't need to bother with too many details for this.
Inspired-by: Vivek Goyal <vgoyal@redhat.com>
Cc: 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>
---
fs/ext4/ialloc.c | 2 +-
fs/inode.c | 4 ++--
fs/xfs/xfs_inode.c | 2 +-
include/linux/fs.h | 12 ++++++++++++
4 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 633ae7becd61..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 = fsuid_into_mnt(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 a047ab306f9a..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 = fsuid_into_mnt(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 = fsgid_into_mnt(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 8703408bd1aa..20846810f13f 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_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 189673721726..0cde0cbc20fc 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1620,6 +1620,18 @@ static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns)
return kgid_from_mnt(mnt_userns, current_fsgid());
}
+static inline void inode_fsuid_set(struct inode *inode,
+ struct user_namespace *mnt_userns)
+{
+ inode->i_uid = idmapped_fsuid(mnt_userns);
+}
+
+static inline void inode_fsgid_set(struct inode *inode,
+ struct user_namespace *mnt_userns)
+{
+ inode->i_gid = idmapped_fsgid(mnt_userns);
+}
+
static inline bool fsuidgid_has_mapping(struct super_block *sb,
struct user_namespace *mnt_userns)
{
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] fs: introduce two little fs{u,g}id inode initialization helpers
2021-03-15 14:54 ` [PATCH 3/3] fs: introduce two little fs{u,g}id inode initialization helpers Christian Brauner
@ 2021-03-18 6:27 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2021-03-18 6:27 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Al Viro, Vivek Goyal, Darrick J . Wong,
linux-fsdevel, linux-xfs
On Mon, Mar 15, 2021 at 03:54:19PM +0100, Christian Brauner wrote:
> Give filesystem two little helpers so that do the right thing when
> initializing the i_uid and i_gid fields on idmapped and non-idmapped
> mounts. Filesystemd don't need to bother with too many details for this.
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 10+ messages in thread