Re: [PATCH] lib/vsprintf: Validate sleepable context during restrictred pointer formatting

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-03-18 12:03:39 [+0100], Petr Mladek wrote:
> Now, really with John and Sebastian in Cc.
> 
> On Wed 2026-03-18 09:48:06, Thomas Weißschuh wrote:
> > On Tue, Mar 17, 2026 at 12:41:23PM +0100, Thomas Weißschuh wrote:
> > > Depending on the system configuration, the restricted pointer formatting
> > > might call into the security subsystem which might sleep.
> > > As %pK is intended to be only used from read handlers of virtual files,
> > > which always run in task context, this should never happen 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 might_sleep(), so that misuse of %pK from atomic context is
> > > detected right away.
> > > 
> > > 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>
> > > ---
> > > This depends on commit 5886cc8f895b ("drm/msm/dpu: Don't use %pK through
> > > printk (again)"), which was merged in v7.0-rc2.
> > > ---
> > >  lib/vsprintf.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > > index 800b8ac49f53..eb9dbb28fb9b 100644
> > > --- a/lib/vsprintf.c
> > > +++ b/lib/vsprintf.c
> > > @@ -862,6 +862,9 @@ static noinline_for_stack
> > >  char *restricted_pointer(char *buf, char *end, const void *ptr,
> > >  			 struct printf_spec spec)
> > >  {
> > > +	/* Only usable from task context, The call to has_capability_noaudit() might sleep. */
> > > +	might_sleep();
> > > +
> > 
> > So might_sleep() is not actually the right thing to do here.
> > Some callers use %pK under a spinlock, which fails the might_sleep() check.
> > However this is fine to do, as has_capability_noaudit() also only takes a
> > spinlock.
> 
> My understanding is that we want to simulate that we are going
> to call a normal spin_lock() which might sleep in PREEMPT_RT.
> So that we trigger the warning when %pK is used under
> a raw_spin_lock.
> 
> It might likely be achieved by adding a "fake" spin_lock and might_lock().
> I am not sure if there is an easier way by using just
> a struct lockdep_map.
> 
> I am always a bit confused by the lockdep annotation.

might_sleep() seems reasonable if any usage under spinlock_t can be
excluded. If the usage is intended to happen only from task context then
in_task() would make sense. This would already ban any softirq context
and everything else I know we had in the past.

The fake spinlock_t could be achieved by just something like
might_lock() which would avoid any overhead of real locking.

But do we still need %pK? There are far more %p users in seq_printf()
context as per
	git grep -E 'seq_printf.*%p\>'

than %pK. According to commit 312b4e226951f ("vsprintf: check real
user/group id for %pK") it is temporary as of 2013. There should be a
check at open time instead.

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

Sebastian