Re: [PATCH] kasan:fix access invalid shadow address when input is illegal

[Andrey Konovalov <andreyknvl@gmail.com>]

On Thu, Sep 14, 2023 at 8:29 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > --- a/mm/kasan/kasan.h
> > +++ b/mm/kasan/kasan.h
> > @@ -304,8 +304,17 @@ static __always_inline bool addr_has_metadata(const void *addr)
> >  #ifdef __HAVE_ARCH_SHADOW_MAP
> >       return (kasan_mem_to_shadow((void *)addr) != NULL);
> >  #else
> > -     return (kasan_reset_tag(addr) >=
> > -             kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> > +     u8 *shadow, shadow_val;
> > +
> > +     if (kasan_reset_tag(addr) <
> > +             kasan_shadow_to_mem((void *)KASAN_SHADOW_START))
> > +             return false;
> > +     /* use read with nofault to check whether the shadow is accessible */
> > +     shadow = kasan_mem_to_shadow((void *)addr);
> > +     __get_kernel_nofault(&shadow_val, shadow, u8, fault);
> > +     return true;
> > +fault:
> > +     return false;
> >  #endif
> >  }
>
> Are we able to identify a Fixes: target for this?
> 9d7b7dd946924de43021f57a8bee122ff0744d93 ("kasan: split out
> print_report from __kasan_report") altered the code but I expect the
> bug was present before that commit.
>
> Seems this bug has been there for over a year.  Can you suggest why it
> has been discovered after such a lengthy time?

Accessing unmapped memory with KASAN always led to a crash when
checking shadow memory. This was reported/discussed before. To improve
crash reporting for this case, Jann added kasan_non_canonical_hook and
Mark integrated it into arm64. But AFAIU, for some reason, it stopped
working.

Instead of this patch, we need to figure out why
kasan_non_canonical_hook stopped working and fix it.

This approach taken by this patch won't work for shadow checks added
by compiler instrumentation. It only covers explicitly checked
accesses, such as via memcpy, etc.