Re: [PATCH v10 02/11] unwind_user/sframe: Store sframe section data in per-mm maple tree

["Liam R. Howlett" <Liam.Howlett@oracle.com>]

* Steven Rostedt <rostedt@goodmis.org> [250828 10:28]:
> On Wed, 27 Aug 2025 21:46:01 -0400
> "Liam R. Howlett" <Liam.Howlett@oracle.com> wrote:
> 
> > >  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) {
> > > +			found = true;
> > > +			ret |= __sframe_remove_section(mm, sec);
> > > +		}
> > > +	}  
> > 
> 
> Josh should be able to answer this better than I can, as he wrote it, and
> I'm not too familiar with how to use maple tree (reading the documentation
> now).
> 
> > If you use the advanced interface you have to handle the locking, but it
> > will be faster.  I'm not sure how frequent you loop across many entries,
> > but you can do something like:
> > 
> > MA_SATE(mas, &mm->sframe_mt, index, index);
> > 
> > mas_lock(&mas);
> > mas_for_each(&mas, sec, ULONG_MAX) {
> > ...
> > }
> > mas_unlock(&mas);
> > 
> > The maple state contains memory addresses of internal nodes, so you
> > cannot just edit the tree without it being either unlocked (which
> > negates the gains you would have) or by using it in the modification.
> > 
> > This seems like a good choice considering the __sframe_remove_section()
> > is called from only one place. You can pass the struct ma_state through
> > to the remove function and use it with mas_erase().
> > 
> > Actually, reading it again,  why are you starting a search at 0?  And
> > why are you deleting everything after the sframe_start to ULONG_MAX?
> > This seems incorrect.  Can you explain your plan a bit here?
> 
> Let me give a brief overview of how and why maple trees are used for
> sframes:
> 
> The sframe section is mapped to the user space address from the elf file
> when the application starts. The dynamic library loader could also do a
> system call to tell the kernel where the sframe is for some dynamically
> loaded code. Since there can be more than one text section that has an
> sframe associated to it, the mm->sframe_mt is used to hold the range of
> text to find its corresponding sframe section. That is, there's one sframe
> section for the code that was loaded during exec(), and then there may be a
> separate sframe section for every library that is loaded. Note, it is
> possible that the same sframe section may cover more than one range of text.
> 
> When doing stack walking, the instruction pointer is used as the key in the
> maple tree to find its corresponding sframe section.
> 
> Now, if the sframe is determined to be corrupted, it must be removed from
> the current->mm->sframe_mt. It also gets removed when the dynamic loader
> removes some text from the application that has the code.
> 
> I'm guessing that the 0 to ULONG_MAX is to simply find and remove all the
> associated sframe sections, as there may be more than one text range that a
> single sframe section covers.
> 
> Does this make sense?
> 

Perhaps it's the corruption part that I'm missing here.  If the sframe
is corrupt, you are iterating over all elements and checking the start
address passed in against the section start.

So if the section is corrupted then how can we depend on the
sec->sframe_start?

And is the maple tree corrupted?  I mean, the mappings to sframe_start
-> sec is still reliable, right?

Looking at the storing code, you store text_start - text_end to sec,
presumably the text_start cannot be smaller than the sframe_start?

Thanks,
Liam