Re: [PATCH] apparmor: fix cred UAF caused by begin_current_label_crit_section()

On Thu, Aug 6, 2026 at 10:06 AM John Johansen
<[email protected]> wrote:
> On 8/6/26 00:32, Peter Zijlstra wrote:
> > On Tue, Jul 14, 2026 at 05:38:07PM +0200, Jann Horn wrote:
> >
> >> diff --git a/include/linux/task_work.h b/include/linux/task_work.h
> >> index 0646804860ff..ce19fc14060c 100644
> >> --- a/include/linux/task_work.h
> >> +++ b/include/linux/task_work.h
> >> @@ -33,6 +33,7 @@ struct callback_head *task_work_cancel_match(struct task_struct *task,
> >>      bool (*match)(struct callback_head *, void *data), void *data);
> >>   struct callback_head *task_work_cancel_func(struct task_struct *, task_work_func_t);
> >>   bool task_work_cancel(struct task_struct *task, struct callback_head *cb);
> >> +bool task_work_has_func(struct task_struct *task, task_work_func_t func);
> >>   void task_work_run(void);
> >>
> >>   static inline void exit_task_work(struct task_struct *task)
> >> diff --git a/kernel/task_work.c b/kernel/task_work.c
> >> index 0f7519f8e7c9..f83d1528e0bc 100644
> >> --- a/kernel/task_work.c
> >> +++ b/kernel/task_work.c
> >> @@ -189,6 +189,20 @@ bool task_work_cancel(struct task_struct *task, struct callback_head *cb)
> >>      return ret == cb;
> >>   }
> >>
> >> +bool task_work_has_func(struct task_struct *task, task_work_func_t func)
> >> +{
> >> +    struct callback_head *work;
> >> +
> >> +    if (!task_work_pending(task))
> >> +            return false;
> >> +    guard(raw_spinlock_irqsave)(&task->pi_lock);
> >> +    for (work = READ_ONCE(task->task_works); work; work = READ_ONCE(work->next)) {
> >> +            if (work->func == func)
> >> +                    return true;
> >> +    }
> >> +    return false;
> >> +}
> >> +
> >>   /**
> >>    * task_work_run - execute the works added by task_work_add()
> >>    *
> >
> > This thing is quite terrible. And AFAICT the only purpose is to
> > determine if said task already has said function enqueued. Why not add a
> > single bit to struct task_struct for this? I'm sure we have a spare bit
> > somewhere.
> >
>
> single bit wouldn't work generically to represent the different functions
> that could be enqueued but we could stick a flag in the apparmor task
> security blob, so we could just check if apparmor has enqueued its
> function.
>
> The trade-off is you don't get an admittedly ugly generic fn that someone
> else could use.

Yeah, makes sense, I'll rework this to not touch task_work.c and
instead do something with the apparmor task blob.