linux-kernel - [PATCH] lockdep: Fix wait context check on softirq for PREEMPT

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

Message-Id: <20241202012017.14910-1-ryotkkr98@gmail.com>
Date: Mon,  2 Dec 2024 10:20:17 +0900
From: Ryo Takakura <ryotkkr98@...il.com>
To: peterz@...radead.org,
	mingo@...hat.com,
	will@...nel.org,
	longman@...hat.com,
	boqun.feng@...il.com,
	bigeasy@...utronix.de,
	clrkwllms@...nel.org,
	rostedt@...dmis.org,
	tglx@...utronix.de
Cc: linux-kernel@...r.kernel.org,
	linux-rt-devel@...ts.linux.dev,
	Ryo Takakura <ryotkkr98@...il.com>
Subject: [PATCH] lockdep: Fix wait context check on softirq for PREEMPT_RT

Commit 0c1d7a2c2d32 ("lockdep: Remove softirq accounting on
PREEMPT_RT.") stopped updating @softirq_context on PREEMPT_RT
to ignore "inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage"
as the report accounts softirq context which PREEMPT_RT doesn't
have to.

However, wait context check still needs to report mutex usage
within softirq, even when its threaded on PREEMPT_RT. The check
is failing to report the usage as task_wait_context() checks if
its in softirq by referencing @softirq_context, ending up not 
assigning the correct wait type of LD_WAIT_CONFIG for PREEMPT_RT's
softirq.

[    0.184549]   | wait context tests |
[    0.184549]   --------------------------------------------------------------------------
[    0.184549]                                  | rcu  | raw  | spin |mutex |
[    0.184549]   --------------------------------------------------------------------------
[    0.184550]                in hardirq context:  ok  |  ok  |  ok  |  ok  |
[    0.185083] in hardirq context (not threaded):  ok  |  ok  |  ok  |  ok  |
[    0.185606]                in softirq context:  ok  |  ok  |  ok  |FAILED|

Account softirq context but only when !PREEMPT_RT so that
task_wait_context() returns LD_WAIT_CONFIG as intended.

Signed-off-by: Ryo Takakura <ryotkkr98@...il.com>


---

Hi! 

I wasn't able come up with a way to fix the wait context test while 
keeping the commit 0c1d7a2c2d32 ("lockdep: Remove softirq accounting 
on PREEMPT_RT.") without referencing @softirq_context...
Hoping to get a feedback on it!

Also I wonder if the test can be skipped as I believe its taken care 
by spinlock wait context test since the PREEMPT_RT's softirq context is 
protected by local_lock which is mapped to rt_spinlock.

Thanks!
Ryo Takakura

---
 include/linux/irqflags.h |  2 +-
 kernel/locking/lockdep.c | 11 +++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 3f003d5fd..c33c3bbd8 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -121,7 +121,7 @@ do {						\
 # define lockdep_irq_work_exit(__work)		do { } while (0)
 #endif
 
-#if defined(CONFIG_TRACE_IRQFLAGS) && !defined(CONFIG_PREEMPT_RT)
+#if defined(CONFIG_TRACE_IRQFLAGS)
 # define lockdep_softirq_enter()		\
 do {						\
 	current->softirq_context++;		\
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 536bd4715..2a508d6a6 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4602,7 +4602,7 @@ mark_usage(struct task_struct *curr, struct held_lock *hlock, int check)
 				if (!mark_lock(curr, hlock,
 						LOCK_USED_IN_HARDIRQ_READ))
 					return 0;
-			if (curr->softirq_context)
+			if (!IS_ENABLED(CONFIG_PREEMPT_RT) && curr->softirq_context)
 				if (!mark_lock(curr, hlock,
 						LOCK_USED_IN_SOFTIRQ_READ))
 					return 0;
@@ -4610,7 +4610,7 @@ mark_usage(struct task_struct *curr, struct held_lock *hlock, int check)
 			if (lockdep_hardirq_context())
 				if (!mark_lock(curr, hlock, LOCK_USED_IN_HARDIRQ))
 					return 0;
-			if (curr->softirq_context)
+			if (!IS_ENABLED(CONFIG_PREEMPT_RT) && curr->softirq_context)
 				if (!mark_lock(curr, hlock, LOCK_USED_IN_SOFTIRQ))
 					return 0;
 		}
@@ -4651,8 +4651,11 @@ mark_usage(struct task_struct *curr, struct held_lock *hlock, int check)
 
 static inline unsigned int task_irq_context(struct task_struct *task)
 {
-	return LOCK_CHAIN_HARDIRQ_CONTEXT * !!lockdep_hardirq_context() +
-	       LOCK_CHAIN_SOFTIRQ_CONTEXT * !!task->softirq_context;
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		return LOCK_CHAIN_HARDIRQ_CONTEXT * !!lockdep_hardirq_context();
+	else
+		return LOCK_CHAIN_HARDIRQ_CONTEXT * !!lockdep_hardirq_context() +
+		       LOCK_CHAIN_SOFTIRQ_CONTEXT * !!task->softirq_context;
 }
 
 static int separate_irq_context(struct task_struct *curr,
-- 
2.34.1