linux-kernel - Re: [PATCH V4 7/9] x86/alternative: Batch of patch operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190214090620.1ee73a36@gandalf.local.home>
Date:   Thu, 14 Feb 2019 09:06:20 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Daniel Bristot de Oliveira <bristot@...hat.com>,
        linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jiri Kosina <jkosina@...e.cz>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Chris von Recklinghausen <crecklin@...hat.com>,
        Jason Baron <jbaron@...mai.com>, Scott Wood <swood@...hat.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Clark Williams <williams@...hat.com>, x86@...nel.org
Subject: Re: [PATCH V4 7/9] x86/alternative: Batch of patch operations

On Thu, 14 Feb 2019 13:53:30 +0100
Borislav Petkov <bp@...en8.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)  
>