Re: [PATCH] fs: require CAP_SYS_ADMIN in target namespace for idmapped mounts

From: Christian Brauner <brauner@kernel.org>
To: Seth Forshee <sforshee@digitalocean.com>
Cc: linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] fs: require CAP_SYS_ADMIN in target namespace for idmapped mounts
Date: Tue, 16 Aug 2022 19:07:51 +0200	[thread overview]
Message-ID: <20220816170751.wdpzqff345voonyq@wittgenstein> (raw)
In-Reply-To: <20220816164752.2595240-1-sforshee@digitalocean.com>

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>