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

[Petr Mladek]

On Thu 2026-06-11 12:09:08, Thomas Weißschuh wrote:
> On Mon, Jun 08, 2026 at 04:21:45PM +0200, 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.
> > This is intentionally a single lock instance shared by all callers,
> > as that mirrors what can happen in the security subsystem.
> > 
> > This check comes intentionally after the in_task() one, to have the
> > clearer diagnostic first when the function is called from IRQ context,
> > which will trigger both.
> > 
> > Link: https://lore.kernel.org/lkml/20250113171731-dc10e3c1-da64-4af0-b767-7c7070468023@xxxxxxxxxxxxx/
> > Link: https://lore.kernel.org/lkml/20241217142032.55793-1-acarmina@xxxxxxxxxx/
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
> > Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
> > ---
> >  lib/vsprintf.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 09e0e5194d41..728a1acd69ae 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -864,7 +864,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);
> > +
> >  	lockdep_assert(in_task());
> > +	guard(lock_map_acquire)(&vsprintf_restricted_pointer_map);
> 
> The kernel test robot found a lockdep violation with this patch:
> https://lore.kernel.org/all/202606110945.d3871219-lkp@xxxxxxxxx/
> 
> My suspicion is that this is a pre-existing problem that was not visible
> to lockdep so far, exactly what this patch is supposed to mitigate.
> I'll investigate some more and try to reproduce it.

I was curious and found the following. The problematic patch seems to be:

[   92.754552][ T3827]   lock_acquire (locking/lockdep.c:5868)
[   92.757345][ T3827]   class_lock_map_acquire_constructor (linux/lockdep.h:557 (discriminator 1))
[   92.759895][ T3827]   restricted_pointer (vsprintf.c:874)
[   92.770231][ T3827]   pointer (vsprintf.c:2582)
[   92.772166][ T3827]   vsnprintf (vsprintf.c:2956)
[   92.774145][ T3827]   seq_printf (seq_file.c:392 seq_file.c:407)
[   92.776083][ T3827]   tcp6_seq_show (ipv6/tcp_ipv6.c:2147 (discriminator 1) ipv6/tcp_ipv6.c:2221 (discriminator 1))
[   92.778212][ T3827]   traverse (seq_file.c:112)
[   92.780125][ T3827]   seq_lseek (seq_file.c:324 (discriminator 1))
[   92.782106][ T3827]   proc_reg_llseek (proc/inode.c:283)
[   92.789616][ T3827]   __ia32_sys_lseek (read_write.c:391)
[   92.791786][ T3827]   ia32_sys_call (kbuild/obj/consumer/i386-randconfig-013-20260610/./arch/x86/include/generated/asm/syscalls_32.h:20)
[   92.793792][ T3827]   __do_fast_syscall_32 (x86/entry/syscall_32.c:83)
[   92.795973][ T3827]   do_fast_syscall_32 (x86/entry/syscall_32.c:332)
[   92.797992][ T3827]   do_SYSENTER_32 (x86/entry/syscall_32.c:370)
[   92.799987][ T3827]   entry_SYSENTER_32 (x86/entry/entry_32.S:835)

If I get it correctly then traverse() might take spin_lock_bh(lock)
via:

 + traverse()
   + tcp_seq_start()
     + tcp_get_idx()
       + established_get_idx()
	 + established_get_first()
	  + spin_lock_bh(lock);

And then it calls vsnprintf(" %pK" in

  + tcp6_seq_show()
    + get_tcp6_sock()
      + seq_printf()

By other words, it calls vsnprintf("%pK") with disabled softirqs.

If I get it correctly then this might create a deadlock because
vsnprinf("%pK") can be called also with (soft)irqs enabled,
then soft(irq) might came and want to take the spin_lock(lock)...

As a result, we should not use %pK in tcp6_seq_show().

Best Regards,
Petr