Messages in this thread | | | Date | Tue, 01 Apr 2014 12:17:55 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH 3/7] uprobes/x86: Conditionalize the usage of handle_riprel_insn() |
| |
(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@redhat.com>
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?
Thank you,
> --- > arch/x86/kernel/uprobes.c | 6 ++---- > 1 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c > index 098e56e..d72dfbf 100644 > --- a/arch/x86/kernel/uprobes.c > +++ b/arch/x86/kernel/uprobes.c > @@ -376,8 +376,6 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, > * and annotate arch_uprobe->fixups accordingly. To start with, ->fixups > * is either zero or it reflects rip-related fixups. > */ > - handle_riprel_insn(auprobe, mm, &insn); > - > switch (OPCODE1(&insn)) { > case 0x9d: /* popf */ > auprobe->fixups |= UPROBE_FIX_SETF; > @@ -406,9 +404,9 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, > case 4: case 5: /* jmp or ljmp, indirect */ > fix_ip = false; > } > - break; > + /* fall through */ > default: > - break; > + handle_riprel_insn(auprobe, mm, &insn); > } > > if (fix_ip) >
-- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com
| |