Re: [PATCH] kasan: don't call find_vm_area() in in_interrupt() for possible deadlock

[Andrey Ryabinin <ryabinin.a.a@gmail.com>]

On 7/2/25 6:04 PM, Yeoreum Yun wrote:
> Hi Andrey,
> 
>>
>> FYI some of email addresses in CC look corrupted, e.g. "kpm@linux-foundation.org", "nd@arm.com"
> 
> Sorry and Thanks to let me know :)
> 
>>
>>> In below senario, kasan causes deadlock while reporting vm area informaion:
>>>
>>> CPU0                                CPU1
>>> vmalloc();
>>>  alloc_vmap_area();
>>>   spin_lock(&vn->busy.lock)
>>>                                     spin_lock_bh(&some_lock);
>>>    <interrupt occurs>
>>>    <in softirq>
>>>    spin_lock(&some_lock);
>>>                                     <access invalid address>
>>>                                     kasan_report();
>>>                                      print_report();
>>>                                       print_address_description();
>>>                                        kasan_find_vm_area();
>>>                                         find_vm_area();
>>>                                          spin_lock(&vn->busy.lock) // deadlock!
>>>
>> ...
>>
>>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>>> index 8357e1a33699..61c590e8005e 100644
>>> --- a/mm/kasan/report.c
>>> +++ b/mm/kasan/report.c
>>> @@ -387,7 +387,7 @@ static inline struct vm_struct *kasan_find_vm_area(void *addr)
>>>  	static DEFINE_WAIT_OVERRIDE_MAP(vmalloc_map, LD_WAIT_SLEEP);
>>>  	struct vm_struct *va;
>>>
>>> -	if (IS_ENABLED(CONFIG_PREEMPT_RT))
>>> +	if (IS_ENABLED(CONFIG_PREEMPT_RT) || in_interrupt())
>>
>> in_interrupt() returns true if BH disabled, so this indeed should avoid the deadlock.
>> However, it seems we have similar problem with 'spin_lock_irq[save](&some_lock)' case and
>> in_interrupt() check doesn't fix it.
>> And adding irqs_disabled() check wouldn't make sense because print_report() always
>>  runs with irqs disabled.
>> I see no obvious way to fix this rather than remove find_vm_area() call completely and just
>> print less info.
> 
> Right. unless there is API -- find_vm_area() with spin_trylock(),
> kasan_find_vm_area() should be removed.
> 
> But, I'm not sure adding the new API used only bv kasan is better then
> just remove kasan_find_vm_area().
> 
> Do you have any idea for this?
> 

I'd say the info from vm_struct is nice to have, but it's not essential for debugging.
It's probably not worth trying to keep it, so I'd vote for for removing kasan_find_vm_area().


> Thanks.
> 
> --
> Sincerely,
> Yeoreum Yun