Re: [PATCH v2] fork: reset pointer tag of vmapped thread stack before vfree

[Andrey Ryabinin]

Shaobo Huang <huangshaobo3@xxxxxxxxxx> writes:

> thread_stack_free_rcu() frees the vmalloc'd thread stack via
> vfree(vm_area->addr).  In RCU callback context, vfree() routes to
> vfree_atomic(), which calls llist_add((struct llist_node *)addr, ...)
> and writes 8 bytes to the base of the region being freed.
>
> With KASAN_SW_TAGS, vm_area->addr carries a random tag.  If
> kasan_unpoison_task_stack_below() has rewritten the shadow covering
> [base, sp] to KASAN_TAG_KERNEL (0xff) -- which it does on every CPU
> resume for the current task's stack -- the llist_add store checks
> shadow[base] (0xff) against the pointer tag (random) and reports an
> invalid-access, although writing to the base of a stack queued for
> deferred free is legitimate.
>
> Reset the pointer tag to KASAN_TAG_KERNEL before vfree() so that
> kasan_check_range() short-circuits the check, the same way the task
> accesses its own stack at runtime via sp.  The vmalloc lookup is safe:
> __find_vmap_area() resets the tag before comparing against va_start.
>
> Fixes: 9f7d416c3612 ("kprobes: Unpoison stack in jprobe_return() for KASAN")

This commit merely renamed the resume helper without changing what it
unpoisons. Only generic KASAN existed back then, so it cannot have
introduced a tagged-mode problem.

The problem became visible with

  449e0b4ed5a1 ("fork: clean-up naming of vm_stack/vm_struct variables
in vmap stacks code")

which changed thread_stack_free_rcu() from vfree(vm_stack) to
vfree(vm_area->addr). vm_stack is derived from tsk->stack, whose tag
alloc_thread_stack_node() resets to 0xff. Since that commit, vfree_atomic()
gets the tagged vm_area->addr instead.

> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: zhipuai:glm-5.2
> Signed-off-by: Shaobo Huang <huangshaobo3@xxxxxxxxxx>
> ---
> Changes since v1:
> - Drop the 12-line comment; keep just the one-line fix.
> - Fix the Fixes: tag to point to the commit that introduced
>   kasan_unpoison_task_stack_below (9f7d416c3612), which added
>   both the function definition and the _cpu_resume call site, not
>   the 2016 vfree_atomic commit.
> - Add Assisted-by tag per Documentation/process/coding-assistants.rst.
> - Use real name (Shaobo Huang) instead of "sparkhuang".
> - Trim the commit message; remove the full KASAN dump.
> v1:
>   https://lore.kernel.org/all/20260806123020.90869-1-huangshaobo3@xxxxxxxxxx/
> ---
>  kernel/fork.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 45300f59cf2c..9a66b10749de 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -238,7 +238,7 @@ static void thread_stack_free_rcu(struct rcu_head *rh)
>  	if (try_release_thread_stack_to_cache(vm_stack->stack_vm_area))
>  		return;
>
> -	vfree(vm_area->addr);
> +	vfree(kasan_reset_tag(vm_area->addr));

Thanks for report the problem and the patch. NAK on the fix though.
It addresses the symptom rather than the cause.

The problem is not that vfree() is called with a tagged pointer. The
problem is the 0xff written into the shadow by
kasan_unpoison_task_stack_below().

That helper is only needed in generic mode. There the compiler expects
the stack shadow to be zero, so the redzones left behind by discarded
frames must be cleared before the stack is reused.

The tagged mode has no such requirement. The compiler fully initializes
the shadow of stack variables on function entry, so stale tags left
below the watermark are harmless.

kasan_unpoison_task_stack_below() should simply be a no-op in tagged
mode. I'll send a patch.