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

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.

>> +/* replace the current task's stale label on syscall return */
>> +void aa_schedule_stale_label_replacement(void)
>> +{
>> +	struct callback_head *tw;
>> +
>> +	if (task_work_has_func(current, aa_replace_stale_label_tw_func))
>> +		return;
>> +	tw = kmalloc_obj(struct callback_head);
>> +	if (!tw)
>> +		return;
>> +	init_task_work(tw, aa_replace_stale_label_tw_func);
>> +	if (task_work_add(current, tw, TWA_RESUME))
>> +		kfree(tw);
>> +}
>