Re: [DISCUSSION] vsprintf: the current state of restricted pointers (%pK)

[Petr Mladek <pmladek@suse.com>]

On Tue 2025-01-14 16:35:57, Andy Shevchenko wrote:
> On Mon, Jan 13, 2025 at 05:46:44PM +0100, Thomas Weißschuh wrote:
> > Hi everybody,
> > 
> > as you know, leaking raw kernel pointers to the user is problematic as
> > they can be used to break KASLR.
> > Therefore back in 2011 the %pK format specifier was added [0], printing
> > certain pointers zeroed out or raw depending on the usage context.
> > Then in 2017 even the default %p format was changed to hash the pointers [1].
> > 
> > Both mechanisms are similar in their intention but have different,
> > cross-interacting effects and configuration knobs.
> > The end result is not always obvious. For example:
> > * "no_hash_pointers" does not work for %pK if kernel.kptr_restrict>=1
> > * If kernel.kptr_restrict=1, "restricted" pointers are effectively
> >   less restricted than "normal" pointers.
> > * For other values of kernel.kptr_restrict %p and %pK have the same
> >   security properties, but still different string representations.
> > 
> > Additionally the current usage of %pK is incorrect in many cases.
> > As %pK relies on the current task context for its permission check, it
> > was only ever meant to be used from procfs/sysfs/debugfs handlers [2].
> > In reality many callers use it through printk(), leaking addresses
> > into dmesg. While restricted_pointer() tries to detect some of such
> > situations at runtime, this check is not and can not be always complete.
> > 
> > File handlers which could use %pK correctly today, often use
> > kallsyms_show_value() instead. This is similar, but checks explicitly
> > against the credentials from an opened file instead of the implicit task
> > credentials. This behavior was the goal for %pK all along [3].
> 
> > Is it time to inspect the users of %pK and migrate them to either
> > %p/%px, kallsyms_show_value() or some similar new API?
> > Then alias %pK to %p, maybe removing it at some point.
> 
> To me this paragraph sounds like a good plan, which I agree on!

+1

> > A different, but slightly related issue occurs with PREEMPT_RT.
> > Calling printk("%pK") while holding a raw spinlock will trigger an
> > invalid wait context and latency spikes if an LSM using sleeping
> > spinlocks is enabled.
> > As printk() should be callable from any context this is an issue.
> > Removing the implicit group check would also avoid this.

Good to know.

Best Regards,
Petr