2021-09-23 09:58:49

by Yee Lee (李建誼)

[permalink] [raw]
Subject: [PATCH] scs: Release kasan vmalloc poison in scs_free process

From: Yee Lee <[email protected]>

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 <[email protected]>
CC: Will Deacon <[email protected]>
CC: Sami Tolvanen <[email protected]>
Signed-off-by: Yee Lee <[email protected]>
---
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
--
2.18.0


Subject: Re: [PATCH] scs: Release kasan vmalloc poison in scs_free process

On Thu, 2021-09-23 at 17:53 +0800, [email protected] wrote:
> From: Yee Lee <[email protected]>
>
> 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 <[email protected]>
> CC: Will Deacon <[email protected]>
> CC: Sami Tolvanen <[email protected]>
> Signed-off-by: Yee Lee <[email protected]>
> ---
> 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.

2021-09-24 14:35:29

by Sami Tolvanen

[permalink] [raw]
Subject: Re: [PATCH] scs: Release kasan vmalloc poison in scs_free process

Hi,

On Thu, Sep 23, 2021 at 2:53 AM <[email protected]> wrote:
>
> From: Yee Lee <[email protected]>
>
> 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 <[email protected]>
> CC: Will Deacon <[email protected]>
> CC: Sami Tolvanen <[email protected]>
> Signed-off-by: Yee Lee <[email protected]>
> ---
> 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);

Thanks for the patch! I believe I had this in the original vmap SCS
series, but Will pointed out that unpoisoning shouldn't be necessary
before vfree_atomic() and I don't remember seeing this KASAN error
back then. I wonder if something changed there? Either way, the patch
looks good to me.

Reviewed-by: Sami Tolvanen <[email protected]>

Sami

2021-09-25 00:57:20

by Sami Tolvanen

[permalink] [raw]
Subject: Re: [PATCH] scs: Release kasan vmalloc poison in scs_free process

On Thu, Sep 23, 2021 at 4:19 AM Kuan-Ying Lee
<[email protected]> wrote:
>
> On Thu, 2021-09-23 at 17:53 +0800, [email protected] wrote:
> > From: Yee Lee <[email protected]>
> >
> > 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 <[email protected]>
> > CC: Will Deacon <[email protected]>
> > CC: Sami Tolvanen <[email protected]>
> > Signed-off-by: Yee Lee <[email protected]>
> > ---
> > 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

Subject: Re: [PATCH] scs: Release kasan vmalloc poison in scs_free process

On Fri, 2021-09-24 at 07:35 -0700, Sami Tolvanen wrote:
> On Thu, Sep 23, 2021 at 4:19 AM Kuan-Ying Lee
> <[email protected]> wrote:
> >
> > On Thu, 2021-09-23 at 17:53 +0800, [email protected] wrote:
> > > From: Yee Lee <[email protected]>
> > >
> > > 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 <[email protected]>
> > > CC: Will Deacon <[email protected]>
> > > CC: Sami Tolvanen <[email protected]>
> > > Signed-off-by: Yee Lee <[email protected]>
> > > ---
> > > 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.

Got it.
Thanks.

Reviewed-by: Kuan-Ying Lee <[email protected]>

>
> Sami

2021-09-29 12:04:23

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] scs: Release kasan vmalloc poison in scs_free process

On Thu, Sep 23, 2021 at 05:53:12PM +0800, [email protected] wrote:
> From: Yee Lee <[email protected]>
>
> 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

Thanks, I can reproduce this easily with mainline. We only recently gained
vmalloc support for kasan on arm64, so that's probably why we didn't spot
this earlier.

> 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 don't think we should be unpoisoning the stack pages if we're putting
them back on to the local cache -- unpoisoning happens on the alloc path
in that case anyway so that we can zero them.

So how about sending a v2 with this moved a bit later (see below)? With
that change:

Acked-by: Will Deacon <[email protected]>
Tested-by: Will Deacon <[email protected]>

Thanks,

Will

--->8

diff --git a/kernel/scs.c b/kernel/scs.c
index e2a71fc82fa0..579841be8864 100644
--- a/kernel/scs.c
+++ b/kernel/scs.c
@@ -78,6 +78,7 @@ void scs_free(void *s)
if (this_cpu_cmpxchg(scs_cache[i], 0, s) == NULL)
return;

+ kasan_unpoison_vmalloc(s, SCS_SIZE);
vfree_atomic(s);
}

2022-05-23 07:49:28

by Yee Lee (李建誼)

[permalink] [raw]
Subject: Re: [PATCH] scs: Release kasan vmalloc poison in scs_free process

Test
On Thu, 2021-09-23 at 17:53 +0800, [email protected] wrote:
> From: Yee Lee <[email protected]>
>
> 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 <[email protected]>
> CC: Will Deacon <[email protected]>
> CC: Sami Tolvanen <[email protected]>
> Signed-off-by: Yee Lee <[email protected]>
> ---
> 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