Re: [PATCH 3/7] uprobes/x86: Conditionalize the usage of handle_riprel_insn()

[Oleg Nesterov]

On 04/01, Masami Hiramatsu wrote:
>
> (2014/04/01 4:44), Oleg Nesterov wrote:
> > arch_uprobe_analyze_insn() calls handle_riprel_insn() at the start,
> > but only "0xff" and "default" cases need the UPROBE_FIX_RIP_ logic.
> > Move the callsite into "default" case and change the "0xff" case to
> > fall-through.
> >
> > We are going to add the various hooks to handle the rip-relative
> > jmp/call instructions (and more), we need this change to enforce the
> > fact that the new code can't conflict with is_riprel_insn() code.
> >
> > Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
>
> Hmm, this seems not obviously reasonable at this point.
> However, the code itself is not wrong. Could you merge
> this change to that new hooks?

Good point, I'll send v2.

I'd still prefer to do this in a separate patch if you do not object,
and this patch should come after "add uprobe_xol_ops". In this case
it should be more clear why do we need this change, and the changelog
can tell more. Say, it can mention that otherwise uprobe_abort_xol()
can be confused until at least we add uprobe_xol_ops->abort().

And probably I'll add another patch into this series, "restart if
->post_xol() fails" (see "TODO:" in 7/7). We need this change anyway
to emulate the "call" insn. We could fix this before we add the hooks,
but after 7/7 the change will be more simple/clear.

Thanks!

Oleg.

--
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/