Hi lianbo,
Just some nits.
On Wed, Aug 27, 2025 at 4:10 PM Lianbo Jiang <lijiang@redhat.com> wrote:
>
> Without the patch:
> crash> log
> ...
> [ 2174.308966] Call Trace:
> [ 2174.311693] <TASK>
> [ 2174.314033] dump_stack_lvl+0x5d/0x80
> [ 2174.318125] panic+0x156/0x32a
> [ 2174.321539] _RNvCscb18lrEyTSA_10rust_panic10area_in_hp+0xf7/0x120 [rust_panic]
> [ 2174.329700] ? console_unlock+0x9c/0x140
> [ 2174.334080] ? irq_work_queue+0x2d/0x50
> [ 2174.338352] ? __pfx_init_module+0x10/0x10 [rust_panic]
> [ 2174.344183] _RNvMCscb18lrEyTSA_10rust_panicNtB2_10HelloPanic8step_two+0x20/0xe0 [rust_panic]
> [ 2174.353698] ? _printk+0x6b/0x90
> ...
>
> With the patch:
> crash> log
> ...
> [ 2174.308966] Call Trace:
> [ 2174.311693] <TASK>
> [ 2174.314033] dump_stack_lvl+0x5d/0x80
> [ 2174.318125] panic+0x156/0x32a
> [ 2174.321539] rust_panic::area_in_hp+0xf7/0x120 [rust_panic]
> [ 2174.329700] ? console_unlock+0x9c/0x140
> [ 2174.334080] ? irq_work_queue+0x2d/0x50
> [ 2174.338352] ? __pfx_init_module+0x10/0x10 [rust_panic]
> [ 2174.344183] <rust_panic::HelloPanic>::step_two+0x20/0xe0 [rust_panic]
> [ 2174.353698] ? _printk+0x6b/0x90
> ...
>
> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> ---
> Makefile | 2 +-
> printk.c | 28 +++++++++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index fc1c9588dcfa..b277129f6df2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -421,7 +421,7 @@ kernel.o: ${GENERIC_HFILES} kernel.c
> ${CC} -c ${CRASH_CFLAGS} kernel.c -I${BFD_DIRECTORY} -I${GDB_INCLUDE_DIRECTORY} ${WARNING_OPTIONS} ${WARNING_ERROR}
>
> printk.o: ${GENERIC_HFILES} printk.c
> - ${CC} -c ${CRASH_CFLAGS} printk.c ${WARNING_OPTIONS} ${WARNING_ERROR}
> + ${CC} -c ${CRASH_CFLAGS} printk.c -I${GDB_INCLUDE_DIRECTORY} ${WARNING_OPTIONS} ${WARNING_ERROR}
>
> gdb_interface.o: ${GENERIC_HFILES} gdb_interface.c
> ${CC} -c ${CRASH_CFLAGS} gdb_interface.c ${WARNING_OPTIONS} ${WARNING_ERROR}
> diff --git a/printk.c b/printk.c
> index 95db7e607e4c..6a87a790b892 100644
> --- a/printk.c
> +++ b/printk.c
> @@ -1,5 +1,6 @@
> #include "defs.h"
> #include <ctype.h>
> +#include "demangle.h"
>
> /* convenience struct for passing many values to helper functions */
> struct prb_map {
> @@ -205,10 +206,35 @@ dump_record(struct prb_map *m, unsigned long id, int msg_flags)
> if (*p == '\n')
> fprintf(fp, "\n%s", space(ilen));
> else if (isprint(*p) || isspace(*p))
> - fputc(*p, fp);
> + sprintf(&buf[i], "%c", *p);
> else
> fputc('.', fp);
> }
> + /*
> + * Try to demangle a mangled Rust symbol(calltrace) from log buffer
> + */
> + char *p1 = strstr(buf, "_R");
> + if (!p1)
> + p1 = strstr(buf, "_ZN");
> + char *p2 = strchrnul(buf, '+');
> + if (p1 && p2) {
> + char mangled[BUFSIZE] = {0};
> + char demangled[BUFSIZE] = {0};
> + char *res;
> + size_t slen = p1 - buf;
> +
> + if (slen)
> + memcpy(demangled, buf, slen);
> +
> + memcpy(mangled, p1, p2-p1);
> + res = rust_demangle(mangled, DMGL_RUST);
> + if (res) {
> + sprintf(demangled+slen, "%s%s", res,p2);
The (de)mangled buf size is 1500. First copied slen chars into
demangled, then sprintf res and p2 into demangled buf without length
check.Though this might be a rare case, but I guess a snprintf with a
Theoretically yes, but actually I never saw a very long string in the printk buffer because it would have bad readability.
Anyway, how about the following changes?
diff --git a/printk.c b/printk.c
index 6a87a790b892..c547ea8c3f9f 100644
--- a/printk.c
+++ b/printk.c
@@ -202,6 +202,11 @@ dump_record(struct prb_map *m, unsigned long id, int msg_flags)
text = m->text_data + begin;
+ if (text_len > BUFSIZE) {
+ error(WARNING, "\nThe messages could be truncated!\n");
+ text_len = BUFSIZE;
+ }
+
for (i = 0, p = text; i < text_len; i++, p++) {
if (*p == '\n')
fprintf(fp, "\n%s", space(ilen));
@@ -229,7 +234,7 @@ dump_record(struct prb_map *m, unsigned long id, int msg_flags)
memcpy(mangled, p1, p2-p1);
res = rust_demangle(mangled, DMGL_RUST);
if (res) {
- sprintf(demangled+slen, "%s%s", res,p2);
+ snprintf(demangled+slen, BUFSIZE, "%s%s", res, p2);
fprintf(fp, "%s",demangled);
free(res);
}
Thanks
Lianbo
length check is safer for preventing overflow, since we are within
ringbuffer buf, I'm not sure how much slen it may be.
The rest of the patch are good to me.
Thanks,
Tao Liu
> + fprintf(fp, "%s",demangled);
> + free(res);
> + }
> + } else
> + fprintf(fp, "%s", buf);
>
> if (msg_flags & SHOW_LOG_DICT) {
> text = info + OFFSET(printk_info_dev_info) +
> --
> 2.50.1
> --
> Crash-utility mailing list -- devel@lists.crash-utility.osci.io
> To unsubscribe send an email to devel-leave@lists.crash-utility.osci.io
> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
> Contribution Guidelines: https://github.com/crash-utility/crash/wiki