Skip to content

Commit 106adb1

Browse files
committed
use uniform permission checks for all mount propagation changes
jira VULN-98606 jira VULN-98607 cve-bf CVE-2025-38498 commit-author Al Viro <[email protected]> commit cffd044 upstream-diff This is a partial backport due to missing 9ffb14e which introduces do_set_group(). This is where the below operation on diffferent aspects of the same thing is introduced. This change does however introduce some additional safety checks which should be considered. do_change_type() and do_set_group() are operating on different aspects of the same thing - propagation graph. The latter asks for mounts involved to be mounted in namespace(s) the caller has CAP_SYS_ADMIN for. The former is a mess - originally it didn't even check that mount *is* mounted. That got fixed, but the resulting check turns out to be too strict for userland - in effect, we check that mount is in our namespace, having already checked that we have CAP_SYS_ADMIN there. What we really need (in both cases) is * only touch mounts that are mounted. That's a must-have constraint - data corruption happens if it get violated. * don't allow to mess with a namespace unless you already have enough permissions to do so (i.e. CAP_SYS_ADMIN in its userns). That's an equivalent of what do_set_group() does; let's extract that into a helper (may_change_propagation()) and use it in both do_set_group() and do_change_type(). Fixes: 12f147d "do_change_type(): refuse to operate on unmounted/not ours mounts" Acked-by: Andrei Vagin <[email protected]> Reviewed-by: Pavel Tikhomirov <[email protected]> Tested-by: Pavel Tikhomirov <[email protected]> Reviewed-by: Christian Brauner <[email protected]> Signed-off-by: Al Viro <[email protected]> (cherry picked from commit cffd044) Signed-off-by: Jonathan Maple <[email protected]>
1 parent 6436952 commit 106adb1

File tree

1 file changed

+16
-3
lines changed

1 file changed

+16
-3
lines changed

fs/namespace.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,6 +2254,19 @@ static int graft_tree(struct mount *mnt, struct mount *p, struct mountpoint *mp)
22542254
return attach_recursive_mnt(mnt, p, mp, NULL);
22552255
}
22562256

2257+
static int may_change_propagation(const struct mount *m)
2258+
{
2259+
struct mnt_namespace *ns = m->mnt_ns;
2260+
2261+
// it must be mounted in some namespace
2262+
if (IS_ERR_OR_NULL(ns)) // is_mounted()
2263+
return -EINVAL;
2264+
// and the caller must be admin in userns of that namespace
2265+
if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN))
2266+
return -EPERM;
2267+
return 0;
2268+
}
2269+
22572270
/*
22582271
* Sanity check the flags to change_mnt_propagation.
22592272
*/
@@ -2290,10 +2303,10 @@ static int do_change_type(struct path *path, int ms_flags)
22902303
return -EINVAL;
22912304

22922305
namespace_lock();
2293-
if (!check_mnt(mnt)) {
2294-
err = -EINVAL;
2306+
err = may_change_propagation(mnt);
2307+
if (err)
22952308
goto out_unlock;
2296-
}
2309+
22972310
if (type == MS_SHARED) {
22982311
err = invent_group_ids(mnt, recurse);
22992312
if (err)

0 commit comments

Comments
 (0)