linux-kernel - [RFC v4 2/4] softirq: reorder trace_softirqs

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: <20180417040748.212236-3-joelaf@google.com>
Date:   Mon, 16 Apr 2018 21:07:46 -0700
From:   Joel Fernandes <joelaf@...gle.com>
To:     linux-kernel@...r.kernel.org, linux-rt-users@...r.kernel.org
Cc:     Joel Fernandes <joelaf@...gle.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Peter Zilstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Tom Zanussi <tom.zanussi@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Thomas Glexiner <tglx@...utronix.de>,
        Boqun Feng <boqun.feng@...il.com>,
        Paul McKenney <paulmck@...ux.vnet.ibm.com>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Fenguang Wu <fengguang.wu@...el.com>,
        Baohong Liu <baohong.liu@...el.com>,
        Vedang Patel <vedang.patel@...el.com>
Subject: [RFC v4 2/4] softirq: reorder trace_softirqs_on to prevent lockdep splat

Currently there's an issue which is not always reproducible, where the
softirqs annotation in lockdep goes out of sync with the reality of the
world and causes a lockdep splat. This is because the preemptoff tracer
calls into lockdep which can cause a softirqs invalid annotation splat.

The same issue was fixed in local_bh_disable_ip.
/*
 * The preempt tracer hooks into preempt_count_add and will break
 * lockdep because it calls back into lockdep after SOFTIRQ_OFFSET
 * is set and before current->softirq_enabled is cleared.
 * We must manually increment preempt_count here and manually
 * call the trace_preempt_off later.
 */

I am not fully sure why the issue is reproducible with my patches easily,
but it seems like the same ordering issue can exist so we ought to fix
it.

Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: Peter Zilstra <peterz@...radead.org>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: Tom Zanussi <tom.zanussi@...ux.intel.com>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: Thomas Glexiner <tglx@...utronix.de>
Cc: Boqun Feng <boqun.feng@...il.com>
Cc: Paul McKenney <paulmck@...ux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@...il.com>
Cc: Randy Dunlap <rdunlap@...radead.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Fenguang Wu <fengguang.wu@...el.com>
Cc: Baohong Liu <baohong.liu@...el.com>
Cc: Vedang Patel <vedang.patel@...el.com>
Signed-off-by: Joel Fernandes <joelaf@...gle.com>
---
 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index 177de3640c78..8a040bcaa033 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -139,9 +139,13 @@ static void __local_bh_enable(unsigned int cnt)
 {
 	lockdep_assert_irqs_disabled();
 
+	if (preempt_count() == cnt)
+		trace_preempt_on(CALLER_ADDR0, get_lock_parent_ip());
+
 	if (softirq_count() == (cnt & SOFTIRQ_MASK))
 		trace_softirqs_on(_RET_IP_);
-	preempt_count_sub(cnt);
+
+	__preempt_count_sub(cnt);
 }
 
 /*
-- 
2.17.0.484.g0c8726318c-goog