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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4Z2TW_HaANvT4VH@smile.fi.intel.com>
Date: Tue, 14 Jan 2025 16:35:57 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: Greg KH <gregkh@...uxfoundation.org>, Kees Cook <kees@...nel.org>,
	Petr Mladek <pmladek@...e.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"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: Re: [DISCUSSION] vsprintf: the current state of restricted pointers
 (%pK)

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!

> 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.

> [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/

-- 
With Best Regards,
Andy Shevchenko