linux-kernel - Re: [PATCH v4 03/16] x86/alternatives,jump_label: Provide better text

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]

Date:   Mon, 21 Oct 2019 11:21:58 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org, rostedt@...dmis.org,
        mhiramat@...nel.org, bristot@...hat.com, jbaron@...mai.com,
        torvalds@...ux-foundation.org, tglx@...utronix.de,
        namit@...are.com, hpa@...or.com, luto@...nel.org,
        ard.biesheuvel@...aro.org, jpoimboe@...hat.com, jeyu@...nel.org
Subject: Re: [PATCH v4 03/16] x86/alternatives,jump_label: Provide better
 text_poke() batching interface

On Mon, Oct 21, 2019 at 10:48:02AM +0200, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@...radead.org> wrote:
> 
> > --- a/arch/x86/kernel/jump_label.c
> > +++ b/arch/x86/kernel/jump_label.c
> > @@ -35,18 +35,19 @@ static void bug_at(unsigned char *ip, in
> >  	BUG();
> >  }
> >  
> > -static void __jump_label_set_jump_code(struct jump_entry *entry,
> > -				       enum jump_label_type type,
> > -				       union jump_code_union *code,
> > -				       int init)
> > +static const void *
> > +__jump_label_set_jump_code(struct jump_entry *entry, enum jump_label_type type, int init)
> >  {
> > +	static union jump_code_union code; /* relies on text_mutex */
> >  	const unsigned char default_nop[] = { STATIC_KEY_INIT_NOP };
> >  	const unsigned char *ideal_nop = ideal_nops[NOP_ATOMIC5];
> >  	const void *expect;
> >  	int line;
> >  
> > -	code->jump = 0xe9;
> > -	code->offset = jump_entry_target(entry) -
> > +	lockdep_assert_held(&text_mutex);
> > +
> > +	code.jump = JMP32_INSN_OPCODE;
> > +	code.offset = jump_entry_target(entry) -
> >  		       (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
> >  
> >  	if (init) {
> > @@ -54,23 +55,23 @@ static void __jump_label_set_jump_code(s
> >  	} else if (type == JUMP_LABEL_JMP) {
> >  		expect = ideal_nop; line = __LINE__;
> >  	} else {
> > -		expect = code->code; line = __LINE__;
> > +		expect = code.code; line = __LINE__;
> 
> Side note: the whole 'line' logic looked weird to me and it obsfuscates 
> the logic a bit, and I had to look it up to see what it's about: 
> improving the debug output of text-patching crashes.
> 
> How about something like the below on top of your queue? We have %phD 
> that can nicely print instructions in hex.

I have a patch like that somewhere; see here:

  https://lkml.kernel.org/r/20191007090012.00469193.6@infradead.org

But yes, the __LINE__ thing is mostly about identifying which case it is
and I suppose we can infer that when we have the expected text printed
too.