Re: [PATCH 1/2] hung_task: show the blocker task if the task is hung on rtmutex

Ruipeng Qi <[email protected]> Mon, 3 Aug 2026 22:42:48 +0800
Newsgroups dev.linux.lists.linux-rt-devel
Message-ID <[email protected]>
On 8/1/2026 12:07 AM, Petr Mladek wrote:
> On Thu 2026-07-30 10:29:08, ruipengqi wrote:
>> From: Ruipeng Qi <[email protected]>
>>
>> Currently, debug_show_blocker() only tracks mutex, semaphore and rwsem
>> lock types. Extend it to also cover rtmutex or rt_mutex-based
>> implementations locks on PREEMPT_RT, so that when a task is hung
>> on an rtmutex, the hung task detector can identify and report which
>> task holds the lock.
>>
>> On 64-bit systems, lock pointers are 8-byte aligned, so their three
>> least significant bits are always zero. Use these bits to encode the
>> blocker type.
>>
>> Unlike semaphores, rtmutex has built-in owner tracking via lock->owner,
>> so no additional bookkeeping is needed. The owner can be read with
>> rt_mutex_owner().
>>
>> --- a/kernel/hung_task.c
>> +++ b/kernel/hung_task.c
>> @@ -27,6 +27,7 @@
>>   #include <linux/sys_info.h>
>>   
>>   #include <trace/events/sched.h>
>> +#include <linux/rtmutex.h>
>>   
>>   /*
>>    * The number of tasks checked:
>> @@ -149,12 +150,15 @@ static void debug_show_blocker(struct task_struct *task, unsigned long timeout)
>>   	blocker_type = hung_task_get_blocker_type(blocker);
>>   
>>   	switch (blocker_type) {
>> +#ifndef CONFIG_PREEMPT_RT
>>   	case BLOCKER_TYPE_MUTEX:
>>   		owner = mutex_get_owner(hung_task_blocker_to_lock(blocker));
>>   		break;
>> +#endif
>>   	case BLOCKER_TYPE_SEM:
>>   		owner = sem_last_holder(hung_task_blocker_to_lock(blocker));
>>   		break;
>> +#ifndef CONFIG_PREEMPT_RT
>>   	case BLOCKER_TYPE_RWSEM_READER:
>>   	case BLOCKER_TYPE_RWSEM_WRITER:
>>   		owner = (unsigned long)rwsem_owner(
>> @@ -165,6 +169,12 @@ static void debug_show_blocker(struct task_struct *task, unsigned long timeout)
>>   					hung_task_blocker_to_lock(blocker)) ?
>>   					"reader" : "writer";
>>   		break;
>> +#endif
>> +#if defined(CONFIG_64BIT) && defined(CONFIG_RT_MUTEXES)
>> +	case BLOCKER_TYPE_RTMUTEX:
>> +		owner = (unsigned long)rt_mutex_owner(hung_task_blocker_to_lock(blocker));
>> +		break;
>> +#endif
>>   	default:
>>   		WARN_ON_ONCE(1);
>>   		return;
> On one hand, it is "elegant" to handle rtmutex the same way as
> the other locks.
>
> On the other hand, this adds a lot of if-deffery and
> does not work on 32-bit systems.
>
> Alternative solution would use the existing tracking,
> something like:
>
> 	if (rt_mutext_blocking_task(task))
> 		owner = (unsigned long)rt_mutex_owner(rt_mutex_blocking_task(task));
>
> , where rt_mutex_blocking_task() would be a racy public variant
> of the existing task_blocked_on_lock(). Something like:
>
> static inline struct rt_mutex_base *rt_mutex_blocking_task(struct task_struct *p)
> {
> 	struct rt_mutex_waiter *waiter;
>
> 	waiter = data_race(READ_ONCE(p->pi_blocked_on)
>
> 	if (waiter)
> 		return data_race(READ_ONCE(waiter->lock);
>
> 	return NULL;
> }
Thanks to Petr, Lance, and Xusheng for the feedback.

One concern with the alternative approach is that 'waiter' is typically 
allocated on the stack (for instance, in __rt_mutex_slowlock_locked()). 
If the blocked task gets woken up and resumes execution, its stack frame 
is destroyed and the 'waiter' structure becomes invalid.

Since the hung task detector reads p->pi_blocked_on asynchronously 
without holding the lock, dereferencing waiter->lock could lead to a UAF 
race condition if the waiter is destroyed concurrently. Therefore, the 
first solution (encoding the blocker type in the pointer bits) seems 
safer, as it avoids dereferencing a potentially stale stack pointer.

I am currently preparing the V2 patch to address the issues you mentioned.

v2 changes:
  - Reduce #ifdef-deffery.
  - Remove the unused function debug_trace_blocker.
  - Clean up and update relevant comments.

Best regards,
Ruipeng
> Plus, it would require using WRITE_ONCE() when task->pi_blocked_on
> is updated to make sure that the value is always consistent
> (valid pointer or NULL).
>
> It would avoid the duplicate tracking in the rt_mutex code
> and might be more acceptable for the rt_mutex maintainers.
>
> Best Regards,
> Petr