fhandle: use more consistent rules for decoding file handle from userns

Commit 620c266f39493 ("fhandle: relax open_by_handle_at() permission
checks") relaxed the coditions for decoding a file handle from non init
userns.

The conditions are that that decoded dentry is accessible from the user
provided mountfd (or to fs root) and that all the ancestors along the
path have a valid id mapping in the userns.

These conditions are intentionally more strict than the condition that
the decoded dentry should be "lookable" by path from the mountfd.

For example, the path /home/amir/dir/subdir is lookable by path from
unpriv userns of user amir, because /home perms is 755, but the owner of
/home does not have a valid id mapping in unpriv userns of user amir.

The current code did not check that the decoded dentry itself has a
valid id mapping in the userns.  There is no security risk in that,
because that final open still performs the needed permission checks,
but this is inconsistent with the checks performed on the ancestors,
so the behavior can be a bit confusing.

Add the check for the decoded dentry itself, so that the entire path,
including the last component has a valid id mapping in the userns.

Fixes: 620c266f39493 ("fhandle: relax open_by_handle_at() permission checks")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Link: https://lore.kernel.org/20250827194309.1259650-1-amir73il@gmail.com
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/fhandle.c b/fs/fhandle.c
index 68a7d2861c58fe620d07276b64e1a12f3f4b1ed1..a907ddfac4d51c3c2595213f00080f62ba632900 100644 (file)
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -207,6 +207,14 @@ static int vfs_dentry_acceptable(void *context, struct dentry *dentry)
        if (!ctx->flags)
                return 1;
 
+       /*
+        * Verify that the decoded dentry itself has a valid id mapping.
+        * In case the decoded dentry is the mountfd root itself, this
+        * verifies that the mountfd inode itself has a valid id mapping.
+        */
+       if (!privileged_wrt_inode_uidgid(user_ns, idmap, d_inode(dentry)))
+               return 0;
+
        /*
         * It's racy as we're not taking rename_lock but we're able to ignore
         * permissions and we just need an approximation whether we were able