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

Message-ID: <20150723105309.GE18642@nazgul.tnic>
Date:	Thu, 23 Jul 2015 12:53:09 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Jason Baron <jasonbaron0@...il.com>,
	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 Thu, Jul 23, 2015 at 12:42:15PM +0200, Peter Zijlstra wrote:
> > static_likely_init_true_branch(struct static_likely_init_true_key *key)
> > static_likely_init_false_branch(struct static_likely_init_false_key *key)
> > static_unlikely_init_false_branch(struct static_unlikely_init_false_key *key)
> > static_unlikely_init_true_branch(struct static_unlikely_init_true_key *key)
> 
> I'm sorely tempted to go quote cypress hill here...

Yah, those are at least too long and nuts.

> And I realize part of the problem is that we're wanting to use jump
> labels before we can patch them. But surely we can do better.
> 
> extern bool ____wrong_branch_error(void);
> 
> struct static_key_true;
> struct static_key_false;
> 
> #define static_branch_likely(x)							\
> ({										\
> 	bool branch;								\
> 	if (__builtin_types_compatible_p(typeof(x), struct static_key_true))	\
> 		branch = !arch_static_branch(&(x)->key);			\
> 	else if (__builtin_types_compatible_p(typeof(x), struct static_key_false)) \
> 		branch = !arch_static_branch_jump(&(x)->key);			\
> 	else									\
> 		branch = ____wrong_branch_error();				\
> 	branch;									\
> })
> 
> #define static_branch_unlikely(x)						\
> ({										\
> 	bool branch;								\
> 	if (__builtin_types_compatible_p(typeof(x), struct static_key_true))	\
> 		branch = arch_static_branch(&(x)->key);				\
> 	else if (__builtin_types_compatible_p(typeof(x), struct static_key_false)) \
> 		branch = arch_static_branch_jump(&(x)->key);			\
> 	else									\
> 		branch = ____wrong_branch_error();				\
> 	branch;									\
> })
> 
> Can't we make something like that work?
> 
> So the immediate problem appears to be the 4 different key inits, which don't
> seem very supportive of this separation:
> 
> +#define STATIC_KEY_LIKEY_INIT_TRUE ((struct static_unlikely_init_true_key)        \

LIKELY

> +    { .key.enabled = ATOMIC_INIT(1),                \
> +      .key.entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH })
> 
> +#define STATIC_KEY_LIKEY_INIT_FALSE ((struct static_unlikely_init_false_key)    \

Yuck, those struct names are still too long IMO.

> +    { .key.enabled = ATOMIC_INIT(0),                \
> +      .key.entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH })
> 
> +#define STATIC_KEY_UNLIKELY_INIT_TRUE ((struct static_unlikely_init_true_key)    \
> +    { .key.enabled = ATOMIC_INIT(1),                \
> +      .key.entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH })
> 
> +#define STATIC_KEY_UNLIKELY_INIT_FALSE ((struct static_unlikely_init_false_key)    \
> +    { .key.enabled = ATOMIC_INIT(0),                \
> +      .key.entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH })
> 
> 
> But I think we can fix that by using a second __jump_table section, then
> we can augment the LABEL_TYPE_{TRUE,FALSE} thing with the section we
> find the jump_entry in.
> 
> Then we can do:
> 
> #define STATIC_KEY_TRUE_INIT  (struct static_key_true) { .key = STATIC_KEY_INIT_TRUE,  }
> #define STATIC_KEY_FALSE_INIT (struct static_key_false){ .key = STATIC_KEY_INIT_FALSE, }

Let's abbreviate that "STATIC_KEY" thing too:

SK_TRUE_INIT
SK_FALSE_INIT
...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
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/