On Fri, 2021-09-24 at 07:35 -0700, Sami Tolvanen wrote:

On Thu, Sep 23, 2021 at 4:19 AM Kuan-Ying Lee
[off-list ref] wrote:

quoted

On Thu, 2021-09-23 at 17:53 +0800, yee.lee@mediatek.com wrote:

quoted

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
  __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@gmail.com>
CC: Will Deacon <will@kernel.org>
CC: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Yee Lee <redacted>
---
 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 <redacted>

Sami

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek