the attached patch is in testing.

From 4433a1cda57b07177e322806f5f9fff1f6765787 Mon Sep 17 00:00:00 2001
From: John Johansen <[email protected]>
Date: Wed, 12 Aug 2026 02:27:15 -0700
Subject: [PATCH] apparmor: fix deadlock in complain-mode change_hat

The use of change_hat when in complain mode can cause a deadlock
when the hat doesn't exist and a new learning profile is created
for the missing profile. This is because change_hat() has taken
the lock to search the hat list and creating the new learning
profile needs to take the lock to add it to the list.

From the bug report:

Originally found in 7.0.0 in LTS ubuntu 26.04 with pam_apparmor + su
in complain mode set to change hats.  Then verified in newest
available vanilla kernel I've compiled to see if still present:

7.2-rc7 vanilla -> affected

checked also some other kernels:
6.18.44 vanilla -> affected
6.12.95 with debian patches -> unaffected

On systems without bug (for example 6.12.95 debian) it just prints:

aa_change_hat rc=0

On systems with bug, the executable always hangs, prints nothing and
becomes unkillable.  (And once stuck this way, it will cause any
further hat changes to also cause the changing process to get stuck)

Then in syslog you can find hint about cause:

kernel: INFO: task hat:3409 blocked for more than 483 seconds.
kernel:       Not tainted 7.2.0-rc7 #1
kernel: "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
kernel: task:hat             state:D stack:0     pid:3409  tgid:3409  ppid:2605 
  task_flags:0x400000 flags:0x00080800
kernel: Call Trace:
kernel:  <TASK>
kernel:  __schedule+0x48f/0xfe0
kernel:  schedule+0x27/0xa0
kernel:  schedule_preempt_disabled+0x15/0x30
kernel:  __mutex_lock.constprop.0+0x569/0xa10
kernel:  aa_new_learning_profile+0x15f/0x210
kernel:  build_change_hat+0x19f/0x3b0
kernel:  change_hat.isra.0+0x5dd/0xd60
kernel:  aa_change_hat+0x2f3/0x710
kernel:  aa_setprocattr_changehat+0x121/0x1f0
kernel:  do_setattr+0x28c/0x340
kernel:  apparmor_setselfattr+0x20/0x50
kernel:  security_setselfattr+0xf6/0x110
kernel:  __x64_sys_lsm_set_self_attr+0x53/0x90
kernel:  do_syscall_64+0xdd/0x5e0
kernel:  ? __mod_memcg_lruvec_state+0xfd/0x260
kernel:  ? lruvec_stat_mod_folio+0x8d/0xd0
kernel:  ? __folio_mod_stat+0x2d/0x90
kernel:  ? map_anon_folio_pte_nopf+0xd1/0x1f0
kernel:  ? do_anonymous_page+0x184/0xa10
kernel:  ? __handle_mm_fault+0x805/0x870
kernel:  ? count_memcg_events+0xef/0x230
kernel:  ? handle_mm_fault+0x1f0/0x2f0
kernel:  ? do_user_addr_fault+0x2bb/0x7b0
kernel:  ? do_syscall_64+0x94/0x5e0
kernel:  ? exc_page_fault+0x75/0x160
kernel:  entry_SYSCALL_64_after_hwframe+0x76/0x7e
kernel: RIP: 0033:0x7f815e134c8d
kernel: RSP: 002b:00007fff6df94ea8 EFLAGS: 00000246 ORIG_RAX: 00000000000001cc
kernel: RAX: ffffffffffffffda RBX: 0000556d8c81d040 RCX: 00007f815e134c8d
kernel: RDX: 0000000000000046 RSI: 0000556d8c81d040 RDI: 0000000000000064
kernel: RBP: 00007fff6df94ef0 R08: 00007f815e212ac8 R09: 000000000000000c
kernel: R10: 0000000000000000 R11: 0000000000000246 R12: 0000556d8c81d010
kernel: R13: 0000000000000026 R14: 0000000000000046 R15: 0000000000000064
kernel:  </TASK>
kernel: INFO: task hat:3409 is blocked on a mutex likely owned by task hat:3409.

To fix the issue, lift the locking out of the core of
aa_new_learning_profile(), introduce a wrapper function that takes the
lock where needed, and have build_change_hat() call the core function
that no longer takes the lock.

In addition fix 4 other issues introduced by commit
32e92764d6f8d ("apparmor: grab ns lock and refresh when looking up changehat child 
profiles")
- aa_get_profile_rcu() was replaced-by: aa_get_profile without the
  accompanying rcu_dereference_protected()
- an extra aa_get_label(label) was introduced at the start of
  change_hat() without an accompanying aa_put_label() causing a
  reference count leak.
- a reference count leak was introduced in the label_is_stale(label)
  case, where the newest profile would be leaked instead of the
  label passed to the function.
- a potential UAF when the lookup walks up the tree with new_ns != ns
  the new label reference is put, and then used for the next lookup.
  The mutex_lock, will block replacement, and removal in the locked
  ns. However there are two cases where putting the reference can
  result in the label being freed even with the lock held.

  1. the label does not have a list reference (possible for temporary
     or special profiles) in which case the put can trigger the
     cleanup.
  2. the new label reference is in a different namespace, which does
     not have a lock held on it. This extends case 1 to also include
     replacement, and removal that could be occurring in the namespace
     new is in.

Reported-by: Martin Petricek <[email protected]>
Link: https://lists.ubuntu.com/archives/apparmor/2026-August/014907.html
Fixes: 32e92764d6f8d ("apparmor: grab ns lock and refresh when looking up changehat 
child profiles")
Signed-off-by: John Johansen <[email protected]>
---
 security/apparmor/domain.c         | 22 +++++++++++++++-------
 security/apparmor/include/policy.h |  3 +++
 security/apparmor/policy.c         | 22 +++++++++++++++++-----
 3 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index e20d177a43574..3d2c76a908209 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -1138,8 +1138,8 @@ static struct aa_label *build_change_hat(const struct 
cred *subj_cred,
        if (!hat) {
                error = -ENOENT;
                if (COMPLAIN_MODE(profile)) {
-                       hat = aa_new_learning_profile(profile, true, name,
-                                                     GFP_KERNEL);
+                       hat = __aa_new_learning_profile(profile, true, name,
+                                                       GFP_KERNEL);
                        if (!hat) {
                                info = "failed null profile create";
                                error = -ENOMEM;
@@ -1177,6 +1177,7 @@ static struct aa_label *change_hat(const struct cred 
*subj_cred,
        bool sibling = false;
        const char *name, *info = NULL;
        int i, error;
+       bool needput = false;
AA_BUG(!label);
        AA_BUG(!hats);
@@ -1189,7 +1190,6 @@ static struct aa_label *change_hat(const struct cred 
*subj_cred,
         * the profiles and label, we can rely on the namespaces being live
         * and avoid incrementing their refcounts while grabbing the lock.
         */
-       label = aa_get_label(label);
        ns = labels_ns(label);
retry:
@@ -1197,15 +1197,19 @@ static struct aa_label *change_hat(const struct cred 
*subj_cred,
        if (label_is_stale(label)) {
                new = aa_get_newest_label(label);
                new_ns = labels_ns(new);
+               /* check if replaced with label in parent ns, and lock there */
                if (new_ns != ns) {
-                       aa_put_label(new);
                        mutex_unlock(&ns->lock);
+                       if (needput)
+                               aa_put_label(label);
                        ns = new_ns;
                        label = new;
+                       needput = true;
+                       /* this will bottom out at the root of the tree */
                        goto retry;
                }
-               aa_put_label(label);
                label = new;
+               needput = true;
        }
if (PROFILE_IS_HAT(labels_profile(label)))
@@ -1216,7 +1220,8 @@ static struct aa_label *change_hat(const struct cred 
*subj_cred,
                name = hats[i];
                label_for_each_in_scope(it, labels_ns(label), label, profile) {
                        if (sibling && PROFILE_IS_HAT(profile)) {
-                               root = aa_get_profile(profile->parent);
+                               root = 
aa_get_profile(rcu_dereference_protected(profile->parent,
+                                                     
mutex_is_locked(&ns->lock)));
                        } else if (!sibling && !PROFILE_IS_HAT(profile)) {
                                root = aa_get_profile(profile);
                        } else {        /* conflicting change type */
@@ -1277,6 +1282,8 @@ static struct aa_label *change_hat(const struct cred 
*subj_cred,
                }
        }
        mutex_unlock(&ns->lock);
+       if (needput)
+               aa_put_label(label);
        return ERR_PTR(error);
build:
@@ -1287,7 +1294,8 @@ static struct aa_label *change_hat(const struct cred 
*subj_cred,
        mutex_unlock(&ns->lock);
        AA_BUG(!new);
        /* return new label or error ptr */
-
+       if (needput)
+               aa_put_label(label);
        return new;
 }
diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index 23b9f0e5df3c3..0dcbf8cf1029a 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -294,6 +294,9 @@ struct aa_profile *aa_alloc_profile(const char *name, 
struct aa_proxy *proxy,
                                    gfp_t gfp);
 struct aa_profile *aa_alloc_null(struct aa_profile *parent, const char *name,
                                 gfp_t gfp);
+struct aa_profile *__aa_new_learning_profile(struct aa_profile *parent,
+                                            bool hat, const char *base,
+                                            gfp_t gfp);
 struct aa_profile *aa_new_learning_profile(struct aa_profile *parent, bool hat,
                                           const char *base, gfp_t gfp);
 void aa_free_profile(struct aa_profile *profile);
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index f6f1b72d7c3df..397f9ff78aeb1 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -720,7 +720,7 @@ struct aa_profile *aa_alloc_null(struct aa_profile *parent, 
const char *name,
 }
/**
- * aa_new_learning_profile - create or find a null-X learning profile
+ * __aa_new_learning_profile - create or find a null-X learning profile
  * @parent: profile that caused this profile to be created (NOT NULL)
  * @hat: true if the null- learning profile is a hat
  * @base: name to base the null profile off of
@@ -737,8 +737,9 @@ struct aa_profile *aa_alloc_null(struct aa_profile *parent, 
const char *name,
  *
  * Returns: new refcounted profile else NULL on failure
  */
-struct aa_profile *aa_new_learning_profile(struct aa_profile *parent, bool hat,
-                                          const char *base, gfp_t gfp)
+struct aa_profile *__aa_new_learning_profile(struct aa_profile *parent,
+                                            bool hat, const char *base,
+                                            gfp_t gfp)
 {
        struct aa_profile *p, *profile;
        const char *bname;
@@ -746,6 +747,7 @@ struct aa_profile *aa_new_learning_profile(struct 
aa_profile *parent, bool hat,
        size_t name_sz;
AA_BUG(!parent);
+       AA_BUG(!mutex_is_locked(&parent->ns->lock));
if (base) {
                name_sz = strlen(parent->base.hname) + 8 + strlen(base);
@@ -779,7 +781,6 @@ struct aa_profile *aa_new_learning_profile(struct 
aa_profile *parent, bool hat,
        if (hat)
                profile->label.flags |= FLAG_HAT;
- mutex_lock_nested(&profile->ns->lock, profile->ns->level);
        p = __find_child(&parent->base.profiles, bname);
        if (p) {
                aa_free_profile(profile);
@@ -787,7 +788,6 @@ struct aa_profile *aa_new_learning_profile(struct 
aa_profile *parent, bool hat,
        } else {
                __add_profile(&parent->base.profiles, profile);
        }
-       mutex_unlock(&profile->ns->lock);
/* refcount released by caller */
 out:
@@ -801,6 +801,18 @@ struct aa_profile *aa_new_learning_profile(struct 
aa_profile *parent, bool hat,
        return NULL;
 }
+struct aa_profile *aa_new_learning_profile(struct aa_profile *parent, bool hat,
+                                          const char *base, gfp_t gfp)
+{
+       struct aa_profile *profile;
+
+       mutex_lock_nested(&parent->ns->lock, parent->ns->level);
+       profile = __aa_new_learning_profile(parent, hat, base, gfp);
+       mutex_unlock(&parent->ns->lock);
+
+       return profile;
+}
+
 /**
  * replacement_allowed - test to see if replacement is allowed
  * @profile: profile to test if it can be replaced  (MAYBE NULL)
--
2.53.0



Reply via email to