open_tree_attr: do not allow id-mapping changes without OPEN_TREE_CLONE

[PATCH 0/2] open_tree_attr: do not allow id-mapping changes without OPEN_TREE_CLONE

Posted by Aleksa Sarai 1 month, 4 weeks ago

As described in commit 7a54947e727b ('Merge patch series "fs: allow
changing idmappings"'), open_tree_attr(2) was necessary in order to
allow for a detached mount to be created and have its idmappings changed
without the risk of any racing threads operating on it. For this reason,
mount_setattr(2) still does not allow for id-mappings to be changed.

However, there was a bug in commit 2462651ffa76 ("fs: allow changing
idmappings") which allowed users to bypass this restriction by calling
open_tree_attr(2) *without* OPEN_TREE_CLONE.

can_idmap_mount() prevented this bug from allowing an attached
mountpoint's id-mapping from being modified (thanks to an is_anon_ns()
check), but this still allows for detached (but visible) mounts to have
their be id-mapping changed. This risks the same UAF and locking issues
as described in the merge commit, and was likely unintentional.

For what it's worth, I found this while working on the open_tree_attr(2)
man page, and was trying to figure out what open_tree_attr(2)'s
behaviour was in the (slightly fruity) ~OPEN_TREE_CLONE case.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Aleksa Sarai (2):
      open_tree_attr: do not allow id-mapping changes without OPEN_TREE_CLONE
      selftests/mount_setattr: add smoke tests for open_tree_attr(2) bug

 fs/namespace.c                                     |  3 +-
 .../selftests/mount_setattr/mount_setattr_test.c   | 77 ++++++++++++++++++----
 2 files changed, 66 insertions(+), 14 deletions(-)
---
base-commit: 66639db858112bf6b0f76677f7517643d586e575
change-id: 20250808-open_tree_attr-bugfix-idmap-bb741166dc04

Best regards,
-- 
Aleksa Sarai <cyphar@cyphar.com>

Re: [PATCH 0/2] open_tree_attr: do not allow id-mapping changes without OPEN_TREE_CLONE

Posted by Christian Brauner 1 month, 3 weeks ago

On Fri, 08 Aug 2025 03:55:04 +1000, Aleksa Sarai wrote:
> As described in commit 7a54947e727b ('Merge patch series "fs: allow
> changing idmappings"'), open_tree_attr(2) was necessary in order to
> allow for a detached mount to be created and have its idmappings changed
> without the risk of any racing threads operating on it. For this reason,
> mount_setattr(2) still does not allow for id-mappings to be changed.
> 
> However, there was a bug in commit 2462651ffa76 ("fs: allow changing
> idmappings") which allowed users to bypass this restriction by calling
> open_tree_attr(2) *without* OPEN_TREE_CLONE.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/2] open_tree_attr: do not allow id-mapping changes without OPEN_TREE_CLONE
      https://git.kernel.org/vfs/vfs/c/75a7ed5ce861
[2/2] selftests/mount_setattr: add smoke tests for open_tree_attr(2) bug
      https://git.kernel.org/vfs/vfs/c/a597ba0a020b