lkml.org 
[lkml]   [2014]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] uprobes/x86: Rename arch_uprobe->def into ->dflt, minor comment updates
    (2014/06/02 4:25), Oleg Nesterov wrote:
    > Purely cosmetic, no changes in .o,
    >
    > 1. As Jim pointed out arch_uprobe->def looks ambiguous, rename it to
    > ->dflt.
    >
    > 2. Add the comment into default_post_xol_op() to explain "regs->sp +=".
    >
    > 3. Remove the stale part of the comment in arch_uprobe_analyze_insn().
    >
    > Suggested-by: Jim Keniston <jkenisto@us.ibm.com>
    > Signed-off-by: Oleg Nesterov <oleg@redhat.com>

    Looks OK for me :)

    Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

    > ---
    > arch/x86/include/asm/uprobes.h | 2 +-
    > arch/x86/kernel/uprobes.c | 37 ++++++++++++++++++-------------------
    > 2 files changed, 19 insertions(+), 20 deletions(-)
    >
    > diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h
    > index 7be3c07..b3d9442 100644
    > --- a/arch/x86/include/asm/uprobes.h
    > +++ b/arch/x86/include/asm/uprobes.h
    > @@ -52,7 +52,7 @@ struct arch_uprobe {
    > struct {
    > u8 fixups;
    > u8 ilen;
    > - } def;
    > + } dflt;
    > };
    > };
    >
    > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
    > index fcf6279..33e239f 100644
    > --- a/arch/x86/kernel/uprobes.c
    > +++ b/arch/x86/kernel/uprobes.c
    > @@ -254,7 +254,7 @@ static inline bool is_64bit_mm(struct mm_struct *mm)
    > * If arch_uprobe->insn doesn't use rip-relative addressing, return
    > * immediately. Otherwise, rewrite the instruction so that it accesses
    > * its memory operand indirectly through a scratch register. Set
    > - * def->fixups accordingly. (The contents of the scratch register
    > + * dflt->fixups accordingly. (The contents of the scratch register
    > * will be saved before we single-step the modified instruction,
    > * and restored afterward).
    > *
    > @@ -372,14 +372,14 @@ static void riprel_analyze(struct arch_uprobe *auprobe, struct insn *insn)
    > */
    > if (reg != 6 && reg2 != 6) {
    > reg2 = 6;
    > - auprobe->def.fixups |= UPROBE_FIX_RIP_SI;
    > + auprobe->dflt.fixups |= UPROBE_FIX_RIP_SI;
    > } else if (reg != 7 && reg2 != 7) {
    > reg2 = 7;
    > - auprobe->def.fixups |= UPROBE_FIX_RIP_DI;
    > + auprobe->dflt.fixups |= UPROBE_FIX_RIP_DI;
    > /* TODO (paranoia): force maskmovq to not use di */
    > } else {
    > reg2 = 3;
    > - auprobe->def.fixups |= UPROBE_FIX_RIP_BX;
    > + auprobe->dflt.fixups |= UPROBE_FIX_RIP_BX;
    > }
    > /*
    > * Point cursor at the modrm byte. The next 4 bytes are the
    > @@ -398,9 +398,9 @@ static void riprel_analyze(struct arch_uprobe *auprobe, struct insn *insn)
    > static inline unsigned long *
    > scratch_reg(struct arch_uprobe *auprobe, struct pt_regs *regs)
    > {
    > - if (auprobe->def.fixups & UPROBE_FIX_RIP_SI)
    > + if (auprobe->dflt.fixups & UPROBE_FIX_RIP_SI)
    > return &regs->si;
    > - if (auprobe->def.fixups & UPROBE_FIX_RIP_DI)
    > + if (auprobe->dflt.fixups & UPROBE_FIX_RIP_DI)
    > return &regs->di;
    > return &regs->bx;
    > }
    > @@ -411,18 +411,18 @@ scratch_reg(struct arch_uprobe *auprobe, struct pt_regs *regs)
    > */
    > static void riprel_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
    > {
    > - if (auprobe->def.fixups & UPROBE_FIX_RIP_MASK) {
    > + if (auprobe->dflt.fixups & UPROBE_FIX_RIP_MASK) {
    > struct uprobe_task *utask = current->utask;
    > unsigned long *sr = scratch_reg(auprobe, regs);
    >
    > utask->autask.saved_scratch_register = *sr;
    > - *sr = utask->vaddr + auprobe->def.ilen;
    > + *sr = utask->vaddr + auprobe->dflt.ilen;
    > }
    > }
    >
    > static void riprel_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
    > {
    > - if (auprobe->def.fixups & UPROBE_FIX_RIP_MASK) {
    > + if (auprobe->dflt.fixups & UPROBE_FIX_RIP_MASK) {
    > struct uprobe_task *utask = current->utask;
    > unsigned long *sr = scratch_reg(auprobe, regs);
    >
    > @@ -499,16 +499,16 @@ static int default_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs
    > struct uprobe_task *utask = current->utask;
    >
    > riprel_post_xol(auprobe, regs);
    > - if (auprobe->def.fixups & UPROBE_FIX_IP) {
    > + if (auprobe->dflt.fixups & UPROBE_FIX_IP) {
    > long correction = utask->vaddr - utask->xol_vaddr;
    > regs->ip += correction;
    > - } else if (auprobe->def.fixups & UPROBE_FIX_CALL) {
    > - regs->sp += sizeof_long();
    > - if (push_ret_address(regs, utask->vaddr + auprobe->def.ilen))
    > + } else if (auprobe->dflt.fixups & UPROBE_FIX_CALL) {
    > + regs->sp += sizeof_long(); /* Pop incorrect return address */
    > + if (push_ret_address(regs, utask->vaddr + auprobe->dflt.ilen))
    > return -ERESTART;
    > }
    > /* popf; tell the caller to not touch TF */
    > - if (auprobe->def.fixups & UPROBE_FIX_SETF)
    > + if (auprobe->dflt.fixups & UPROBE_FIX_SETF)
    > utask->autask.saved_tf = true;
    >
    > return 0;
    > @@ -711,12 +711,11 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
    >
    > /*
    > * Figure out which fixups default_post_xol_op() will need to perform,
    > - * and annotate def->fixups accordingly. To start with, ->fixups is
    > - * either zero or it reflects rip-related fixups.
    > + * and annotate dflt->fixups accordingly.
    > */
    > switch (OPCODE1(&insn)) {
    > case 0x9d: /* popf */
    > - auprobe->def.fixups |= UPROBE_FIX_SETF;
    > + auprobe->dflt.fixups |= UPROBE_FIX_SETF;
    > break;
    > case 0xc3: /* ret or lret -- ip is correct */
    > case 0xcb:
    > @@ -742,8 +741,8 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
    > riprel_analyze(auprobe, &insn);
    > }
    >
    > - auprobe->def.ilen = insn.length;
    > - auprobe->def.fixups |= fix_ip_or_call;
    > + auprobe->dflt.ilen = insn.length;
    > + auprobe->dflt.fixups |= fix_ip_or_call;
    >
    > auprobe->ops = &default_xol_ops;
    > return 0;
    >


    --
    Masami HIRAMATSU
    Software Platform Research Dept. Linux Technology Research Center
    Hitachi, Ltd., Yokohama Research Laboratory
    E-mail: masami.hiramatsu.pt@hitachi.com




    \
     
     \ /
      Last update: 2014-06-02 04:21    [W:2.562 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site