On Wed, 22 Oct 2025 22:41:20 +0200
Jiri Olsa <[email protected]> wrote:

> >   
> > >   ANNOTATE_NOENDBR
> > > + push $return_to_handler
> > > + UNWIND_HINT_FUNC  
> > 
> > OK, so what happened here is that you put in the return_to_handle into the
> > stack and told ORC that this is a normal function, and that when it
> > triggers to do a lookup from the handler itself.  
> 
> together with the "push $return_to_handler" it suppose to instruct 
> ftrace_graph_ret_addr
> to go get the 'real' return address from shadow stack
> 
> > 
> > I wonder if we could just add a new UNWIND_HINT that tells ORC to do that?  
> 
> if I remove the initial UNWIND_HINT_UNDEFINED I get objtool warning
> about unreachable instruction

Right. I was thinking we add UNWIND_HINT_RETHOOK and an
UNWIND_HINT_TYPE_RETHOOK that lets objtool know that this function is a
"return_to_hook" function and the unwinder can do something special with it.

> 
> >   
> > >  
> > >   /* Save ftrace_regs for function exit context  */
> > >   subq $(FRAME_SIZE), %rsp
> > > @@ -360,6 +362,9 @@ SYM_CODE_START(return_to_handler)
> > >   movq %rax, RAX(%rsp)
> > >   movq %rdx, RDX(%rsp)
> > >   movq %rbp, RBP(%rsp)
> > > + movq %rsp, RSP(%rsp)
> > > + movq $0, EFLAGS(%rsp)
> > > + movq $__KERNEL_CS, CS(%rsp)  
> > 
> > Is this simulating some kind of interrupt?  
> 
> there are several checks in pt_regs on these fields 
> 
> - in get_perf_callchain we check user_mode(regs) so CS has to be set
> - in perf_callchain_kernel we call perf_hw_regs(regs), so EFLAGS has to be set

So this is a different issue. I rather have this added in
kprobe_multi_link_prog_run as its the only user of it. Or have the
ftrace_regs conversion update it. This isn't something that should be done
at every call and slow everyone else down.

> 
> >   
> > >   movq %rsp, %rdi
> > >  
> > >   call ftrace_return_to_handler  
> > 
> > Now it gets tricky in the ftrace_return_to_handler as the first thing it
> > does is to pop the shadow stack, which makes the return_to_handler lookup
> > different, as its no longer on the stack that the unwinder will use.  
> 
> hum strange.. the resulting stack trace seems ok, I'll make it a
> selftest I send it
> 
> ftrace_graph_ret_addr that checks on the 'real return address seems
> to have 2 ways of getting to it:
> 
>         i = *idx ? : task->curr_ret_stack;
> 
> I dont know how that previous pop affects this, but I'm sure it's
> more complicated than this ;-)

Oh wait, it may be OK. I forgot I had to change the pop function to give
the data back, but it doesn't modify the task->curr_ret_stack until after
it calls all the callbacks. That's because the shadow stack still has the
data that is being passed from the entry callback. So it can't be updated
yet otherwise that data on the shadow stack will get corrupted.

Yeah, the return_to_handler should work up until the end of
ftrace_return_to_handler().

-- Steve


Reply via email to