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

[PATCH v5 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).

v4…v5: https://patch.msgid.link/20261019-vsprintf_kallsym-v4-0-ffd3c012d00c@linutronix.de
 - Use %p instead snprintf_default_pointer(). This might recurse back to
   vsnprintf() if used by '%ps' but it already did so. Suggested by Petr
   Mladek.

v3…v4: https://patch.msgid.link/20261015-vsprintf_kallsym-v3-0-c175c27f38b7@linutronix.de
 - In symbol_string(), !CONFIG_KALLSYMS case, use the 'value' argument
   instead 'ptr' to consider the '%pR' case. Noticed by lkp@intel.com.
 - The kerneldoc for snprintf_default_pointer() must describe the 'len'
   argument instead the not existing 'size'. Noticed by lkp@intel.com.
 - sashiko noticed that the string isn't properly terminated if the
   input buffer is too small.

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

 kernel/kallsyms.c | 2 +-
 lib/vsprintf.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
---
base-commit: 091b06388b117fefdcb63336ac09956a4914b961
change-id: 20261015-vsprintf_kallsym-73d5ed6f800e

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

[PATCH v5 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.

Reviewed-by: Petr Mladek <pmladek@suse.com>
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..40565c857913d 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, (void *)value, spec);
 #endif
 }
 

-- 
2.55.0

[PATCH v5 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. This affects passed pointers and
callchain content since it is not part of the register dump.

Use %p for the hashed pointer output if the pointer has not been
resolved.

Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/kallsyms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index aec2f06858afd..6a3e0ded8df1a 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -483,7 +483,7 @@ 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 sprintf(buffer, "0x%p", address - symbol_offset);
 
 	offset -= symbol_offset;
 

-- 
2.55.0

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

[kernel test robot]

Hi Sebastian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 091b06388b117fefdcb63336ac09956a4914b961]

url:    https://github.com/intel-lab-lkp/linux/commits/Sebastian-Andrzej-Siewior/vsprintf-Don-t-leak-pointers-for-ps-without-KALLSYMS-enabled/20261023-184623
base:   091b06388b117fefdcb63336ac09956a4914b961
patch link:    https://lore.kernel.org/r/20261023-vsprintf_kallsym-v5-2-57ff92187c1d%40linutronix.de
patch subject: [PATCH v5 2/2] kallsyms: Don't leak pointers for unresolved symbols
config: sh-rsk7201_defconfig (https://download.01.org/0day-ci/archive/20261024/202610241049.9evBxlP8-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 16.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261024/202610241049.9evBxlP8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610241049.9evBxlP8-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/kallsyms.c: In function '__sprint_symbol':
>> kernel/kallsyms.c:486:44: warning: format '%p' expects argument of type 'void *', but argument 3 has type 'long unsigned int' [-Wformat=]
     486 |                 return sprintf(buffer, "0x%p", address - symbol_offset);
         |                                           ~^   ~~~~~~~~~~~~~~~~~~~~~~~
         |                                            |           |
         |                                            void *      long unsigned int
         |                                           %ld


vim +486 kernel/kallsyms.c

   469	
   470	/* Look up a kernel symbol and return it in a text buffer. */
   471	static int __sprint_symbol(char *buffer, unsigned long address,
   472				   int symbol_offset, int add_offset, int add_buildid)
   473	{
   474		char *modname;
   475		const unsigned char *buildid;
   476		unsigned long offset, size;
   477		int len;
   478	
   479		/* Prevent module removal until modname and modbuildid are printed */
   480		guard(rcu)();
   481	
   482		address += symbol_offset;
   483		len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
   484					       buffer);
   485		if (!len)
 > 486			return sprintf(buffer, "0x%p", address - symbol_offset);
   487	
   488		offset -= symbol_offset;
   489	
   490		if (add_offset)
   491			len += sprintf(buffer + len, "+%#lx/%#lx", offset, size);
   492	
   493		if (modname) {
   494			len += sprintf(buffer + len, " [%s", modname);
   495			if (add_buildid)
   496				len += append_buildid(buffer + len, modname, buildid);
   497			len += sprintf(buffer + len, "]");
   498		}
   499	
   500		return len;
   501	}
   502	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki