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

[Haibo Li <haibo.li@mediatek.com>]

> > On Thu, Sep 14, 2023 at 10:41 PM Jann Horn <jannh@google.com> wrote:
> > >
> > > > 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.
> > >
> > > FWIW, AFAICS kasan_non_canonical_hook() currently only does anything
> > > under CONFIG_KASAN_INLINE;
> > 
> > Ah, right. I was thinking about the inline mode, but the patch refers
> > to the issue with the outline mode.
> > 
> > However, I just checked kasan_non_canonical_hook for SW_TAGS with the
> > inline mode: it does not work when accessing 0x42ffffb80aaaaaaa, the
> > addr < KASAN_SHADOW_OFFSET check fails. It appears there's something
> > unusual about how instrumentation calculates the shadow address. I
> > didn't investigate further yet.
Sorry to miss this message.
I checked inline mode just now.kasan_non_canonical_hook can print 
something like below:
Unable to handle kernel paging request at virtual address ffffffb80aaaaaaa
KASAN: maybe wild-memory-access in range [0xffffff80aaaaaaa0-0xffffff80aaaaaaaf]
...
[ffffffb80aaaaaaa] pgd=000000005d3d6003, p4d=000000005d3d6003, pud=000000005d3d6003,
pmd=0000000000000000
...
pc : __hwasan_check_x20_67043363+0x4/0x34
lr : do_ib_ob+0x108/0x114
...
Call trace:
 __hwasan_check_x20_67043363+0x4/0x34
 die_selftest+0x68/0x80
 param_attr_store+0xec/0x164
 module_attr_store+0x34/0x4c
 sysfs_kf_write+0x78/0x8c
 kernfs_fop_write_iter+0x154/0x214
 vfs_write+0x36c/0x4c4
 ksys_write+0x98/0x110
 __arm64_sys_write+0x3c/0x48
 invoke_syscall+0x58/0x154
 el0_svc_common+0xe8/0x120
 do_el0_svc_compat+0x2c/0x38
 el0_svc_compat+0x34/0x84
 el0t_32_sync_handler+0x78/0xb4
 el0t_32_sync+0x194/0x198

When addr < KASAN_SHADOW_OFFSET meets,the original addr_has_metadata should return false
and trigger kasan_report in kasan_check_range.
> > 
> > > I think the idea when I added that was that
> > > it assumes that when KASAN checks an access in out-of-line
> > > instrumentation or a slowpath, it will do the required checks to avoid
> > > this kind of fault?
> > 
> > Ah, no, KASAN doesn't do it.
> > 
> > However, I suppose we could add what the original patch proposes for
> > the outline mode. For the inline mode, it seems to be pointless, as
> > most access checks happen though the compiler inserted code anyway.
> > 
> > I also wonder how much slowdown this patch will introduce.
> > 
> > Haibo, could you check how much slower the kernel becomes with your
> > patch? If possible, with all GENERIC/SW_TAGS and INLINE/OUTLINE
> > combinations.
> > 
> > If the slowdown is large, we can just make kasan_non_canonical_hook
> > work for both modes (and fix it for SW_TAGS).
> 
> Thanks.
> The patch checks each shadow address,so it introduces extra overhead.
> Now kasan_non_canonical_hook only works for CONFIG_KASAN_INLINE.
> And CONFIG_KASAN_OUTLINE is set in my case.
> Is it possible to make kasan_non_canonical_hook works for both 
> INLINE and OUTLINE by simply remove the "#ifdef CONFIG_KASAN_INLINE"?
> Since kasan_non_canonical_hook is only used after kernel fault,it 
> is better if there is no limit.