2021-09-30 08:36:43

by Yee Lee (李建誼)

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

From: Yee Lee <[email protected]>

Since scs allocation is 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-g92477dd1faa6-dirty #1
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
==================================================================

Suggested-by: Kuan-Ying Lee <[email protected]>
Reviewd-by: Will Deacon <[email protected]>
Reviewd-by: Sami Tolvanen <[email protected]>
Signed-off-by: Yee Lee <[email protected]>
---
v2:
- changed the unpoison call to be followed by vfree_atomic

---
kernel/scs.c | 1 +
1 file changed, 1 insertion(+)

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);
}

--
2.18.0


2021-09-30 08:41:02

by Will Deacon

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

On Thu, Sep 30, 2021 at 04:16:13PM +0800, [email protected] wrote:
> From: Yee Lee <[email protected]>
>
> Since scs allocation is 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-g92477dd1faa6-dirty #1
> 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
> ==================================================================

Thanks, I'll take this via the arm64 tree as we're the only use of SCS.

One thing for future:

> Suggested-by: Kuan-Ying Lee <[email protected]>
> Reviewd-by: Will Deacon <[email protected]>

I gave an "Acked-by" and a "Tested-by" at [1], so those are the tags you
should be using. Please don't convert them into a "Reviewed-by".

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

This should be "Reviewed-by" (you have a typo).

Anyway, I'll fix these locally, no need to resend this time.

Will

[1] https://lore.kernel.org/r/20210929115447.GA21631@willie-the-truck

2021-09-30 09:01:01

by Will Deacon

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

On Thu, 30 Sep 2021 16:16:13 +0800, [email protected] wrote:
> From: Yee Lee <[email protected]>
>
> Since scs allocation is 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.
>
> [...]

Applied to arm64 (for-next/scs), thanks!

[1/1] scs: Release kasan vmalloc poison in scs_free process
https://git.kernel.org/arm64/c/528a4ab45300

Cheers,
--
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

2021-09-30 09:11:06

by Yee Lee (李建誼)

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

Got it. Thank you.

Yee

On Thu, 2021-09-30 at 09:35 +0100, Will Deacon wrote:
> On Thu, Sep 30, 2021 at 04:16:13PM +0800, [email protected] wrote:
> > From: Yee Lee <[email protected]>
> >
> > Since scs allocation is 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-
> > g92477dd1faa6-dirty #1
> > 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
> > ==================================================================
>
> Thanks, I'll take this via the arm64 tree as we're the only use of
> SCS.
>
> One thing for future:
>
> > Suggested-by: Kuan-Ying Lee <[email protected]>
> > Reviewd-by: Will Deacon <[email protected]>
>
> I gave an "Acked-by" and a "Tested-by" at [1], so those are the tags
> you
> should be using. Please don't convert them into a "Reviewed-by".
>
> > Reviewd-by: Sami Tolvanen <[email protected]>
>
> This should be "Reviewed-by" (you have a typo).
>
> Anyway, I'll fix these locally, no need to resend this time.
>
> Will
>
> [1]
> https://urldefense.com/v3/__https://lore.kernel.org/r/20210929115447.GA21631@willie-the-truck__;!!CTRNKA9wMg0ARbw!yjJi1VvoIbI7CZQJplT_jdN087Pkk6pzXAZ12_FpegZzaUEQutkzBjX85rub_NI$
>