From ccbd0c991985afc53670e2b01840517922fc30e4 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Fri, 29 Apr 2022 15:57:48 +0200 Subject: docs: Add small intro to idmap examples When reading the documentation, I didn't understand why this list examples of things that fail without using the mount idmap feature. It seems pretty pointless and I doubted if I was missing something, until I finished the examples, the next section and saw the examples revisited. After that, it all made sense. Let's add one small sentence before, so the reader knows where this is going and why examples that don't might seem relevant are used. Link: https://lore.kernel.org/r/20220429135748.481301-1-rodrigo@sdfg.com.ar Reviewed-by: Christian Brauner (Microsoft) Signed-off-by: Rodrigo Campos Signed-off-by: Christian Brauner (Microsoft) --- Documentation/filesystems/idmappings.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/filesystems/idmappings.rst b/Documentation/filesystems/idmappings.rst index 7a879ec3b6bf..c1db8748389c 100644 --- a/Documentation/filesystems/idmappings.rst +++ b/Documentation/filesystems/idmappings.rst @@ -369,6 +369,11 @@ kernel maps the caller's userspace id down into a kernel id according to the caller's idmapping and then maps that kernel id up according to the filesystem's idmapping. +Let's see some examples with caller/filesystem idmapping but without mount +idmappings. This will exhibit some problems we can hit. After that we will +revisit/reconsider these examples, this time using mount idmappings, to see how +they can solve the problems we observed before. + Example 1 ~~~~~~~~~ -- cgit v1.2.3 From e1bbcd277a53e08d619ffeec56c5c9287f2bf42f Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 10 May 2022 11:58:40 +0200 Subject: fs: hold writers when changing mount's idmapping Hold writers when changing a mount's idmapping to make it more robust. The vfs layer takes care to retrieve the idmapping of a mount once ensuring that the idmapping used for vfs permission checking is identical to the idmapping passed down to the filesystem. For ioctl codepaths the filesystem itself is responsible for taking the idmapping into account if they need to. While all filesystems with FS_ALLOW_IDMAP raised take the same precautions as the vfs we should enforce it explicitly by making sure there are no active writers on the relevant mount while changing the idmapping. This is similar to turning a mount ro with the difference that in contrast to turning a mount ro changing the idmapping can only ever be done once while a mount can transition between ro and rw as much as it wants. This is a minor user-visible change. But it is extremely unlikely to matter. The caller must've created a detached mount via OPEN_TREE_CLONE and then handed that O_PATH fd to another process or thread which then must've gotten a writable fd for that mount and started creating files in there while the caller is still changing mount properties. While not impossible it will be an extremely rare corner-case and should in general be considered a bug in the application. Consider making a mount MOUNT_ATTR_NOEXEC or MOUNT_ATTR_NODEV while allowing someone else to perform lookups or exec'ing in parallel by handing them a copy of the OPEN_TREE_CLONE fd or another fd beneath that mount. Link: https://lore.kernel.org/r/20220510095840.152264-1-brauner@kernel.org Cc: Seth Forshee Cc: Christoph Hellwig Cc: Al Viro Cc: linux-fsdevel@vger.kernel.org Signed-off-by: Christian Brauner (Microsoft) --- fs/namespace.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index afe2b64b14f1..41461f55c039 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -4026,8 +4026,9 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt) static inline bool mnt_allow_writers(const struct mount_kattr *kattr, const struct mount *mnt) { - return !(kattr->attr_set & MNT_READONLY) || - (mnt->mnt.mnt_flags & MNT_READONLY); + return (!(kattr->attr_set & MNT_READONLY) || + (mnt->mnt.mnt_flags & MNT_READONLY)) && + !kattr->mnt_userns; } static int mount_setattr_prepare(struct mount_kattr *kattr, struct mount *mnt) -- cgit v1.2.3