[PATCH v2] mm: Fix possible deadlock in console_trylock_spinning

[PATCH v2] mm: Fix possible deadlock in console_trylock_spinning

[Gu Bowen]

Our syztester report the lockdep WARNING [1]. kmemleak_scan_thread()
invokes scan_block() which may invoke a nomal printk() to print warning
message. This can cause a deadlock in the scenario reported below:

       CPU0                    CPU1
       ----                    ----
  lock(kmemleak_lock);
                               lock(&port->lock);
                               lock(kmemleak_lock);
  lock(console_owner);

To solve this problem, switch to printk_safe mode before printing warning
message, this will redirect all printk()-s to a special per-CPU buffer,
which will be flushed later from a safe context (irq work), and this
deadlock problem can be avoided. The proper API to use should be
printk_deferred_enter()/printk_deferred_exit() if we want to deferred the
printing [2].

This patch also fix some similar cases that need to use the printk
deferring [3].

[1]
https://lore.kernel.org/all/20250730094914.566582-1-gubowen5@huawei.com/
[2]
https://lore.kernel.org/all/5ca375cd-4a20-4807-b897-68b289626550@redhat.com/
[3]
https://lore.kernel.org/all/aJCir5Wh362XzLSx@arm.com/
====================

Cc: stable@vger.kernel.org # 5.10
Signed-off-by: Gu Bowen <gubowen5@huawei.com>
---
 mm/kmemleak.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 4801751cb6b6..381145dde54f 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -390,9 +390,15 @@ static struct kmemleak_object *lookup_object(unsigned long ptr, int alias)
 		else if (object->pointer == ptr || alias)
 			return object;
 		else {
+			/*
+			 * Printk deferring due to the kmemleak_lock held.
+			 * This is done to avoid deadlock.
+			 */
+			printk_deferred_enter();
 			kmemleak_warn("Found object by alias at 0x%08lx\n",
 				      ptr);
 			dump_object_info(object);
+			printk_deferred_exit();
 			break;
 		}
 	}
@@ -433,8 +439,15 @@ static struct kmemleak_object *mem_pool_alloc(gfp_t gfp)
 		list_del(&object->object_list);
 	else if (mem_pool_free_count)
 		object = &mem_pool[--mem_pool_free_count];
-	else
+	else {
+		/*
+		 * Printk deferring due to the kmemleak_lock held.
+		 * This is done to avoid deadlock.
+		 */
+		printk_deferred_enter();
 		pr_warn_once("Memory pool empty, consider increasing CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE\n");
+		printk_deferred_exit();
+	}
 	raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
 
 	return object;
@@ -632,6 +645,11 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
 		else if (parent->pointer + parent->size <= ptr)
 			link = &parent->rb_node.rb_right;
 		else {
+			/*
+			 * Printk deferring due to the kmemleak_lock held.
+			 * This is done to avoid deadlock.
+			 */
+			printk_deferred_enter();
 			kmemleak_stop("Cannot insert 0x%lx into the object search tree (overlaps existing)\n",
 				      ptr);
 			/*
@@ -639,6 +657,7 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
 			 * be freed while the kmemleak_lock is held.
 			 */
 			dump_object_info(parent);
+			printk_deferred_exit();
 			kmem_cache_free(object_cache, object);
 			object = NULL;
 			goto out;
-- 
2.25.1

Re: [PATCH v2] mm: Fix possible deadlock in console_trylock_spinning

[Catalin Marinas]

On Thu, Aug 07, 2025 at 05:14:44PM +0800, Gu Bowen wrote:
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 4801751cb6b6..381145dde54f 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -390,9 +390,15 @@ static struct kmemleak_object *lookup_object(unsigned long ptr, int alias)
>  		else if (object->pointer == ptr || alias)
>  			return object;
>  		else {
> +			/*
> +			 * Printk deferring due to the kmemleak_lock held.
> +			 * This is done to avoid deadlock.
> +			 */
> +			printk_deferred_enter();
>  			kmemleak_warn("Found object by alias at 0x%08lx\n",
>  				      ptr);
>  			dump_object_info(object);
> +			printk_deferred_exit();
>  			break;
>  		}
>  	}

This hunk is fine.

> @@ -433,8 +439,15 @@ static struct kmemleak_object *mem_pool_alloc(gfp_t gfp)
>  		list_del(&object->object_list);
>  	else if (mem_pool_free_count)
>  		object = &mem_pool[--mem_pool_free_count];
> -	else
> +	else {
> +		/*
> +		 * Printk deferring due to the kmemleak_lock held.
> +		 * This is done to avoid deadlock.
> +		 */
> +		printk_deferred_enter();
>  		pr_warn_once("Memory pool empty, consider increasing CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE\n");
> +		printk_deferred_exit();
> +	}
>  	raw_spin_unlock_irqrestore(&kmemleak_lock, flags);

I wouldn't bother with printk deferring here, just set a bool warn
variable and report it after unlocking. We recently merged another patch
that does this.

>  
>  	return object;
> @@ -632,6 +645,11 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
>  		else if (parent->pointer + parent->size <= ptr)
>  			link = &parent->rb_node.rb_right;
>  		else {
> +			/*
> +			 * Printk deferring due to the kmemleak_lock held.
> +			 * This is done to avoid deadlock.
> +			 */
> +			printk_deferred_enter();
>  			kmemleak_stop("Cannot insert 0x%lx into the object search tree (overlaps existing)\n",
>  				      ptr);
>  			/*
> @@ -639,6 +657,7 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
>  			 * be freed while the kmemleak_lock is held.
>  			 */
>  			dump_object_info(parent);
> +			printk_deferred_exit();

This is part of __link_object(), called with the lock held, so easier to
defer the printing as above.

BTW, the function names in the diff don't match mainline. Which kernel
version is this patch based on?

With the second change above using a bool warn, feel free to add:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks.

-- 
Catalin

Re: [PATCH v2] mm: Fix possible deadlock in console_trylock_spinning

[Gu Bowen]

On 8/7/2025 11:11 PM, Catalin Marinas wrote:
> 
>> @@ -433,8 +439,15 @@ static struct kmemleak_object *mem_pool_alloc(gfp_t gfp)
>>   		list_del(&object->object_list);
>>   	else if (mem_pool_free_count)
>>   		object = &mem_pool[--mem_pool_free_count];
>> -	else
>> +	else {
>> +		/*
>> +		 * Printk deferring due to the kmemleak_lock held.
>> +		 * This is done to avoid deadlock.
>> +		 */
>> +		printk_deferred_enter();
>>   		pr_warn_once("Memory pool empty, consider increasing CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE\n");
>> +		printk_deferred_exit();
>> +	}
>>   	raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
> 
> I wouldn't bother with printk deferring here, just set a bool warn
> variable and report it after unlocking. We recently merged another patch
> that does this.
> 

That's fine, I will send another patch that does not include this part.

>>   
>>   	return object;
>> @@ -632,6 +645,11 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
>>   		else if (parent->pointer + parent->size <= ptr)
>>   			link = &parent->rb_node.rb_right;
>>   		else {
>> +			/*
>> +			 * Printk deferring due to the kmemleak_lock held.
>> +			 * This is done to avoid deadlock.
>> +			 */
>> +			printk_deferred_enter();
>>   			kmemleak_stop("Cannot insert 0x%lx into the object search tree (overlaps existing)\n",
>>   				      ptr);
>>   			/*
>> @@ -639,6 +657,7 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
>>   			 * be freed while the kmemleak_lock is held.
>>   			 */
>>   			dump_object_info(parent);
>> +			printk_deferred_exit();
> 
> This is part of __link_object(), called with the lock held, so easier to
> defer the printing as above.
> 
> BTW, the function names in the diff don't match mainline. Which kernel
> version is this patch based on?
> 
The kernel version of this patch is stable-5.10. This part of the code 
exists in function __link_object() in the mainline.