* [PATCH] fs: require CAP_SYS_ADMIN in target namespace for idmapped mounts
@ 2022-08-16 16:47 Seth Forshee
2022-08-16 17:07 ` Christian Brauner
0 siblings, 1 reply; 3+ messages in thread
From: Seth Forshee @ 2022-08-16 16:47 UTC (permalink / raw
To: Christian Brauner; +Cc: linux-fsdevel
Idmapped mounts should not allow a user to map file ownsership into a
range of ids which is not under the control of that user. However, we
currently don't check whether the mounter is privileged wrt to the
target user namespace.
Currently no FS_USERNS_MOUNT filesystems support idmapped mounts, thus
this is not a problem as only CAP_SYS_ADMIN in init_user_ns is allowed
to set up idmapped mounts. But this could change in the future, so add a
check to refuse to create idmapped mounts when the mounter does not have
CAP_SYS_ADMIN in the target user namespace.
Fixes: bd303368b776 ("fs: support mapped mounts of mapped filesystems")
Signed-off-by: Seth Forshee <sforshee@digitalocean.com>
---
fs/namespace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 68789f896f08..51416e6caf90 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4003,8 +4003,9 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
if (!(m->mnt_sb->s_type->fs_flags & FS_ALLOW_IDMAP))
return -EINVAL;
- /* We're not controlling the superblock. */
- if (!ns_capable(fs_userns, CAP_SYS_ADMIN))
+ /* Mounter doesn't control the source or target namespaces. */
+ if (!ns_capable(fs_userns, CAP_SYS_ADMIN) ||
+ !ns_capable(kattr->mnt_userns, CAP_SYS_ADMIN))
return -EPERM;
/* Mount has already been visible in the filesystem hierarchy. */
--
2.37.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] fs: require CAP_SYS_ADMIN in target namespace for idmapped mounts
2022-08-16 16:47 [PATCH] fs: require CAP_SYS_ADMIN in target namespace for idmapped mounts Seth Forshee
@ 2022-08-16 17:07 ` Christian Brauner
2022-08-16 18:07 ` Seth Forshee
0 siblings, 1 reply; 3+ messages in thread
From: Christian Brauner @ 2022-08-16 17:07 UTC (permalink / raw
To: Seth Forshee; +Cc: linux-fsdevel
On Tue, Aug 16, 2022 at 11:47:52AM -0500, Seth Forshee wrote:
> Idmapped mounts should not allow a user to map file ownsership into a
> range of ids which is not under the control of that user. However, we
> currently don't check whether the mounter is privileged wrt to the
> target user namespace.
>
> Currently no FS_USERNS_MOUNT filesystems support idmapped mounts, thus
> this is not a problem as only CAP_SYS_ADMIN in init_user_ns is allowed
> to set up idmapped mounts. But this could change in the future, so add a
> check to refuse to create idmapped mounts when the mounter does not have
> CAP_SYS_ADMIN in the target user namespace.
>
> Fixes: bd303368b776 ("fs: support mapped mounts of mapped filesystems")
> Signed-off-by: Seth Forshee <sforshee@digitalocean.com>
> ---
Fwiw, I think we can probably move the check into build_mount_idmapped()
right before we setup kattr->mnt_userns so we don't end up calling this
multiple times for each mount. But no need to resend for this. I can
move this. In general that seems like a good idea and good future
proofing,
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] fs: require CAP_SYS_ADMIN in target namespace for idmapped mounts
2022-08-16 17:07 ` Christian Brauner
@ 2022-08-16 18:07 ` Seth Forshee
0 siblings, 0 replies; 3+ messages in thread
From: Seth Forshee @ 2022-08-16 18:07 UTC (permalink / raw
To: Christian Brauner; +Cc: linux-fsdevel
On Tue, Aug 16, 2022 at 07:07:51PM +0200, Christian Brauner wrote:
> On Tue, Aug 16, 2022 at 11:47:52AM -0500, Seth Forshee wrote:
> > Idmapped mounts should not allow a user to map file ownsership into a
> > range of ids which is not under the control of that user. However, we
> > currently don't check whether the mounter is privileged wrt to the
> > target user namespace.
> >
> > Currently no FS_USERNS_MOUNT filesystems support idmapped mounts, thus
> > this is not a problem as only CAP_SYS_ADMIN in init_user_ns is allowed
> > to set up idmapped mounts. But this could change in the future, so add a
> > check to refuse to create idmapped mounts when the mounter does not have
> > CAP_SYS_ADMIN in the target user namespace.
> >
> > Fixes: bd303368b776 ("fs: support mapped mounts of mapped filesystems")
> > Signed-off-by: Seth Forshee <sforshee@digitalocean.com>
> > ---
>
> Fwiw, I think we can probably move the check into build_mount_idmapped()
> right before we setup kattr->mnt_userns so we don't end up calling this
> multiple times for each mount. But no need to resend for this. I can
> move this. In general that seems like a good idea and good future
> proofing,
> Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
That makes sense. Thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-16 18:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-16 16:47 [PATCH] fs: require CAP_SYS_ADMIN in target namespace for idmapped mounts Seth Forshee
2022-08-16 17:07 ` Christian Brauner
2022-08-16 18:07 ` Seth Forshee
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.