Message-ID: <20250113171731-dc10e3c1-da64-4af0-b767-7c7070468023@linutronix.de>
Date: Mon, 13 Jan 2025 17:46:44 +0100
From: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
To: Greg KH <gregkh@...uxfoundation.org>, Kees Cook <kees@...nel.org>, 
	Petr Mladek <pmladek@...e.com>, Steven Rostedt <rostedt@...dmis.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, "Gustavo A. R. Silva" <gustavoars@...nel.org>, 
	John Ogness <john.ogness@...utronix.de>, Rasmus Villemoes <linux@...musvillemoes.dk>, 
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Sergey Senozhatsky <senozhatsky@...omium.org>, 
	Thomas Gleixner <tglx@...utronix.de>, linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [DISCUSSION] vsprintf: the current state of restricted pointers (%pK)

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.


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.


Thanks,
Thomas


[0] 455cd5ab305c ("kptr_restrict for hiding kernel pointers from unprivileged users"),
[1] ad67b74d2469 ("printk: hash addresses printed with %p")
[2] Documentation/core-api/printk-formats.rst:
	This modifier is *only* intended when producing content of a file read by
	userspace from e.g. procfs or sysfs, not for dmesg. Please refer to the
	section about %p above for discussion about how to manage hashing pointers
	in printk().
[3] Documentation/admin-guide/sysctl/kernel.rst:
	"The correct long-term solution is to do the permission checks at open() time."
[4] https://lore.kernel.org/lkml/20241217142032.55793-1-acarmina@redhat.com/

Powered by blists - more mailing lists