Re: [PATCH v3 3/5] lib/vsprintf: Validate spinlock context during restricted pointer formatting

[Petr Mladek <pmladek@suse.com>]

On Wed 2026-05-20 10:40:02, Thomas Weißschuh wrote:
> Depending on the system configuration, the restricted pointer formatting
> might call into the security subsystem which takes spinlocks, which
> might sleep under PREEMPT_RT. As %pK is intended to be only used from
> read handlers of virtual files, which always run in task context,
> this should not be a problem in practice.
> However, developers have used %pK before from atomic context without
> realizing this restriction. While all existing user of %pK through
> printk() have been removed, new ones might be reintroduced accidentally
> in the future.
> 
> Add a lockdep annotation to unconditionally introduce a fake spinlock in
> restricted_pointer(), so lockdep can detect misuse even if the current
> test system configuration would not exhibit the issue.
> 
> Link: https://lore.kernel.org/lkml/20250113171731-dc10e3c1-da64-4af0-b767-7c7070468023@linutronix.de/
> Link: https://lore.kernel.org/lkml/20241217142032.55793-1-acarmina@redhat.com/
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  lib/vsprintf.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 9f359b31c8d1..021db95087fe 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -29,6 +29,7 @@
>  #include <linux/hex.h>
>  #include <linux/kernel.h>
>  #include <linux/kallsyms.h>
> +#include <linux/lockdep.h>
>  #include <linux/math64.h>
>  #include <linux/uaccess.h>
>  #include <linux/ioport.h>
> @@ -862,6 +863,14 @@ static noinline_for_stack
>  char *restricted_pointer(char *buf, char *end, const void *ptr,
>  			 struct printf_spec spec)
>  {
> +	/*
> +	 * has_capability_noaudit() may use spinlocks.
> +	 * Make sure %pK is only used from valid contexts.
> +	 */
> +	static DEFINE_WAIT_ASSERT_MAP(vsprintf_restricted_pointer_map, LD_WAIT_CONFIG);
> +
> +	guard(lock_map_acquire)(&vsprintf_restricted_pointer_map);

Here is Sashiko AI review from
https://sashiko.dev/#/patchset/20260520-restricted-pointers-final-v3-0-76bca6a6ab3f%40linutronix.de

<paste>
Will this single global lockdep map create false dependencies and
inconsistent IRQ usage splats?

Because lock_map_acquire() acquires the fake lock with trylock=0, lockdep
fully tracks it in the dependency graph. Since this is a single global
static map, it acts as a central bottleneck: every lock held by any caller
of %pK forms a forward dependency to the fake lock, and the fake lock forms
a forward dependency to any lock acquired inside %pK. This artificially
links completely independent locking contexts across the kernel, which might
generate massive false-positive circular deadlock warnings.

Furthermore, if %pK is evaluated in an IRQ context, the lock map is marked
USED_IN_HARDIRQ. If it is later evaluated in a process context with IRQs
enabled, the map is marked ENABLED_HARDIRQ. Won't lockdep immediately
trigger an inconsistent IRQ usage splat upon detecting both states?

While using trylock=1 (like lock_map_acquire_try()) might seem like a way
around this, lockdep's check_wait_context() explicitly bypasses wait-context
validation when trylock=1. This would defeat the purpose of this patch.

Is there a different mechanism to assert the wait context without fully
tracking it in the lock dependency graph?
</paste>

My opinion:

The fear of generating massive false-positive cirular locks does
not fit here. It is the opposite. This fake lock represents the
various locks which might be taken by has_capability_noaudit()
on purpose. And %pK formatting must never be used under these
locks because it might create deadlock otherwise.

The fear of inconsistent IRQ usage reports makes some sense.
It might happen and it might be confusing because %pK fomatting
must not be used in IRQ context at all. The misleading IRQ
reports should be prevented by adding the lockdep_assert(in_task)
later in this patch set.

Resume:

This patch looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

>  	switch (kptr_restrict) {
>  	case 0:
>  		/* Handle as %p, hash and do _not_ leak addresses. */

Best Regards,
Petr