On Fri, 2014-04-04 at 21:32 +0200, Oleg Nesterov wrote:
> On 04/04, Oleg Nesterov wrote:
> >
> > Now let me send the draft RFC patch which fixes the "call" handling...
> 
> Damn. apparently I can't understand lib/insn.c...
> 
> Please see the draft below. Lets ignore 32bit tasks, lets ignore jmp's,
> please ignore how the (pseudo) code written, I'll change it anyway.
> 
> Questions:

So far, I have answers for just #1 and #2.

> 
>       1. Why insn_get_displacement() doesn't work? See "HELP!!!"
>          below.

insn->moffset1.value seems to be what you want.

> 
>       2. Do I use lib/insn.c correctly (ignoring displacement) ?

insn_get_length() has the side-effect of processing the entire
instruction, so just calling that should be sufficient.  Looks OK
otherwise -- but I checked very quickly.

More in a day or two.

Jim

> 
>            In particular, is 'turn this insn into "1: call 1b;"'
>            below correct?
> 
>       3. Jim, do you still think it would be better to rewrite the
>          call insns using a scratch register ?
> 
>       4. Is there other call insns with OPCODE1() != 0xe8 which
>          should be fixed too?
> 
> Thanks,
> 
> Oleg.
> 
> 
> diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h
> index 9f8210b..cca62c5 100644
> --- a/arch/x86/include/asm/uprobes.h
> +++ b/arch/x86/include/asm/uprobes.h
> @@ -44,9 +44,15 @@ struct arch_uprobe {
>       u16                             fixups;
>       const struct uprobe_xol_ops     *ops;
> 
> +     union {
>  #ifdef CONFIG_X86_64
> -     unsigned long                   rip_rela_target_address;
> +             unsigned long                   rip_rela_target_address;
>  #endif
> +             struct {
> +                     s32     disp;
> +                     u8      ilen;
> +             }                               ttt;
> +     };
>  };
> 
>  struct arch_uprobe_task {
> diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> index b820668..423ae86 100644
> --- a/arch/x86/kernel/uprobes.c
> +++ b/arch/x86/kernel/uprobes.c
> @@ -461,6 +461,52 @@ static struct uprobe_xol_ops default_xol_ops = {
>       .post_xol = default_post_xol_op,
>  };
> 
> +static bool ttt_emulate_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
> +{
> +
> +     if (put_user(regs->ip + auprobe->ttt.ilen, (long __user *)(regs->sp - 
> 8)))
> +             return false;
> +
> +     regs->sp -= 8;
> +     regs->ip += auprobe->ttt.ilen + auprobe->ttt.disp;
> +
> +     return true;
> +}
> +
> +static int ttt_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
> +{
> +     regs->sp += 8;
> +     if (ttt_emulate_op(auprobe, regs))
> +             return 0;
> +     return -ERESTART;
> +}
> +
> +static struct uprobe_xol_ops ttt_xol_ops = {
> +     .emulate  = ttt_emulate_op,
> +     .post_xol = ttt_post_xol_op,
> +};
> +
> +static int ttt_setup_xol_ops(struct arch_uprobe *auprobe, struct insn *insn)
> +{
> +     s32 *disp;
> +
> +     insn_get_length(insn);
> +     auprobe->ttt.ilen = insn->length;
> +
> +     insn_get_displacement(insn);
> +     auprobe->ttt.disp = insn->displacement.value;
> +     // HELP!!! the above doesn't work, ->displacement.value == 0
> +     auprobe->ttt.disp = *(s32 *)(auprobe->insn + 1);
> +
> +     // turn this insn into "1: call 1b;", we only need to xol it
> +     // to expand the stack if ->emulate() fails.
> +     disp = (void *)auprobe->insn + insn_offset_displacement(insn);
> +     *disp = -(s32)auprobe->ttt.ilen;
> +
> +     auprobe->ops = &ttt_xol_ops;
> +     return 0;
> +}
> +
>  /**
>   * arch_uprobe_analyze_insn - instruction analysis including validity and 
> fixups.
>   * @mm: the probed address space.
> @@ -484,6 +530,9 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, 
> struct mm_struct *mm,
>        * is either zero or it reflects rip-related fixups.
>        */
>       switch (OPCODE1(&insn)) {
> +     case 0xe8:              /* call relative - has its own xol_ops */
> +             return ttt_setup_xol_ops(auprobe, &insn);
> +
>       case 0x9d:              /* popf */
>               auprobe->fixups |= UPROBE_FIX_SETF;
>               break;
> @@ -493,9 +542,6 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, 
> struct mm_struct *mm,
>       case 0xca:
>               fix_ip = false;
>               break;
> -     case 0xe8:              /* call relative - Fix return addr */
> -             fix_call = true;
> -             break;
>       case 0x9a:              /* call absolute - Fix return addr, not ip */
>               fix_call = true;
>               fix_ip = false;
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to