lkml.org 
[lkml]   [2015]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 07/10] uprobes/x86: Introduce arch_uretprobe_is_alive()
On 05/07, Srikar Dronamraju wrote:
>
> >
> > diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h
> > index f011fd0..60777f3 100644
> > --- a/arch/x86/include/asm/uprobes.h
> > +++ b/arch/x86/include/asm/uprobes.h
> > @@ -57,6 +57,7 @@ struct arch_uprobe {
> > };
> >
> > struct arch_uretprobe {
> > + unsigned long sp;
>
> While this looks good, I was wondering if you did think of having the sp
> in the return_instance structure itself. I mean can we use
> user_stack_pointer() to populate the ri->sp?

Yes, yes, I considered this option. And note that we can cleanup the
(a bit ugly) arch_uretprobe_hijack_return_addr() if we export
"struct return_instance" and pass it to arch_ helper.

> In which case the weak function itself should suffice for most archs.
>
> Something like this.
> prepare_uretprobe() we can have
> ri->sp = user_stack_pointer(regs)

Yes, and we can do this without changing arch_uretprobe_hijack_return_addr()
interface (which imo should be changed anyway, but this is off-topic).

> and handle_trampoline() would call something like
>
> arch_uretprobe_is_alive(next->sp, regs);
>
> bool __weak arch_uretprobe_is_alive(unsigned long sp, struct pt_regs *regs)
> {
> return user_stack_pointer(regs) <= sp;
> }

The problem is, I simply do not know if this is right on !x86.

And. I wanted to ensure that if (say) arch/ppc needs something else to
save/check in hijack/alive, then this architecture can just add the new
members in arch_uretprobe and change the arch_ helpers.

> Am I missing something?

I do not know. Lets wait for the comments from arch/ maintainers?

Oleg.



\
 
 \ /
  Last update: 2015-05-07 19:21    [W:0.376 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site