Messages in this thread | | | Date | Thu, 14 Jan 2010 10:39:09 -0500 | From | Mathieu Desnoyers <> | Subject | Re: [RFC PATCH 2/8] jump label v4 - x86: Introduce generic jump patching without stop_machine |
| |
* Steven Rostedt (rostedt@goodmis.org) wrote: > On Tue, 2010-01-12 at 11:26 -0500, Jason Baron wrote: > > > +/** > > + * text_poke_fixup() -- cross-modifying kernel text with fixup address. > > + * @addr: Modifying address. > > + * @opcode: New instruction. > > + * @len: length of modifying bytes. > > + * @fixup: Fixup address. > > + * > > + * Note: You must backup replaced instructions before calling this, > > + * if you need to recover it. > > + * Note: Must be called under text_mutex. > > + */ > > +void *__kprobes text_poke_fixup(void *addr, const void *opcode, size_t len, > > + void *fixup) > > +{ > > + static const unsigned char int3_insn = BREAKPOINT_INSTRUCTION; > > + static const int int3_size = sizeof(int3_insn); > > + > > + /* Replacing 1 byte can be done atomically. */ > > + if (unlikely(len <= 1)) > > + return text_poke(addr, opcode, len); > > This part bothers me. The text_poke just writes over the text directly > (using a separate mapping). But if that memory is in the pipeline of > another CPU, I think this could cause a GPF.
It looks like we are thinking along the same lines.
I'm under the impression that I pointed out this exact same issue in the previous round of review a few weeks ago. Does this submission reflect the up-to-date state of this patch ?
Thanks,
Mathieu
> > -- Steve > > > + > > + /* Preparing */ > > + patch_fixup_addr = fixup; > > + wmb(); > > + patch_fixup_from = (u8 *)addr + int3_size; /* IP address after int3 */ > > + > > + /* Cap by an int3 */ > > + text_poke(addr, &int3_insn, int3_size); > > + sync_core_all(); > > + > > + /* Replace tail bytes */ > > + text_poke((char *)addr + int3_size, (const char *)opcode + int3_size, > > + len - int3_size); > > + sync_core_all(); > > + > > + /* Replace int3 with head byte */ > > + text_poke(addr, opcode, int3_size); > > + sync_core_all(); > > + > > + /* Cleanup */ > > + patch_fixup_from = NULL; > > + wmb(); > > + return addr; > > +} > > + >
-- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |