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

[Yeoreum Yun <yeoreum.yun@arm.com>]

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?

Thanks.

--
Sincerely,
Yeoreum Yun