[PATCH] apparmor: hold candidate profile reference during attachment lookup

[PATCH] apparmor: hold candidate profile reference during attachment lookup

[Weiming Shi]

find_attach() retains the best matching profile in candidate while walking
the profile list under RCU.  When matching xattrs, it pins only the current
profile and drops the RCU read lock around aa_xattrs_match().  A concurrent
policy removal can therefore free an earlier candidate during this window.
If the policy revision changes, the retry also preserves candidate and its
ranking state because they are initialized above the restart label.  The
final aa_get_newest_profile() then dereferences the freed profile.

The reproducer triggers:

  BUG: KASAN: slab-use-after-free in aa_get_newest_label
  Read of size 8 by task poc
  Call Trace:
   aa_get_newest_label security/apparmor/include/label.h:413
   find_attach security/apparmor/domain.c:505
   profile_transition security/apparmor/domain.c:700
   apparmor_bprm_creds_for_exec security/apparmor/domain.c:1034
   security_bprm_creds_for_exec security/security.c:793
   bprm_execve fs/exec.c:1861
  Kernel panic - not syncing: KASAN: panic_on_warn set ...

Take a reference whenever a profile becomes the candidate, drop the
previous candidate reference, and reset all candidate state when restarting
the search.  Release the held reference after acquiring the newest profile
or before returning an unsuccessful match.

Fixes: 8c62ed27a12c ("apparmor: fix aa_xattrs_match() may sleep while holding a RCU lock")
Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/linux-security-module/20261105164839.66760-1-shuangpeng.kernel@gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 security/apparmor/domain.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index af9e8431e3ea..53465caca702 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -395,9 +395,9 @@ static struct aa_label *find_attach(const struct path *path,
 				    struct aa_ns *ns, struct list_head *head,
 				    const char *name, const char **info)
 {
-	int candidate_len = 0, candidate_xattrs = 0;
-	bool conflict = false;
-	struct aa_profile *profile, *candidate = NULL;
+	struct aa_profile *profile, *tmp, *candidate = NULL;
+	int candidate_len, candidate_xattrs;
+	bool conflict;
 
 	AA_BUG(!path);
 	AA_BUG(!name);
@@ -405,6 +405,12 @@ static struct aa_label *find_attach(const struct path *path,
 
 	rcu_read_lock();
 restart:
+	aa_put_profile(candidate);
+	candidate = NULL;
+	candidate_len = 0;
+	candidate_xattrs = 0;
+	conflict = false;
+
 	list_for_each_entry_rcu(profile, head, base.list) {
 		struct aa_attachment *attach = &profile->attach;
 
@@ -477,7 +483,11 @@ static struct aa_label *find_attach(const struct path *path,
 				/* Either the same length with more matching
 				 * xattrs, or a longer match
 				 */
-				candidate = profile;
+				tmp = aa_get_profile_not0(profile);
+				if (!tmp)
+					goto restart;
+				aa_put_profile(candidate);
+				candidate = tmp;
 				candidate_len = max(count, attach->xmatch_len);
 				candidate_xattrs = ret;
 				conflict = false;
@@ -487,7 +497,11 @@ static struct aa_label *find_attach(const struct path *path,
 			 * old exact non-re match, without conditionals such
 			 * as xattrs. no more searching required
 			 */
-			candidate = profile;
+			tmp = aa_get_profile_not0(profile);
+			if (!tmp)
+				goto restart;
+			aa_put_profile(candidate);
+			candidate = tmp;
 			goto out;
 		}
 	}
@@ -495,15 +509,17 @@ static struct aa_label *find_attach(const struct path *path,
 	if (!candidate || conflict) {
 		if (conflict)
 			*info = CONFLICTING_ATTACH_STR;
+		aa_put_profile(candidate);
 		rcu_read_unlock();
 		return NULL;
 	}
 
 out:
-	candidate = aa_get_newest_profile(candidate);
+	tmp = aa_get_newest_profile(candidate);
+	aa_put_profile(candidate);
 	rcu_read_unlock();
 
-	return &candidate->label;
+	return &tmp->label;
 }
 
 static const char *next_name(int xtype, const char *name)
-- 
2.56.0

Re: [PATCH] apparmor: hold candidate profile reference during attachment lookup

[sashiko-bot]

> find_attach() retains the best matching profile in candidate while walking
> the profile list under RCU.  When matching xattrs, it pins only the current
> profile and drops the RCU read lock around aa_xattrs_match().  A concurrent
> policy removal can therefore free an earlier candidate during this window.
> If the policy revision changes, the retry also preserves candidate and its
> ranking state because they are initialized above the restart label.  The
> final aa_get_newest_profile() then dereferences the freed profile.
> 
> The reproducer triggers:
> 
>   BUG: KASAN: slab-use-after-free in aa_get_newest_label
>   Read of size 8 by task poc
>   Call Trace:
>    aa_get_newest_label security/apparmor/include/label.h:413
>    find_attach security/apparmor/domain.c:505
> [ ... ]
> Fixes: 8c62ed27a12c ("apparmor: fix aa_xattrs_match() may sleep while holding a RCU lock")
> Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
> Closes: https://lore.kernel.org/linux-security-module/20261105164839.66760-1-shuangpeng.kernel@gmail.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107183514.1575928-2-bestswngs@gmail.com?part=1