linux-kernel - Re: Kernel broken on processors without performance counters

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] [day] [month] [year] [list]

Message-ID: <55B2486D.1040505@gmail.com>
Date:	Fri, 24 Jul 2015 10:15:09 -0400
From:	Jason Baron <jasonbaron0@...il.com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	Borislav Petkov <bp@...en8.de>,
	Andy Lutomirski <luto@...capital.net>,
	Thomas Gleixner <tglx@...utronix.de>,
	Mikulas Patocka <mpatocka@...hat.com>,
	Paul Mackerras <paulus@...ba.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Kees Cook <keescook@...omium.org>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Vince Weaver <vince@...ter.net>,
	"hillf.zj" <hillf.zj@...baba-inc.com>,
	Valdis Kletnieks <Valdis.Kletnieks@...edu>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Steven Rostedt <rostedt@...dmis.org>
Subject: Re: Kernel broken on processors without performance counters



On 07/24/2015 08:36 AM, Peter Zijlstra wrote:
> On Fri, Jul 24, 2015 at 12:56:02PM +0200, Peter Zijlstra wrote:
>> On Thu, Jul 23, 2015 at 03:14:13PM -0400, Jason Baron wrote:
>>>> +static enum jump_label_type jump_label_type(struct jump_entry *entry)
>>>> +{
>>>> +	struct static_key *key = static_key_cast(iter->key);
>>>> +	bool true_branch = jump_label_get_branch_default(key);
>>>> +	bool state = static_key_enabled(key);
>>>> +	bool inv = static_key_inv(iter->key);
>>>> +
>>>> +	return (true_branch ^ state) ^ inv;
>>> iiuc...this allows both the old style keys to co-exist with the
>>> new ones. IE state wouldn't be set for the new ones. And inv
>>> shouldn't be set for the old ones. Is that correct?
>> @state is the dynamic variable here, the other two are compile time.
>> @true_branch denotes the default (compile time) value, and @inv denotes
>> the (compile time) branch preference.
> Ha!, so that wasn't entirely correct, it turned out @inv means
> arch_static_branch_jump().
>
> That would let us remove the whole argument to the arch functions.
>
> That said, I generated the logic table for @inv meaning the branch type
> and then found a logic similar to what you outlined:
>
>
> /*
>  * Combine the right initial value (type) with the right branch order
>  * to generate the desired result.
>  *
>  *
>  *  type	likely (1)		unlikely (0)
>  * -----------+-----------------------+------------------
>  *            |                       |
>  *  true (1)  |	   ...		      |	   ...
>  *            |    NOP		      |	   JMP L
>  *            |    <br-stmts>	      |	1: ...
>  *            |	L: ...		      |
>  *            |			      |
>  *            |			      |	L: <br-stmts>
>  *            |			      |	   jmp 1b
>  *            |                       |
>  * -----------+-----------------------+------------------
>  *            |                       |
>  *  false (0) |	   ...		      |	   ...
>  *            |    JMP L	      |	   NOP
>  *            |    <br-stmts>	      |	1: ...
>  *            |	L: ...		      |
>  *            |			      |
>  *            |			      |	L: <br-stmts>
>  *            |			      |	   jmp 1b
>  *            |                       |
>  * -----------+-----------------------+------------------
>  *
>  * The initial value is encoded in the LSB of static_key::entries,
>  * type: 0 = false, 1 = true.
>  *
>  * The branch type is encoded in the LSB of jump_entry::key,
>  * branch: 0 = unlikely, 1 = likely.
>  *
>  * This gives the following logic table:
>  *
>  *	enabled	type	branch	  instuction
>  * -----------------------------+-----------
>  *	0	0	0	| NOP
>  *	0	0	1	| JMP
>  *	0	1	0	| NOP
>  *	0	1	1	| JMP
>  *
>  *	1	0	0	| JMP
>  *	1	0	1	| NOP
>  *	1	1	0	| JMP
>  *	1	1	1	| NOP
>  *
>  */
>
> This gives a map: ins = enabled ^ branch, which shows @type to be
> redundant.
>
> And we can trivially switch over the old static_key_{true,false}() to
> emit the right branch type.
>
> Whcih would mean we could remove the type encoding entirely, but I'll
> leave that be for now, maybe it'll come in handy later or whatnot.
>

I would just remove 'type'. Essentially, before we were storing
the 'branch' with the key. However, in this new model the
'branch' is really associated with the branch location, since the
same key can be used now with different branches.

Thanks,

-Jason

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/