[PATCH v3 0/2] vsprintf: Don't leak pointers

[PATCH v3 0/2] vsprintf: Don't leak pointers

[Sebastian Andrzej Siewior]

It is possible to leak pointers via %ps without KALLSYMS enabled.
With KALLSYMS pointers can be leaked if they can not be resolved.

The tiny series has two patches, one for each issue.
I do follow the %p format instead of printing "(unknown)" since I care
more about getting it out of the way than the actual content (as long as
it is not the leaked pointer). Last time the discussion added the h:
prefix to a hashed pointer to identify it as such. I would like to keep
this part out of the series since there are few users that expect a
certain pointer width and adding two extra characters would break that
(we could strip the two 0 on 64bit but again, not related to this
problem here).

v2…v3: https://lore.kernel.org/all/20260821152614.2202196-1-bigeasy@linutronix.de/
 - Instead of "(unknown)" the hashed pointer is printed. This follows
   the generic %p flow.
 - In v2 I argued that it is better so print the actual pointer instead
   hiding it for post-mortem analysis. After looking at some backtraces
   with invalid pointers (not part of the kernel image) I realised the
   the PC is already part of the register dump. There I changed it back
   to what %p would do if it can't resolve a pointer. Again, following
   %p flow so it can be overridden at boot.

v1…v2: https://lore.kernel.org/all/20260814144854.746840-1-bigeasy@linutronix.de/
 - For %ps and !KALLSYMS no_hash_pointers is used to decide if the
   pointer should be written or not. This aligns with the %p policy.
 - For %ps and KALLSYMS, unresolved pointer continue to be leaked. It
   has been pointed out it might be useful to see them in backtraces/
   crashes if the return address became invalid.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Kees Cook <kees@kernel.org>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tycho Andersen <tycho@tycho.pizza>
To: linux-kernel@vger.kernel.org

Sebastian

---
Sebastian Andrzej Siewior (2):
      vsprintf: Don't leak pointers for %ps without KALLSYMS enabled
      kallsyms: Don't leak pointers for unresolved symbols

 include/linux/sprintf.h |  1 +
 kernel/kallsyms.c       |  3 ++-
 lib/vsprintf.c          | 27 ++++++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 2 deletions(-)
---
base-commit: ec8bf8280261294a9b9b4520414b42704c7f9f6b
change-id: 20261015-vsprintf_kallsym-73d5ed6f800e

Best regards,
--  
Sebastian Andrzej Siewior <bigeasy@linutronix.de>

[PATCH v3 1/2] vsprintf: Don't leak pointers for %ps without KALLSYMS enabled

[Sebastian Andrzej Siewior]

The "%ps" format modifier prints the name of the symbol which is more
valuable in terms of debugging and does not leak the actual pointer.

Without KALLSYMS it will leak the pointer which is not intended. The
default policy for pointers is to print a hashed value and not to leak
the actual pointer.

Print a hashed pointer for any symbol resolution witout KALLSYMS
enabled like it is the case for %p.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 lib/vsprintf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1d95ab9314bf7..a3db25cc1e567 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1008,7 +1008,7 @@ char *symbol_string(char *buf, char *end, void *ptr,
 
 	return string_nocheck(buf, end, sym, spec);
 #else
-	return special_hex_number(buf, end, value, sizeof(void *));
+	return default_pointer(buf, end, ptr, spec);
 #endif
 }
 

-- 
2.55.0

[PATCH v3 2/2] kallsyms: Don't leak pointers for unresolved symbols

[Sebastian Andrzej Siewior]

__sprint_symbol() is supposed to resolve the passed address to a symbol
name. If the symbol can not be resolved it will print the actual pointer
that was passed. The pointer policy is to not reveal actual pointer
values.
For post-mortem analysis the pointer in the backtrace shouldn't be
relevant. In BUG(), WARN() and panic() related backtraces the PC
register will contain the actual value.

Add snprintf_default_pointer() as a wrapper around default_pointer() and
use it __sprint_symbol() if the pointer has not been resolved.

Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/sprintf.h |  1 +
 kernel/kallsyms.c       |  3 ++-
 lib/vsprintf.c          | 25 +++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/include/linux/sprintf.h b/include/linux/sprintf.h
index f06f7b7850911..596c73a57371e 100644
--- a/include/linux/sprintf.h
+++ b/include/linux/sprintf.h
@@ -24,6 +24,7 @@ __scanf(2, 0) int vsscanf(const char *, const char *, va_list);
 /* These are for specific cases, do not use without real need */
 extern bool no_hash_pointers;
 void hash_pointers_finalize(bool slub_debug);
+int snprintf_default_pointer(char *buf, unsigned int len, const void *ptr);
 
 /* Used for Rust formatting ('%pA') */
 char *rust_fmt_argument(char *buf, char *end, const void *ptr);
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index aec2f06858afd..5d51c825c1e71 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -483,7 +483,8 @@ static int __sprint_symbol(char *buffer, unsigned long address,
 	len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
 				       buffer);
 	if (!len)
-		return sprintf(buffer, "0x%lx", address - symbol_offset);
+		return snprintf_default_pointer(buffer, KSYM_SYMBOL_LEN,
+						(void *)(address - symbol_offset));
 
 	offset -= symbol_offset;
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a3db25cc1e567..0ed7134ad2f41 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -849,6 +849,31 @@ static char *default_pointer(char *buf, char *end, const void *ptr,
 	return ptr_to_id(buf, end, ptr, spec);
 }
 
+/**
+ * snprintf_default_pointer - Format a pointer and place it in a buffer
+ * @buf: The buffer to place the result into
+ * @size: The size of the buffer, including the trailing null space
+ * @ptr: The pointer to format
+ *
+ * Formats the passed pointer the same way as the %p modifier. Does not write
+ * more than len bytes to buf including the NULL terminator.
+ *
+ * Returns the number of written bytes.
+ */
+int snprintf_default_pointer(char *buf, unsigned int len, const void *ptr)
+{
+	struct printf_spec spec = {
+		.base = 10,
+		.precision = -1,
+		.field_width = -1,
+	};
+	char *endp;
+
+	endp = default_pointer(buf, buf + len, ptr, spec);
+	*endp = '\0';
+	return endp - buf;
+}
+
 int kptr_restrict __read_mostly;
 EXPORT_SYMBOL_FOR_MODULES(kptr_restrict, "printf_kunit");
 

-- 
2.55.0