Re: [PATCH] scs: Release kasan vmalloc poison in scs_free process
From: Will Deacon <will@kernel.org>
Date: 2021-09-29 11:55:14
Also in:
linux-mediatek, lkml
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Thu, Sep 23, 2021 at 05:53:12PM +0800, yee.lee@mediatek.com wrote:
From: Yee Lee <redacted> 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.
quoted hunk ↗ jump to hunk
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 <will@kernel.org> Tested-by: Will Deacon <will@kernel.org> 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); }
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel