Messages in this thread | | | Date | Tue, 28 Feb 2012 09:48:28 +0100 | From | Ingo Molnar <> | Subject | Re: Re: [PATCH v3 -tip] [BUGFIX] x86/kprobes: Fix to recover instructions on optimized path |
| |
* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> (2012/02/27 18:34), Ingo Molnar wrote: > > > > * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote: > > > >> + > >> +#ifdef CONFIG_OPTPROBES > >> +static unsigned long __recover_optprobed_insn(struct kprobe *kp, > >> + kprobe_opcode_t *buf, > >> + unsigned long addr) > >> +{ > >> + long offs = addr - (unsigned long)kp->addr - 1; > >> + struct optimized_kprobe *op = container_of(kp, struct optimized_kprobe, kp); > >> + > >> + /* > >> + * If the kprobe can be optimized, original bytes which can be > >> + * overwritten by jump destination address. In this case, original > >> + * bytes must be recovered from op->optinsn.copied_insn buffer. > >> + */ > >> + memcpy(buf, (void *)addr, MAX_INSN_SIZE * sizeof(kprobe_opcode_t)); > >> + if (addr == (unsigned long)kp->addr) { > >> + buf[0] = kp->opcode; > >> + memcpy(buf + 1, op->optinsn.copied_insn, RELATIVE_ADDR_SIZE); > >> + } else > >> + memcpy(buf, op->optinsn.copied_insn + offs, RELATIVE_ADDR_SIZE - offs); > >> + > >> + return (unsigned long)buf; > >> +} > >> +#endif > > > > Why not stick this into a new kprobes-opt.c file? > > Would you mean that I should split all optprobe stuffs into > new file?
Yeah, that would be sensible I think - and it might help avoid similar complications in the future.
Could (and probably should) be done in a separate patch - to keep the bits that you already fixed and tested intact.
> > This should be a separate, kprobes_recover_opt() method and > > be inside kprobes-opt.c as well. > > OK, I'll do that. But I think it should be separated work. > Just for the bugfix, I think this should go into this style, > because this should be pushed into stable tree too.
I don't think we can push such a large and complex looking patch into v3.3 (let alone into -stable) - it's v3.4 material, and that's why I asked for the cleaner split-out as well. This optprobes code is really non-obvious at the moment and a split-out might improve that and might make future fixes easier to merge.
Thanks,
Ingo
| |