Skip to content

Commit 75f9ef0

Browse files
committed
uprobes/x86: Teach arch_uprobe_post_xol() to restart if possible
SIGILL after the failed arch_uprobe_post_xol() should only be used as a last resort, we should try to restart the probed insn if possible. Currently only adjust_ret_addr() can fail, and this can only happen if another thread unmapped our stack after we executed "call" out-of-line. Most probably the application if buggy, but even in this case it can have a handler for SIGSEGV/etc. And in theory it can be even correct and do something non-trivial with its memory. Of course we can't restart unconditionally, so arch_uprobe_post_xol() does this only if ->post_xol() returns -ERESTART even if currently this is the only possible error. default_post_xol_op(UPROBE_FIX_CALL) can always restart, but as Jim pointed out it should not forget to pop off the return address pushed by this insn executed out-of-line. Note: this is not "perfect", we do not want the extra handler_chain() after restart, but I think this is the best solution we can realistically do without too much uglifications. Signed-off-by: Oleg Nesterov <[email protected]> Reviewed-by: Masami Hiramatsu <[email protected]> Reviewed-by: Jim Keniston <[email protected]>
1 parent 014940b commit 75f9ef0

File tree

1 file changed

+16
-4
lines changed

1 file changed

+16
-4
lines changed

arch/x86/kernel/uprobes.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,16 +443,22 @@ static int default_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs
443443
{
444444
struct uprobe_task *utask = current->utask;
445445
long correction = (long)(utask->vaddr - utask->xol_vaddr);
446-
int ret = 0;
447446

448447
handle_riprel_post_xol(auprobe, regs, &correction);
449448
if (auprobe->fixups & UPROBE_FIX_IP)
450449
regs->ip += correction;
451450

452-
if (auprobe->fixups & UPROBE_FIX_CALL)
453-
ret = adjust_ret_addr(regs->sp, correction);
451+
if (auprobe->fixups & UPROBE_FIX_CALL) {
452+
if (adjust_ret_addr(regs->sp, correction)) {
453+
if (is_ia32_task())
454+
regs->sp += 4;
455+
else
456+
regs->sp += 8;
457+
return -ERESTART;
458+
}
459+
}
454460

455-
return ret;
461+
return 0;
456462
}
457463

458464
static struct uprobe_xol_ops default_xol_ops = {
@@ -599,6 +605,12 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
599605
int err = auprobe->ops->post_xol(auprobe, regs);
600606
if (err) {
601607
arch_uprobe_abort_xol(auprobe, regs);
608+
/*
609+
* Restart the probed insn. ->post_xol() must ensure
610+
* this is really possible if it returns -ERESTART.
611+
*/
612+
if (err == -ERESTART)
613+
return 0;
602614
return err;
603615
}
604616
}

0 commit comments

Comments
 (0)