[ciqlts8_6] CVE-2025-38498 and its bfs #764
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jira VULN-98605 cve CVE-2025-38498 commit-author Al Viro <viro@zeniv.linux.org.uk> commit 12f147d Ensure that propagation settings can only be changed for mounts located in the caller's mount namespace. This change aligns permission checking with the rest of mount(2). Reviewed-by: Christian Brauner <brauner@kernel.org> Fixes: 07b2088 ("beginning of the shared-subtree proper") Reported-by: "Orlando, Noah" <Noah.Orlando@deshaw.com> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> (cherry picked from commit 12f147d) Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
jira VULN-98605 cve-bf CVE-2025-38498 commit-author Pavel Tikhomirov <ptikhomirov@virtuozzo.com> commit 9ffb14e Previously a sharing group (shared and master ids pair) can be only inherited when mount is created via bindmount. This patch adds an ability to add an existing private mount into an existing sharing group. With this functionality one can first create the desired mount tree from only private mounts (without the need to care about undesired mount propagation or mount creation order implied by sharing group dependencies), and next then setup any desired mount sharing between those mounts in tree as needed. This allows CRIU to restore any set of mount namespaces, mount trees and sharing group trees for a container. We have many issues with restoring mounts in CRIU related to sharing groups and propagation: - reverse sharing groups vs mount tree order requires complex mounts reordering which mostly implies also using some temporary mounts (please see https://lkml.org/lkml/2021/3/23/569 for more info) - mount() syscall creates tons of mounts due to propagation - mount re-parenting due to propagation - "Mount Trap" due to propagation - "Non Uniform" propagation, meaning that with different tricks with mount order and temporary children-"lock" mounts one can create mount trees which can't be restored without those tricks (see https://www.linuxplumbersconf.org/event/7/contributions/640/) With this new functionality we can resolve all the problems with propagation at once. Link: https://lore.kernel.org/r/20210715100714.120228-1-ptikhomirov@virtuozzo.com Cc: Eric W. Biederman <ebiederm@xmission.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Christian Brauner <christian.brauner@ubuntu.com> Cc: Mattias Nissler <mnissler@chromium.org> Cc: Aleksa Sarai <cyphar@cyphar.com> Cc: Andrei Vagin <avagin@gmail.com> Cc: linux-fsdevel@vger.kernel.org Cc: linux-api@vger.kernel.org Cc: lkml <linux-kernel@vger.kernel.org> Co-developed-by: Andrei Vagin <avagin@gmail.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com> Signed-off-by: Andrei Vagin <avagin@gmail.com> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> (cherry picked from commit 9ffb14e) Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
jira VULN-98605 cve-bf CVE-2025-38498 commit-author Al Viro <viro@zeniv.linux.org.uk> commit d8cc036 9ffb14e "move_mount: allow to add a mount into an existing group" breaks assertions on ->mnt_share/->mnt_slave. For once, the data structures in question are actually documented. Documentation/filesystem/sharedsubtree.rst: All vfsmounts in a peer group have the same ->mnt_master. If it is non-NULL, they form a contiguous (ordered) segment of slave list. do_set_group() puts a mount into the same place in propagation graph as the old one. As the result, if old mount gets events from somewhere and is not a pure event sink, new one needs to be placed next to the old one in the slave list the old one's on. If it is a pure event sink, we only need to make sure the new one doesn't end up in the middle of some peer group. "move_mount: allow to add a mount into an existing group" ends up putting the new one in the beginning of list; that's definitely not going to be in the middle of anything, so that's fine for case when old is not marked shared. In case when old one _is_ marked shared (i.e. is not a pure event sink), that breaks the assumptions of propagation graph iterators. Put the new mount next to the old one on the list - that does the right thing in "old is marked shared" case and is just as correct as the current behaviour if old is not marked shared (kudos to Pavel for pointing that out - my original suggested fix changed behaviour in the "nor marked" case, which complicated things for no good reason). Reviewed-by: Christian Brauner <brauner@kernel.org> Fixes: 9ffb14e ("move_mount: allow to add a mount into an existing group") Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> (cherry picked from commit d8cc036) Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
jira VULN-98605 cve-bf CVE-2025-38498 commit-author Al Viro <viro@zeniv.linux.org.uk> commit cffd044 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 <avagin@gmail.com> Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com> Tested-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com> Reviewed-by: Christian Brauner <brauner@kernel.org> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> (cherry picked from commit cffd044) Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
🔍 Interdiff Analysis
diff -u b/fs/namespace.c b/fs/namespace.c
--- b/fs/namespace.c
+++ b/fs/namespace.c
@@ -2708,4 +2708,4 @@
static int do_move_mount(struct path *old_path, struct path *new_path)
{
- struct mnt_namespace *ns;
+ struct path parent_path = {.mnt = NULL, .dentry = NULL};
diff -u b/fs/namespace.c b/fs/namespace.c
--- b/fs/namespace.c
+++ b/fs/namespace.c
@@ -2251,4 +2251,4 @@
- return attach_recursive_mnt(mnt, p, mp);
+ return attach_recursive_mnt(mnt, p, mp, NULL);
}
static int may_change_propagation(const struct mount *m)This is an automated interdiff check for backported commits. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DESCRIPTION
Commit 12f147d ("do_change_type(): refuse to operate on unmounted/not ours mounts") is the fix.
Clean cherry-pick.
But it had a bf. Commit cffd044 "use uniform permission checks for all mount propagation changes"
that was not a clean cherry-pick. Similar to what we need for lts9.4 and 9.2,
I cherry-pick the dependency
9ffb14e ("move_mount: allow to add a mount into an existing group").
This required a fix cffd044 ("fix propagation graph breakage by MOVE_MOUNT_SET_GROUP move_mount(2)")
That's how we ended up with 4 commits instead of only 2.
COMMITS
TESTING
BUILD
Kselftests
Check_kernel_commits
Run interdiff
Due to missing 2763d11 ("get rid of detach_mnt()") but it's not needed for this.
Due to missing 86b1da9 ("attach_recursive_mnt(): get rid of flags entirely"), not relevant.
Run jira_pr_check