[apparmor] AppArmor kernel audit locks up system

[Paul Moore]

On Wed, Sep 27, 2023 at 8:20 AM John Johansen
<john.johansen at canonical.com> wrote:
>
> adding paul and the audit list
>
> On 9/26/23 05:56, Andreas Steinmetz wrote:
> > Hi,
> > it happen from time to time while setting up AppArmor properly that
> > instead of creating an audit message AppArmor locks up the system. In
> > most cases I couldn't even get any information, but this time I got
> > some.
> > I don't yet know if this is the case here but in case of "systemd
> > --user" it was a missing grant of cap_sys_resource that caused the
> > lockup. I don't know the exact capability causing the below problem
> > but granting cap_sys_admin, cap_setuid, cap_setgid and
> > cap_sys_resource prevents the lockup so it is definitely capability
> > related. dmesg output follows. Kernel is 6.5.3, distro is Arch Linux.
> >
>
> just to double check, you are using auditd and have some audit filters
> loaded, the code trace indicates yes.
>
> so at a first pass it appears this come when the task calls prlimit64,
> the capability check is denied and apparmor (or really any lsm).
> generates an audit message. The audit message goes into the audit
> subsystems, and there an exe filter which tries to get the task_exe_file
> resulting in deadlock.
>
> do_prlimit()
>         task_lock(tsk->group_leader);
>             ...
>                         if (new_rlim->rlim_max > rlim->rlim_max &&
>                                 !capable(CAP_SYS_RESOURCE))
>                                         ...
>                                         case AUDIT_EXE:
>                                                 result = audit_exe_compare(current, e->rule.exe)
>                                                         ...
>                                                         struct file *get_task_exe_file(struct task_struct *task)
>                                                                 ...
>                                                                 task_lock(task);
>                                                                 ...
>             ...
>         task_unlock(tsk->group_leader);
>
>
> I am not sure how best to handle this.

My apologies for the delay in responding, unfortunately this email was
a bit buried in my inbox and it took me a while to get to it.

Unfortunately, like John, I'm struggling a bit to see how we might
resolve this.  Unfortunately, we don't always know the context in
which audit_filter() is going to be called; in some cases we would
need to ensure the task_lock() was held, in others we may already be
holding it on the current CPU.  The only thing I'm reasonably certain
about is that audit_exe_compare() can't safely use
get_task_exe_file().

I imagine we could think about grabbing a reference to the mm_struct
and stashing it in the audit_context so that we could access
mm_struct::exe_file with only a RCU lock, but then we have to manage
the mm_struct reference in audit_context and I worry that will be ugly
and/or expensive (likely "and").

Does anyone else have any bright ideas or crazy thoughts on this?

-- 
paul-moore.com