lkml.org 
[lkml]   [2019]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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)
>

\
 
 \ /
  Last update: 2019-02-14 15:07    [W:0.080 / U:5.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site