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

Petr Mladek
2021-04-07T13:43:15Z

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