Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: stacktrace_save_utrace() should be more robust
To:
Alexandr Nedvedicky <sashan@fastmail.net>
Cc:
Ted Unangst <tedu@tedunangst.com>, Mark Kettenis <mark.kettenis@xs4all.nl>, tech@openbsd.org
Date:
Thu, 22 May 2025 11:37:23 +0200

Download raw body.

Thread
On Thu, May 22, 2025 at 11:30:10AM +0200, Alexandr Nedvedicky wrote:
> Hello,
> 
> On Thu, May 22, 2025 at 07:17:07AM +0200, Claudio Jeker wrote:
> > On Thu, May 22, 2025 at 05:09:17AM +0200, Alexandr Nedvedicky wrote:
> > > Hello,
> > > 
> > > On Wed, May 21, 2025 at 08:15:48PM -0400, Ted Unangst wrote:
> > > </snip>
> > > > 
> > > > Is it possible to use two loops here? One loop to get out of the kernel,
> > > > and then the second that does the copyin? I think there is not so much
> > > > code in common that duplicating is a problem, and it will be more clear
> > > > what we're trying to do in each loop.
> > > > 
> > > 
> > >     I like this idea. it makes code lot more clear.
> > >     updated diff follows.
> > 
> > I never looked into this proper but instead of walking the full kernel
> > stack, can't we start with the information in the trapframe?
> > Don't remember if there is a pointer to it somewhere in the pcb.
> >  
> 
>     I think I've managed to get working proof-of-concept (see diff below).
>     it works for amd64. However I'm not sure if it is direction we want
>     to go now. Perhaps later. I've cloned existing stactrace_save_utrace()
>     which is a current workhorse for all dtrace probes (syscall, profiling
>     static). I've made a cf_stacktrace_save_utrace() function which takes
>     a clockframe as additional argument. Using a clockframe (intrframe)
>     we can reach for userland stack immediately without iterating over
>     kernel frames like we do in stacktrace_save_utrace(). Similar mechanism
>     seen in cf_stacktrace_save_utrace() can be used for syscall dt probes
>     where we can use trapframe instead of clockframe/intrframe.
> 
>     However I think we still need to get fix for stacktrace_save_utrace() in 
>     because it will remain a workhorse for static probes (given I understand
>     things right here).
> 
> For now I would like to get this trap/intr frame shortcut aside and
> get fix for stacktrace_save_utrace(). I like tedu's idea it works
> fine for i386/amd64. Frame shortcut for stack tracing can
> be revisited later.

I agree with pushing your simple fix now. I was mostly bring up the
trapframe idea because some archs will need to use that since there is not
good way to distinguish between userland pointers and kernel ones.

-- 
:wq Claudio