Re: [PATCH v2 0/9] uprobes/x86: preparations to fix the reprel jmp/call handling.

From: Oleg Nesterov
Date: Fri Apr 04 2014 - 15:32:44 EST


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:

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

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

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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/