Re: [PATCH v3 03/12] dump_stack: Add vmlinux build ID to stack traces

[Petr Mladek]

On Tue 2021-03-30 20:05:11, Stephen Boyd wrote:
> Add the running kernel's build ID[1] to the stacktrace information
> header.  This makes it simpler for developers to locate the vmlinux with
> full debuginfo for a particular kernel stacktrace. Combined with
> scripts/decode_stracktrace.sh, a developer can download the correct
> vmlinux from a debuginfod[2] server and find the exact file and line
> number for the functions plus offsets in a stacktrace.
> 
> This is especially useful for pstore crash debugging where the kernel
> crashes are recorded in the pstore logs and the recovery kernel is
> different or the debuginfo doesn't exist on the device due to space
> concerns (the data can be large and a security concern). The stacktrace
> can be analyzed after the crash by using the build ID to find the
> matching vmlinux and understand where in the function something went
> wrong.
> 
> Example stacktrace from lkdtm:
> 
>  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm]
>  Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE
>  CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 aa23f7a1231c229de205662d5a9e0d4c580f19a1
>  Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
>  pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
>  pc : lkdtm_WARNING+0x28/0x30 [lkdtm]
> 
> The hex string aa23f7a1231c229de205662d5a9e0d4c580f19a1 is the build ID,
> following the kernel version number. Put it all behind a config option,
> STACKTRACE_BUILD_ID, so that kernel developers can remove this
> information if they decide it is too much.
> 
> diff --git a/lib/dump_stack.c b/lib/dump_stack.c
> index f5a33b6f773f..07e0214c2d4d 100644
> --- a/lib/dump_stack.c
> +++ b/lib/dump_stack.c
> @@ -45,13 +52,14 @@ void __init dump_stack_set_arch_desc(const char *fmt, ...)
>   */
>  void dump_stack_print_info(const char *log_lvl)
>  {
> -	printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s\n",
> +	printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s" BUILD_ID_STR "\n",
>  	       log_lvl, raw_smp_processor_id(), current->pid, current->comm,
>  	       kexec_crash_loaded() ? "Kdump: loaded " : "",
>  	       print_tainted(),
>  	       init_utsname()->release,
>  	       (int)strcspn(init_utsname()->version, " "),
> -	       init_utsname()->version);
> +	       init_utsname()->version,
> +	       IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID) ? vmlinux_build_id : "");

I get the following warning when CONFIG_STACKTRACE_BUILD_ID is not
enabled:

  CC      lib/dump_stack.o
lib/dump_stack.c: In function ‘dump_stack_print_info’:
lib/dump_stack.c:62:67: warning: pointer type mismatch in conditional expression
         IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID) ? vmlinux_build_id : "");
                                                                   ^
lib/dump_stack.c:55:9: warning: format ‘%s’ expects argument of type ‘char *’, but argument 11 has type ‘void *’ [-Wformat=]
  printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s" BUILD_ID_STR "\n",
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
lib/dump_stack.c:43:24: note: format string is defined here
 #define BUILD_ID_STR "%s"
                       ~^
                       %p

and this when the options is enabled:

  CC      lib/dump_stack.o
lib/dump_stack.c: In function ‘dump_stack_print_info’:
lib/dump_stack.c:62:67: warning: pointer type mismatch in conditional expression
         IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID) ? vmlinux_build_id : "");


I think that you need to use something like:

#ifdef CONFIG_STACKTRACE_BUILD_ID
#define BUILD_ID_FTM " %20phN"
#define BUILD_ID_VAL vmlinux_build_id
#else
#define BUILD_ID_FTM "%s"
#define BUILD_ID_VAL ""
#endif

	printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s" BUILD_ID_FTM "\n",
	       log_lvl, raw_smp_processor_id(), current->pid, current->comm,
	       kexec_crash_loaded() ? "Kdump: loaded " : "",
	       print_tainted(),
	       init_utsname()->release,
	       (int)strcspn(init_utsname()->version, " "),
	       init_utsname()->version,
	       BUILD_ID_VAL);


Best Regards,
Petr