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

Sasha Levin <[email protected]>
Newsgroups org.kernel.vger.stable
Message-ID <[email protected]>
> 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.

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,
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)
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.

-- 
Thanks,
Sasha
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.