linux-kernel - Re: [PATCH] sched, deadline: Properly initialize def_dl

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:	Wed, 18 Dec 2013 11:01:34 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	tglx@...utronix.de, mingo@...hat.com, oleg@...hat.com,
	fweisbec@...il.com, darren@...art.com, johan.eker@...csson.com,
	p.faure@...tech.ch, linux-kernel@...r.kernel.org,
	claudio@...dence.eu.com, michael@...rulasolutions.com,
	fchecconi@...il.com, tommaso.cucinotta@...up.it,
	juri.lelli@...il.com, nicola.manica@...i.unitn.it,
	luca.abeni@...tn.it, dhaval.giani@...il.com, hgu1972@...il.com,
	paulmck@...ux.vnet.ibm.com, raistlin@...ux.it,
	insop.song@...il.com, liming.wang@...driver.com, jkacur@...hat.com
Subject: Re: [PATCH] sched, deadline: Properly initialize def_dl_bandwidth
 lock

On Tue, Dec 17, 2013 at 03:17:53PM -0500, Steven Rostedt wrote:
> 
> Spinlocks even in structures require to be properly initialized.
> 
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
> 
> Index: linux-rt.git/kernel/sched/deadline.c
> ===================================================================
> --- linux-rt.git.orig/kernel/sched/deadline.c
> +++ linux-rt.git/kernel/sched/deadline.c
> @@ -18,7 +18,9 @@
>  
>  #include <linux/slab.h>
>  
> -struct dl_bandwidth def_dl_bandwidth;
> +struct dl_bandwidth def_dl_bandwidth = {
> +	.dl_runtime_lock = __RAW_SPIN_LOCK_UNLOCKED(def_dl_bandwidth.dl_runtime_lock),
> +};
>  
>  static inline struct task_struct *dl_task_of(struct sched_dl_entity *dl_se)
>  {

The thing is, init_dl_bandwidth() in sched_init() is supposed to already
do that. The stacktrace you provided out of band:

[    0.000000]  [<ffffffff814e2aca>] spin_bug+0x2b/0x2d
[    0.000000]  [<ffffffff8107535c>] do_raw_spin_lock+0x27/0x104
[    0.000000]  [<ffffffff814ec32e>] _raw_spin_lock+0x20/0x24
[    0.000000]  [<ffffffff8106fc41>] init_dl_bw+0x2d/0x79^M
[    0.000000]  [<ffffffff810607b9>] init_rootdomain+0x20/0x4f^M
[    0.000000]  [<ffffffff81b0db07>] sched_init+0x69/0x432
[    0.000000]  [<ffffffff81af3b6c>] start_kernel+0x201/0x41c
[    0.000000]  [<ffffffff81af3773>] ? repair_env_string+0x56/0x56
[    0.000000]  [<ffffffff81af3487>] x86_64_start_reservations+0x2a/0x2c
[    0.000000]  [<ffffffff81af357b>] x86_64_start_kernel+0xf2/0xf9

Has clue though. So it appears we use the lock before we reach that
init_dl_bandwidth.

The below hunk should fix things up -- merged into patch 9/13.

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6768,15 +6768,15 @@ void __init sched_init(void)
 #endif /* CONFIG_CPUMASK_OFFSTACK */
 	}
 
-#ifdef CONFIG_SMP
-	init_defrootdomain();
-#endif
-
 	init_rt_bandwidth(&def_rt_bandwidth,
 			global_rt_period(), global_rt_runtime());
 	init_dl_bandwidth(&def_dl_bandwidth,
 			global_dl_period(), global_dl_runtime());
 
+#ifdef CONFIG_SMP
+	init_defrootdomain();
+#endif
+
 #ifdef CONFIG_RT_GROUP_SCHED
 	init_rt_bandwidth(&root_task_group.rt_bandwidth,
 			global_rt_period(), global_rt_runtime());
--
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/