Messages in this thread | | | Date | Thu, 14 Feb 2019 09:06:20 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH V4 7/9] x86/alternative: Batch of patch operations |
| |
On Thu, 14 Feb 2019 13:53:30 +0100 Borislav Petkov <bp@alien8.de> wrote:
> > arch/x86/include/asm/text-patching.h | 15 ++++ > > arch/x86/kernel/alternative.c | 124 ++++++++++++++++++++++----- > > 2 files changed, 118 insertions(+), 21 deletions(-) > > > > diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h > > index e85ff65c43c3..42ea7846df33 100644 > > --- a/arch/x86/include/asm/text-patching.h > > +++ b/arch/x86/include/asm/text-patching.h > > @@ -18,6 +18,20 @@ static inline void apply_paravirt(struct paravirt_patch_site *start, > > #define __parainstructions_end NULL > > #endif > > > > +/* > > + * Currently, the max observed size in the kernel code is > > + * JUMP_LABEL_NOP_SIZE/RELATIVEJUMP_SIZE, which are 5. > > + * Raise it if needed. > > + */ > > +#define POKE_MAX_OPCODE_SIZE 5 > > + > > +struct text_to_poke { > > That name needs bikeshedding. Maybe struct patch_loc is more fitting as > it is a patch location descriptor, AFAICT.
Although you did say above that "patch" is an overloaded term. Also, I do find the current name rather descriptive.
> > > + void *handler; > > Oh my, took me a while to realize that this is not really a function > which is supposed to handle something but the place we go to when the > temporary breakpoint hits. Please change that name and all the code > which calls it handler. It is very misleading.
void *trampoline;
?
> > > + void *addr; > > + size_t len; > > + const char opcode[POKE_MAX_OPCODE_SIZE]; > > +}; > > + > > extern void *text_poke_early(void *addr, const void *opcode, size_t len); > > > > /* > > @@ -37,6 +51,7 @@ extern void *text_poke_early(void *addr, const void *opcode, size_t len); > > extern void *text_poke(void *addr, const void *opcode, size_t len); > > extern int poke_int3_handler(struct pt_regs *regs); > > extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler); > > +extern void text_poke_bp_batch(struct text_to_poke *tp, unsigned int nr_entries); > > extern int after_bootmem; > > > > #endif /* _ASM_X86_TEXT_PATCHING_H */ > > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c > > index 202af29c43c0..318b6867dc5a 100644 > > --- a/arch/x86/kernel/alternative.c > > +++ b/arch/x86/kernel/alternative.c > > @@ -11,6 +11,8 @@ > > #include <linux/stop_machine.h> > > #include <linux/slab.h> > > #include <linux/kdebug.h> > > +#include <linux/kprobes.h> > > +#include <linux/bsearch.h> > > #include <asm/text-patching.h> > > #include <asm/alternative.h> > > #include <asm/sections.h> > > @@ -738,10 +740,26 @@ static void do_sync_core(void *info) > > } > > > > static bool bp_patching_in_progress; > > -static void *bp_int3_handler, *bp_int3_addr; > > +static struct text_to_poke *bp_int3_tpv; > > +static unsigned int bp_int3_tpv_nr; > > Those names need bikeshedding. See below.
But they are named after you! "bp_" :-)
-- Steve
> > > + > > +static int text_bp_batch_bsearch(const void *key, const void *elt) >
| |