Re: [BUG] apparmor: deadlock in complain-mode change_hat

On 8/11/26 11:55, Sasha Levin wrote:
>> 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
> 
> Thanks for the very complete report - the reproducer and the stack made
> this straightforward to pin down.
> 
> The commit is 32e92764d6f8 ("apparmor: grab ns lock and refresh when
> looking up changehat child profiles"). It makes change_hat() take
> ns->lock and hold it across the build: label. In complain mode with no
> matching hat, build_change_hat() calls aa_new_learning_profile()
> (aa_new_null_profile() on the older trees), which takes the same
> ns->lock again - a recursive acquire of a non-recursive mutex, so the
> task wedges in D state and never drops the namespace lock. That also
> explains why every later change_hat in that namespace hangs behind it.
> 
yep

> John, Ryan - this needs an upstream fix; the deadlock is in mainline
> too. There is nothing in v7.2-rc7 or in linux-next that addresses it,

right. I am working on a fix now.

> and I could not find a posted fix on the list. Two smaller things in the
> same commit that look wrong while you are in there: it swapped
> aa_get_profile_rcu(&profile->parent) for aa_get_profile(profile->parent)

yes, this really should be a dereference protected as parent is updated
under the ns_lock.
> on an __rcu pointer (the kernel test robot flagged the sparse warning
> twice in June), and it adds a label = aa_get_label(label) on entry with
> no matching put on either exit path.
> 

ack