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

[Petr Mladek <pmladek@suse.com>]

On Mon 2026-10-19 12:21:51, Sebastian Andrzej Siewior wrote:
> __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          | 27 +++++++++++++++++++++++++++
>  3 files changed, 30 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));

I do not see any obvious problem with using:

	return snprintf(buffer, KSYM_SYMBOL_LEN, "%p",
			(void *)(address - symbol_offset));

What was the motivation to create a custom wrapper, please?

Best Regards,
Petr

>  
>  	offset -= symbol_offset;
>  
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 40565c857913d..b2ca1d195bc9e 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -849,6 +849,33 @@ 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
> + * @len: 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.
> + *
> + * See the vsnprintf() documentation for the return value.
> + */
> +int snprintf_default_pointer(char *buf, unsigned int len, const void *ptr)
> +{
> +	struct printf_spec spec = {
> +		.base = 10,
> +		.precision = -1,
> +		.field_width = -1,
> +	};
> +	unsigned int w;
> +	char *endp;
> +
> +	endp = default_pointer(buf, buf + len, ptr, spec);
> +	w = min_t(unsigned int, len - 1, endp - buf);
> +	buf[w] = '\0';
> +	return endp - buf;
> +}
> +
>  int kptr_restrict __read_mostly;
>  EXPORT_SYMBOL_FOR_MODULES(kptr_restrict, "printf_kunit");
>  
> 
> -- 
> 2.55.0