Re: [PATCH v15 03/20] unwind_user/sframe: Store .sframe section data in per-mm maple tree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing `MT_FLAGS_USE_RCU` during maple tree initialization causes Use-After-Free of tree internal nodes.
- [High] Synchronous `kfree()` of `sframe_section` bypasses SRCU/RCU grace periods, causing Use-After-Free for concurrent readers.
- [Low] Undeclared identifier `sframe_srcu` causes build failure.
- [Low] Macro expansion of `INIT_MM_SFRAME` breaks compilation on non-x86 architectures.
--

commit ce5ec4616b79b2c0aa851c150b33143b5d3876af
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Store .sframe section data in per-mm maple tree

Associate an .sframe section with its mm by adding it to a per-mm maple
tree which is indexed by the corresponding text address range. A single
.sframe section can be associated with multiple text ranges.

> diff --git a/include/linux/sframe.h b/include/linux/sframe.h
[ ... ]
> @@ -19,18 +21,31 @@ struct sframe_section {
>  	signed char	fp_off;
>  };
>  
> +#define INIT_MM_SFRAME .sframe_mt = MTREE_INIT(sframe_mt, 0),

[Severity: High]
Does this maple tree need to be initialized with MT_FLAGS_USE_RCU?

Without it, the tree operates in non-RCU mode, so lockless iterations
might race with in-place node modifications and synchronous node frees,
potentially causing use-after-free of the internal nodes.

> diff --git a/kernel/fork.c b/kernel/fork.c
[ ... ]
> @@ -1073,6 +1075,13 @@ static void mmap_init_lock(struct mm_struct *mm)
>  #endif
>  }
>  
> +static void mm_init_sframe(struct mm_struct *mm)
> +{
> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> +	mt_init(&mm->sframe_mt);

[Severity: High]
Similar to MTREE_INIT above, does mt_init() need to use mt_init_flags()
with MT_FLAGS_USE_RCU here to prevent use-after-free during lockless reads?

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
[ ... ]
> +static int __sframe_remove_section(struct mm_struct *mm,
> +				   struct sframe_section *sec)
> +{
> +	if (!mtree_erase(&mm->sframe_mt, sec->text_start)) {
> +		dbg("mtree_erase failed: text=%lx\n", sec->text_start);
> +		return -EINVAL;
> +	}
> +
> +	free_section(sec);

[Severity: High]
Can this synchronous free cause a use-after-free for concurrent readers?

If an unwinder or other thread is iterating the tree locklessly and has
already obtained the section pointer before it is erased, freeing it
immediately here without waiting for an RCU or SRCU grace period would
pull the structure out from under them.

> +
> +	return 0;
> +}
> +
>  int sframe_remove_section(unsigned long sframe_start)
>  {
[ ... ]
> +	struct mm_struct *mm = current->mm;
> +	struct sframe_section *sec;
> +	unsigned long index = 0;
> +	bool found = false;
> +	int ret = 0;
> +
> +	guard(srcu)(&sframe_srcu);

[Severity: Low]
Is sframe_srcu defined anywhere? It doesn't appear to be declared in this
file or its headers, which would cause a build failure.

> diff --git a/mm/init-mm.c b/mm/init-mm.c
[ ... ]
> @@ -49,6 +50,7 @@ struct mm_struct init_mm = {
>  #endif
>  	.flexible_array	= MM_STRUCT_FLEXIBLE_ARRAY_INIT,
>  	INIT_MM_CONTEXT(init_mm)
> +	INIT_MM_SFRAME
>  };

[Severity: Low]
Can this cause compilation failures on non-x86 architectures?

If architectures like PowerPC or UML define INIT_MM_CONTEXT without a
trailing comma, placing INIT_MM_SFRAME (which expands to .sframe_mt = ...)
immediately after it might result in a missing-comma syntax error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=3