| Message ID | f50c5f96ef896d7936192c888b0c0a7674e33184.1644943792.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [mm] fix for "kasan, fork: reset pointer tags of vmapped stacks" | expand |
On Tue, 15 Feb 2022 at 17:52, <andrey.konovalov@linux.dev> wrote: > > From: Andrey Konovalov <andreyknvl@google.com> > > That patch didn't update the case when a stack is retrived from > cached_stacks in alloc_thread_stack_node(). As cached_stacks stores > vm_structs and not stack pointers themselves, the pointer tag needs > to be reset there as well. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Reviewed-by: Marco Elver <elver@google.com> Did the test catch this? If not, can this be tested? > --- > kernel/fork.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index 57d624f05182..5e3ad2e7a756 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -226,15 +226,17 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node) > if (!s) > continue; > > - /* Mark stack accessible for KASAN. */ > + /* Reset stack metadata. */ > kasan_unpoison_range(s->addr, THREAD_SIZE); > > + stack = kasan_reset_tag(s->addr); > + > /* Clear stale pointers from reused stack. */ > - memset(s->addr, 0, THREAD_SIZE); > + memset(stack, 0, THREAD_SIZE); > > tsk->stack_vm_area = s; > - tsk->stack = s->addr; > - return s->addr; > + tsk->stack = stack; > + return stack; > } > > /* > -- > 2.25.1 >
On Wed, Feb 16, 2022 at 10:59 AM Marco Elver <elver@google.com> wrote: > > On Tue, 15 Feb 2022 at 17:52, <andrey.konovalov@linux.dev> wrote: > > > > From: Andrey Konovalov <andreyknvl@google.com> > > > > That patch didn't update the case when a stack is retrived from > > cached_stacks in alloc_thread_stack_node(). As cached_stacks stores > > vm_structs and not stack pointers themselves, the pointer tag needs > > to be reset there as well. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > Reviewed-by: Marco Elver <elver@google.com> > > Did the test catch this? If not, can this be tested? Kind of, the kernel crashes on boot. I got KASAN_STACK accidentally disabled in my SW_TAGS config, so I didn't see the crash until now.
diff --git a/kernel/fork.c b/kernel/fork.c index 57d624f05182..5e3ad2e7a756 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -226,15 +226,17 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node) if (!s) continue; - /* Mark stack accessible for KASAN. */ + /* Reset stack metadata. */ kasan_unpoison_range(s->addr, THREAD_SIZE); + stack = kasan_reset_tag(s->addr); + /* Clear stale pointers from reused stack. */ - memset(s->addr, 0, THREAD_SIZE); + memset(stack, 0, THREAD_SIZE); tsk->stack_vm_area = s; - tsk->stack = s->addr; - return s->addr; + tsk->stack = stack; + return stack; } /*