| Date | Thu, 2 Dec 2021 15:27:58 +0100 | From | Marco Elver <> | Subject | Re: [PATCH 21/31] kasan, fork: don't tag stacks allocated with vmalloc |
| |
On Tue, Nov 30, 2021 at 11:07PM +0100, andrey.konovalov@linux.dev wrote: > From: Andrey Konovalov <andreyknvl@google.com> > > Once tag-based KASAN modes start tagging vmalloc() allocations, > kernel stacks will start getting tagged if CONFIG_VMAP_STACK is enabled. > > Reset the tag of kernel stack pointers after allocation. > > For SW_TAGS KASAN, when CONFIG_KASAN_STACK is enabled, the > instrumentation can't handle the sp register being tagged. > > For HW_TAGS KASAN, there's no instrumentation-related issues. However, > the impact of having a tagged SP pointer needs to be properly evaluated, > so keep it non-tagged for now.
Don't VMAP_STACK stacks have guards? So some out-of-bounds would already be caught.
What would be the hypothetical benefit of using a tagged stack pointer? Perhaps wildly out-of-bounds accesses derived from stack pointers?
I agree that unless we understand the impact of using a tagged stack pointers, it should remain non-tagged for now.
> Note, that the memory for the stack allocation still gets tagged to > catch vmalloc-into-stack out-of-bounds accesses.
Will the fact it's tagged cause issues for other code? I think kmemleak already untags all addresses it scans for pointers. Anything else?
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > --- > kernel/fork.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/fork.c b/kernel/fork.c > index 3244cc56b697..062d1484ef42 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -253,6 +253,7 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node) > * so cache the vm_struct. > */ > if (stack) { > + stack = kasan_reset_tag(stack); > tsk->stack_vm_area = find_vm_area(stack); > tsk->stack = stack; > } > -- > 2.25.1 >
|