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

[Steven Rostedt <rostedt@kernel.org>]

On Wed, 6 May 2026 15:50:45 +0200
Jens Remus <jremus@linux.ibm.com> wrote:

> > 
> > mt_init() initializes the maple tree with flags set to 0. Since stack
> > unwinding typically occurs in profiling interrupts (NMIs) which must
> > traverse the maple tree locklessly via RCU, does this missing
> > MT_FLAGS_USE_RCU flag allow concurrent modifications to perform in-place
> > updates instead of RCU-safe copy-on-write allocations? This could result in
> > the NMI unwinder observing corrupted tree states.  
> 
> Please advise.

Sashiko is confused thinking that unwinders happen in NMI context. They
used to, but this will always be done from task context. Hopefully
Sashiko will figure this out soon (Cc'ing Chris and Roman about it).

> 
> > [ ... ]  
> >> @@ -1100,6 +1109,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
> >>  	mm->pmd_huge_pte = NULL;
> >>  #endif
> >>  	mm_init_uprobes_state(mm);
> >> +	mm_init_sframe(mm);
> >>  	hugetlb_count_init(mm);
> >>  
> >>  	mm_flags_clear_all(mm);  
> > 
> > When a process forks, this initializes an empty sframe_mt for the child's
> > mm_struct. While dup_mmap() duplicates the parent's VMAs, it doesn't appear
> > that the registered .sframe entries are propagated to the child's maple tree.
> > Will child processes silently lose their .sframe unwind data for shared
> > libraries registered prior to the fork?  
> 
> Does this ring any bell, Steven?  I ran into it with my signal frame
> stacktracing test on s390.  I think we discussed this during one of the
> SFrame calls and it got lost.  

Oh, this is a good point. I think we need to copy the sframe mt on
fork, not initialize it.




> >> +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);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>  int sframe_remove_section(unsigned long sframe_start)
> >>  {
> >> -	return -ENOSYS;
> >> +	struct mm_struct *mm = current->mm;
> >> +	struct sframe_section *sec;
> >> +	unsigned long index = 0;
> >> +	bool found = false;
> >> +	int ret = 0;
> >> +
> >> +	mt_for_each(&mm->sframe_mt, sec, index, ULONG_MAX) {
> >> +		if (sec->sframe_start == sframe_start) {  
> > 
> > Can concurrent calls to sframe_remove_section() cause a use-after-free and
> > subsequent double free?
> > 
> > mt_for_each() locklessly iterates mm->sframe_mt, and internally acquires and
> > drops the RCU read lock, meaning the returned sec pointer has no lifetime
> > protection in the loop body.
> > 
> > If two threads concurrently invoke sframe_remove_section(), both could
> > receive the exact same sec pointer from the tree. Thread A could then call
> > __sframe_remove_section(), erasing the entry and freeing sec via
> > free_section(). Thread B would then evaluate the if statement using the
> > freed sec pointer, causing a use-after-free read, and potentially proceeding
> > to free it again.  
> 
> Please advise.

I guess it's asking if we should have a read_srcu_lock()?

> 
> > Additionally, does free_section() need to use kfree_rcu() instead of
> > kfree() to prevent lockless NMI unwinders from hitting a use-after-free
> > when reading payloads?  

Again, this is Sashiko's confusion with thinking this can be called by
NMI (it can't!)

-- Steve