Re: [PATCH v19 33/40] dept: call dept_hardirqs_off() in local_irq_*() regardless of irq state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hides missing assembly trace annotations, causing DEPT and Lockdep states to diverge and allowing NMIs to silently revert the workaround.
- [Medium] Exporting `dept_hardirqs_off` as GPL-only breaks the build/loading of non-GPL modules that use `local_irq_save()` or `local_irq_disable()`.
--

commit 3f12c8d53e9de3037ef421690b95c2ca6667e3a1
Author: Byungchul Park <byungchul@sk.com>

dept: call dept_hardirqs_off() in local_irq_*() regardless of irq state

For dept to function properly, dept_task()->hardirqs_enabled must be set
correctly.  If it fails to set this value to false, for example, dept
may mistakenly think irq is still enabled even when it's not.

> diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
> index d8b9cf093f83d..586f5bad4da78 100644
> --- a/include/linux/irqflags.h
> +++ b/include/linux/irqflags.h
> @@ -214,6 +214,13 @@ extern void warn_bogus_irq_restore(void);
>  		raw_local_irq_disable();		\
>  		if (!was_disabled)			\
>  			trace_hardirqs_off();		\
> +		/*					\
> +		 * Just in case that C code has missed	\
> +		 * trace_hardirqs_off() at the first	\
> +		 * place e.g. disabling irq at asm code.\
> +		 */					\
> +		else					\
> +			dept_hardirqs_off();		\
>  	} while (0)

[Severity: High]
Can NMIs silently revert this workaround?

If assembly code disables IRQs but misses the trace_hardirqs_off() annotation,
and then C code calls local_irq_disable(), this skips trace_hardirqs_off()
but executes the new dept_hardirqs_off() branch. Lockdep still thinks IRQs
are enabled, while DEPT thinks they are disabled.

If an NMI occurs while in this state, irqentry_nmi_enter() saves the Lockdep
state (which is incorrectly 'enabled'). Upon NMI exit, irqentry_nmi_exit()
restores the saved Lockdep state by calling lockdep_hardirqs_on(), which
unconditionally calls dept_hardirqs_on().

Will this force DEPT back into an incorrect 'enabled' state while hardware
IRQs are actually disabled, potentially leading to false-positive circular
dependency reports?

[ ... ]

> diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
> index 007e1bc7d201e..35a3667ac8b3c 100644
> --- a/kernel/dependency/dept.c
> +++ b/kernel/dependency/dept.c
> @@ -2095,6 +2095,7 @@ void noinstr dept_hardirqs_off(void)
>  	 */
>  	dept_task()->hardirqs_enabled = false;
>  }
> +EXPORT_SYMBOL_GPL(dept_hardirqs_off);

[Severity: Medium]
Will this break the build or loading of non-GPL modules?

Because dept_hardirqs_off() is now called directly from universal inline
macros like local_irq_disable() and local_irq_save(), any out-of-tree
module marked with a non-GPL license that invokes these macros will fail
to link or load when CONFIG_DEPT is enabled.

Standard trace_hardirqs_off() avoids this issue by being exported via
EXPORT_SYMBOL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=33