iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lockdep: report broken irq restoration

Browse Source 
We generally expect local_irq_save() and local_irq_restore() to be
paired and sanely nested, and so local_irq_restore() expects to be
called with irqs disabled. Thus, within local_irq_restore() we only
trace irq flag changes when unmasking irqs.

This means that a sequence such as:

| local_irq_disable();
| local_irq_save(flags);
| local_irq_enable();
| local_irq_restore(flags);

... is liable to break things, as the local_irq_restore() would mask
irqs without tracing this change. Similar problems may exist for
architectures whose arch_irq_restore() function depends on being called
with irqs disabled.

We don't consider such sequences to be a good idea, so let's define
those as forbidden, and add tooling to detect such broken cases.

This patch adds debug code to WARN() when raw_local_irq_restore() is
called with irqs enabled. As raw_local_irq_restore() is expected to pair
with raw_local_irq_save(), it should never be called with irqs enabled.

To avoid the possibility of circular header dependencies between
irqflags.h and bug.h, the warning is handled in a separate C file.

The new code is all conditional on a new CONFIG_DEBUG_IRQFLAGS symbol
which is independent of CONFIG_TRACE_IRQFLAGS. As noted above such cases
will confuse lockdep, so CONFIG_DEBUG_LOCKDEP now selects
CONFIG_DEBUG_IRQFLAGS.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20210111153707.10071-1-mark.rutland@arm.com
This commit is contained in:
Mark Rutland 2021-01-11 15:37:07 +00:00 committed by Peter Zijlstra
parent 2f0df49c89
commit 997acaf6b4
4 changed files with 32 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
include/linux/irqflags.h
View File

@ -149,6 +149,17 @@ do { \
# define start_critical_timings() do { } while (0)
#endif
#ifdef CONFIG_DEBUG_IRQFLAGS
extern void warn_bogus_irq_restore(void);
#define raw_check_bogus_irq_restore() \
do { \
if (unlikely(!arch_irqs_disabled())) \
warn_bogus_irq_restore(); \
} while (0)
#else
#define raw_check_bogus_irq_restore() do { } while (0)
#endif
/*
* Wrap the arch provided IRQ routines to provide appropriate checks.
*/
@ -162,6 +173,7 @@ do { \
#define raw_local_irq_restore(flags) \
do { \
typecheck(unsigned long, flags); \
raw_check_bogus_irq_restore(); \
arch_local_irq_restore(flags); \
} while (0)
#define raw_local_save_flags(flags) \

1
kernel/locking/Makefile
View File

@ -15,6 +15,7 @@ CFLAGS_REMOVE_mutex-debug.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_rtmutex-debug.o = $(CC_FLAGS_FTRACE)
endif
obj-$(CONFIG_DEBUG_IRQFLAGS) += irqflag-debug.o
obj-$(CONFIG_DEBUG_MUTEXES) += mutex-debug.o
obj-$(CONFIG_LOCKDEP) += lockdep.o
ifeq ($(CONFIG_PROC_FS),y)

11
kernel/locking/irqflag-debug.c Normal file
View File

@ -0,0 +1,11 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <linux/bug.h>
#include <linux/export.h>
#include <linux/irqflags.h>
void warn_bogus_irq_restore(void)
{
WARN_ONCE(1, "raw_local_irq_restore() called with IRQs enabled\n");
}
EXPORT_SYMBOL(warn_bogus_irq_restore);

8
lib/Kconfig.debug
View File

@ -1343,6 +1343,7 @@ config LOCKDEP_SMALL
config DEBUG_LOCKDEP
bool "Lock dependency engine debugging"
depends on DEBUG_KERNEL && LOCKDEP
select DEBUG_IRQFLAGS
help
If you say Y here, the lock dependency engine will do
additional runtime checks to debug itself, at the price
@ -1431,6 +1432,13 @@ config TRACE_IRQFLAGS_NMI
depends on TRACE_IRQFLAGS
depends on TRACE_IRQFLAGS_NMI_SUPPORT
config DEBUG_IRQFLAGS
bool "Debug IRQ flag manipulation"
help
Enables checks for potentially unsafe enabling or disabling of
interrupts, such as calling raw_local_irq_restore() when interrupts
are enabled.
config STACKTRACE
bool "Stack backtrace support"
depends on STACKTRACE_SUPPORT