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:	Thu, 01 Dec 2011 23:08:19 +0100
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Jason Baron <jbaron@...hat.com>
Cc:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>,
	rostedt@...dmis.org
Subject: Re: [PATCH] jump_label: jump_label for boot options.

On Thu, 2011-12-01 at 16:13 -0500, Jason Baron wrote:

> I think what you have below should work modulo the no out-of-line
> branches and the following change:
> 
> > -			if (neg)
> > +			if (neg) {
> >  				sysctl_sched_features &= ~(1UL << i);
> > -			else
> > +#ifdef HAVE_JUMP_LABEL
> > +				if (!jump_label_enabled(&sched_feat_keys[i]))
> > +					jump_label_inc(&sched_feat_keys[i]);
> > +#endif
> 
> I think here its:
> 			if (jump_label_enabled())
> 				jump_label_dec();
> 
> 
> > +			} else {
> >  				sysctl_sched_features |= (1UL << i);
> > +#ifdef HAVE_JUMP_LABEL
> > +				if (jump_label_enabled(&sched_feat_keys[i]))
> > +					jump_label_dec(&sched_feat_keys[i]);
> > +#endif
> > +			}
> 
> Same here:
> 			if (!jump_label_enabled())
> 				jump_label_inc()

Gah, right. An earlier version used !static_branch().

> 
> The inc/dec behavior we have now, in fact will only mess up in the case
> where we define 'static_branch_true()'. Because then, in that case the
> jump_label_inc() will cause a jump to the false branch. So as long as we
> don't introduce 'static_branch_true()' and do an early setting of those
> branches which are true via __init code as you have here, I think things are
> correct.

That's with your definition of static_branch_true(). Not mine :-)

Mine simply doesn't out-of-line the branch, and ideally has a
jump_label_key initializer that initializes to true (1) and has gcc
generate right code.

I can do the initializer, something like the below. What I cannot do is
make gcc generate the enabled case so we don't have the small nop window
(although with the below I'm not sure it really matters).

But the more important point is not getting the branch out-of-line.

---
Index: linux-2.6/include/linux/jump_label.h
===================================================================
--- linux-2.6.orig/include/linux/jump_label.h
+++ linux-2.6/include/linux/jump_label.h
@@ -128,4 +128,6 @@ static inline void jump_label_rate_limit
 }
 #endif	/* HAVE_JUMP_LABEL */
 
+#define jump_label_key_true	((struct jump_label_key){ .enabled = ATOMIC_INIT(1), })
+
 #endif	/* _LINUX_JUMP_LABEL_H */
Index: linux-2.6/kernel/jump_label.c
===================================================================
--- linux-2.6.orig/kernel/jump_label.c
+++ linux-2.6/kernel/jump_label.c
@@ -248,8 +248,13 @@ void jump_label_apply_nops(struct module
 	if (iter_start == iter_stop)
 		return;
 
-	for (iter = iter_start; iter < iter_stop; iter++)
-		arch_jump_label_transform_static(iter, JUMP_LABEL_DISABLE);
+	for (iter = iter_start; iter < iter_stop; iter++) {
+		struct jump_label_key *iterk;
+
+		iterk = (struct jump_label_key *)(unsigned long)iter->key;
+		arch_jump_label_transform_static(iter, jump_label_enabled(iterk) ?
+				JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
+	}
 }
 
 static int jump_label_add_module(struct module *mod)
@@ -289,8 +294,7 @@ static int jump_label_add_module(struct
 		key->next = jlm;
 
 		if (jump_label_enabled(key))
-			__jump_label_update(key, iter, iter_stop,
-					    JUMP_LABEL_ENABLE);
+			__jump_label_update(key, iter, iter_stop, JUMP_LABEL_ENABLE);
 	}
 
 	return 0;

--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ