linux-kernel - Re: [PATCH] scs: Release kasan vmalloc poison in scs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABCJKucWe-C7i1+k42uV4TgeD8Q7NBoMrXxdyV85oPE9Tibdrg@mail.gmail.com>
Date:   Fri, 24 Sep 2021 07:35:19 -0700
From:   Sami Tolvanen <samitolvanen@...gle.com>
To:     Kuan-Ying Lee <Kuan-Ying.Lee@...iatek.com>
Cc:     yee.lee@...iatek.com, LKML <linux-kernel@...r.kernel.org>,
        nicholas.Tang@...iatek.com, chinwen.chang@...iatek.com,
        Matthias Brugger <matthias.bgg@...il.com>,
        Will Deacon <will@...nel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH] scs: Release kasan vmalloc poison in scs_free process

On Thu, Sep 23, 2021 at 4:19 AM Kuan-Ying Lee
<Kuan-Ying.Lee@...iatek.com> wrote:
>
> On Thu, 2021-09-23 at 17:53 +0800, yee.lee@...iatek.com wrote:
> > From: Yee Lee <yee.lee@...iatek.com>
> >
> > Since scs allocation has been moved to vmalloc region, the
> > shadow stack is protected by kasan_posion_vmalloc.
> > However, the vfree_atomic operation needs to access
> > its context for scs_free process and causes kasan error
> > as the dump info below.
> >
> > This patch Adds kasan_unpoison_vmalloc() before vfree_atomic,
> > which aligns to the prior flow as using kmem_cache.
> > The vmalloc region will go back posioned in the following
> > vumap() operations.
> >
> >  ==================================================================
> >  BUG: KASAN: vmalloc-out-of-bounds in llist_add_batch+0x60/0xd4
> >  Write of size 8 at addr ffff8000100b9000 by task kthreadd/2
> >
> >  CPU: 0 PID: 2 Comm: kthreadd Not tainted 5.15.0-rc2-11681-(skip)
> >  Hardware name: linux,dummy-virt (DT)
> >  Call trace:
> >   dump_backtrace+0x0/0x43c
> >   show_stack+0x1c/0x2c
> >   dump_stack_lvl+0x68/0x84
> >   print_address_description+0x80/0x394
> >   kasan_report+0x180/0x1dc
> >   __asan_report_store8_noabort+0x48/0x58
> >   llist_add_batch+0x60/0xd4
> >   vfree_atomic+0x60/0xe0
> >   scs_free+0x1dc/0x1fc
> >   scs_release+0xa4/0xd4
> >   free_task+0x30/0xe4
> >   __put_task_struct+0x1ec/0x2e0
> >   delayed_put_task_struct+0x5c/0xa0
> >   rcu_do_batch+0x62c/0x8a0
> >   rcu_core+0x60c/0xc14
> >   rcu_core_si+0x14/0x24
> >   __do_softirq+0x19c/0x68c
> >   irq_exit+0x118/0x2dc
> >   handle_domain_irq+0xcc/0x134
> >   gic_handle_irq+0x7c/0x1bc
> >   call_on_irq_stack+0x40/0x70
> >   do_interrupt_handler+0x78/0x9c
> >   el1_interrupt+0x34/0x60
> >   el1h_64_irq_handler+0x1c/0x2c
> >   el1h_64_irq+0x78/0x7c
> >   _raw_spin_unlock_irqrestore+0x40/0xcc
> >   sched_fork+0x4f0/0xb00
> >   copy_process+0xacc/0x3648
> >   kernel_clone+0x168/0x534
> >   kernel_thread+0x13c/0x1b0
> >   kthreadd+0x2bc/0x400
> >   ret_from_fork+0x10/0x20
> >
> >  Memory state around the buggy address:
> >   ffff8000100b8f00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >   ffff8000100b8f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >  >ffff8000100b9000: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >                     ^
> >   ffff8000100b9080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >   ffff8000100b9100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >  ==================================================================
> >
> > CC: Matthias Brugger <matthias.bgg@...il.com>
> > CC: Will Deacon <will@...nel.org>
> > CC: Sami Tolvanen <samitolvanen@...gle.com>
> > Signed-off-by: Yee Lee <yee.lee@...iatek.com>
> > ---
> >  kernel/scs.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/kernel/scs.c b/kernel/scs.c
> > index e2a71fc82fa0..25c0d8e416e6 100644
> > --- a/kernel/scs.c
> > +++ b/kernel/scs.c
> > @@ -68,6 +68,7 @@ void scs_free(void *s)
> >
> >       __scs_account(s, -1);
> >
> > +     kasan_unpoison_vmalloc(s, SCS_SIZE);
> >       /*
> >        * We cannot sleep as this can be called in interrupt context,
> >        * so use this_cpu_cmpxchg to update the cache, and
> > vfree_atomic
>
> I'm not sure if we need to add kasan_unpoison_vmalloc() and
> kasan_poison_vmalloc() in this file.
>
> As far as I know, vmalloc() and vfree() will do these two functions.

The idea here is to poison the shadow stack after it's set up to catch
unintentional accesses. Outside of compiler instrumentation, nothing
should read or write from this buffer while the task is running.

Sami