Re: [PATCH v4] mm: Fix possible deadlock in kmemleak

[Waiman Long <llong@redhat.com>]

On 8/20/25 7:02 AM, Catalin Marinas wrote:
> On Tue, Aug 19, 2025 at 11:27:23PM -0400, Waiman Long wrote:
>> On 8/18/25 5:09 AM, Gu Bowen wrote:
>>> @@ -858,8 +870,14 @@ static void delete_object_part(unsigned long ptr, size_t size,
>>>    	object = __find_and_remove_object(ptr, 1, objflags);
>>>    	if (!object) {
>>>    #ifdef DEBUG
>>> +		/*
>>> +		 * Printk deferring due to the kmemleak_lock held.
>>> +		 * This is done to avoid deadlock.
>>> +		 */
>>> +		printk_deferred_enter();
>>>    		kmemleak_warn("Partially freeing unknown object at 0x%08lx (size %zu)\n",
>>>    			      ptr, size);
>>> +		printk_deferred_exit();
>>>    #endif
>> This particular warning message can be moved after unlock by adding a
>> warning flag. Locking is done outside of the other two helper functions
>> above, so it is easier to use printk_deferred_enter/exit() for those.
> I thought about this as well but the above is under an #ifdef DEBUG so
> we end up adding more lines on the unlock path (not sure which one looks
> better; I'd say the above, marginally).
>
> Another option would be to remove the #ifdef and try to identify the
> call sites that trigger the warning. Last time I checked (many years
> ago) they were fairly benign and decided to hide them before an #ifdef.

A bit more code change required the printing is moved.

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 84265983f239..eb4e0af5edba 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -856,13 +856,8 @@ static void delete_object_part(unsigned long ptr, 
size_t size,

         raw_spin_lock_irqsave(&kmemleak_lock, flags);
         object = __find_and_remove_object(ptr, 1, objflags);
-       if (!object) {
-#ifdef DEBUG
-               kmemleak_warn("Partially freeing unknown object at 
0x%08lx (size %zu)\n",
-                             ptr, size);
-#endif
+       if (!object)
                 goto unlock;
-       }

         /*
          * Create one or two objects that may result from the memory block
@@ -882,8 +877,14 @@ static void delete_object_part(unsigned long ptr, 
size_t size,

  unlock:
         raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
-       if (object)
+       if (object) {
                 __delete_object(object);
+       } else {
+#ifdef DEBUG
+               kmemleak_warn("Partially freeing unknown object at 
0x%08lx (size %zu)\n",
+                             ptr, size);
+#endif
+       }

Anyway, I am not against using printk_deferred_enter/exit here. It is 
just that they should be used as a last resort if there is no easy way 
to work around it.

Cheers,
Longman