On Mon, Sep 8, 2025 at 11:23 AM Tao Liu <l...@redhat.com> wrote:
> On Mon, Sep 8, 2025 at 2:55 PM lijiang <liji...@redhat.com> wrote:
> >
> > Hi, Tao
> > Thank you for the review.
> >
> > On Thu, Sep 4, 2025 at 1:08 PM Tao Liu <l...@redhat.com> wrote:
> >>
> >> Hi lianbo,
> >>
> >> Just some nits.
> >>
> >> On Wed, Aug 27, 2025 at 4:10 PM Lianbo Jiang <liji...@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 <liji...@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;
> > + }
> > +
>
> OK.
>
> > 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);
>
> I think it should be:
>
> snprintf(demangled+slen, BUFSIZE - slen, "%s%s", res, p2);
>
> Because demangled[BUFSIZE], with slen offset from demangled, the left
> space is BUFSIZE - slen.
>
Totally agree.
Lianbo
>
> > 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-le...@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
> >>
>
>
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@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
