Hi Lianbo and Tao, Thank you for your suggestions, and I have rewrote a patch in two cases:
[1] with vmcoreinfo: + if (kernel_symbol_exists("kasan_enable_current")) { + min_thread_shift += 1; + thread_shift = (min_thread_shift < machdep->pageshift) ? machdep->pageshift : min_thread_shift; + } BTW: can you help point out the current issue is with or without the > vmcoreinfo? Yeping > My issue is with vmcoreinfo of "SYMBOL("kasan_enable_current")", and works well with this patch. [2] without vmcoreinfo: > What about somehow the disassembly gives us hex values like: > #0xe? It won't work by then. Any ideas for this? In order to use stol() as expected, I have changed the first non-numeric char to '\0': + if ((pos1 = strstr(buf1, "tbnz"))) { + if ((pos2 = strchr(pos1, '#'))) { + pos2 += 1; + pos1 = pos2; + while (*pos2 != '\0') { + if (!((*pos2 >= '0' && *pos2 <= '9') + || (*pos2 >= 'A' && *pos2 <= 'F') + || (*pos2 >= 'a' && *pos2 <= 'f'))) { + *pos2 = '\0'; + break; + } + ++pos2; + } + thread_shift = stol(pos1, RETURN_ON_ERROR|QUIET, &errflag); Thanks. Yeping.ZHENG lijiang <liji...@redhat.com> 于2024年7月26日周五 17:23写道: > On Fri, Jul 19, 2024 at 6:40 AM Tao Liu <l...@redhat.com> wrote: > >> Hi Lianbo, >> >> On Thu, Jul 18, 2024 at 10:50 PM Lianbo Jiang <liji...@redhat.com> wrote: >> > >> > On 7/16/24 4:22 PM, Tao Liu wrote: >> > >> > > Hi Yeping, >> > > >> > > Thanks for the fix. >> > > >> > > On Thu, Jul 11, 2024 at 1:38 PM <wonder...@gmail.com> wrote: >> > >> When using the crash tool to parse the ARM64 dump file with KASAN >> enabled, I found that using the bt -a command will cause this tool to >> crash, the following is the backtrace infomation. >> > >> >> > >> (gdb) bt >> > >> #0 0x00005635ac2b166b in arm64_unwind_frame (frame=0x7ffdaf35cb70, >> bt=0x7ffdaf35d430) >> > >> at arm64.c:2821 >> > >> #1 arm64_back_trace_cmd (bt=0x7ffdaf35d430) at arm64.c:3306 >> > >> #2 0x00005635ac27b108 in back_trace (bt=bt@entry=0x7ffdaf35d430) >> at kernel.c:3239 >> > >> #3 0x00005635ac2880ae in cmd_bt () at kernel.c:2863 >> > >> #4 0x00005635ac1f16dc in exec_command () at main.c:893 >> > >> #5 0x00005635ac1f192a in main_loop () at main.c:840 >> > >> #6 0x00005635ac50df81 in captured_main (data=<optimized out>) at >> main.c:1284 >> > >> #7 gdb_main (args=<optimized out>) at main.c:1313 >> > >> #8 0x00005635ac50e000 in gdb_main_entry (argc=<optimized out>, >> argv=<optimized out>) >> > >> at main.c:1338 >> > >> #9 0x00005635ac1ea2a5 in main (argc=5, argv=0x7ffdaf35dde8) at >> main.c:721 >> > >> Eventually, I found that it was may caused by not setting >> irq_stack_size properly, and provide this patch to solve it. >> > >> >> > > Could you please re-draft your commit message? The original one looks >> > > informal. E.g: >> > > >> > > A segfault issue was observed on KASAN enabled arm64 kernel due to the >> > > incorrect irq_stack_size, see the following stack trace: >> > > ... >> > > The issue was caused by ...., and this patch will fix the issue by >> .... >> > > >> > >> From 34b28aa8c11e77d20adec4f7705a14d239c8a55f Mon Sep 17 00:00:00 >> 2001 >> > >> From: wonderzyp <wonder...@qq.com> >> > >> Date: Mon, 8 Jul 2024 20:11:38 +0800 >> > >> Subject: [PATCH 1131/1131] set_arm64_irq_stack_size >> > >> >> > >> Signed-off-by: Yeping Zheng <wonder...@gmail.com> >> > >> --- >> > >> arm64.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- >> > >> 1 file changed, 47 insertions(+), 2 deletions(-) >> > >> >> > >> diff --git a/arm64.c b/arm64.c >> > >> index b3040d7..39d891b 100644 >> > >> --- a/arm64.c >> > >> +++ b/arm64.c >> > >> @@ -93,6 +93,7 @@ static void arm64_calc_VA_BITS(void); >> > >> static int arm64_is_uvaddr(ulong, struct task_context *); >> > >> static void arm64_calc_KERNELPACMASK(void); >> > >> static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char >> *label, int base); >> > >> +static ulong arm64_set_irq_stack_size(struct machine_specific *ms); >> > >> >> > >> struct kernel_range { >> > >> unsigned long modules_vaddr, modules_end; >> > >> @@ -2223,8 +2224,14 @@ arm64_irq_stack_init(void) >> > >> if (MEMBER_EXISTS("thread_union", "stack")) { >> > >> if ((sz = MEMBER_SIZE("thread_union", >> "stack")) > 0) >> > >> ms->irq_stack_size = sz; >> > >> - } else >> > >> - ms->irq_stack_size = ARM64_IRQ_STACK_SIZE; >> > >> + } else { >> > >> + ulong res = arm64_set_irq_stack_size(ms); >> > >> + if (res > 0){ >> > >> + ms->irq_stack_size = res; >> > >> + } else { >> > >> + ms->irq_stack_size = >> ARM64_IRQ_STACK_SIZE; >> > >> + } >> > >> + } >> > >> >> > >> machdep->flags |= IRQ_STACKS; >> > >> >> > >> @@ -4921,6 +4928,44 @@ static void arm64_calc_KERNELPACMASK(void) >> > >> } >> > >> } >> > >> >> > >> +static ulong arm64_set_irq_stack_size(struct machine_specific *ms) >> > >> +{ >> > >> + char *string; >> > >> + int ret; >> > >> + int KASAN_THREAD_SHIFT = 0; >> > >> + int MIN_THREAD_SHIFT; >> > >> + ulong ARM64_PAGE_SHIFT; >> > >> + ulong THREAD_SHIFT = 0; >> > >> + ulong THREAD_SIZE; >> > > I guess the upper case of variable names is not encouraged, though it >> > > is the variable that comes from kernel config file. >> > > >> > >> + if (kt->ikconfig_flags & IKCONFIG_AVAIL) { >> > >> + if ((ret = get_kernel_config("CONFIG_KASAN_GENERIC", >> NULL) == IKCONFIG_Y) || >> > >> + (ret = >> get_kernel_config("CONFIG_KASAN_SW_TAGS", NULL) == IKCONFIG_Y)) { >> > >> + KASAN_THREAD_SHIFT = 1; >> > >> + } >> > >> + } >> > >> + MIN_THREAD_SHIFT = 14 + KASAN_THREAD_SHIFT; >> > >> + >> > >> + if (kt->ikconfig_flags & IKCONFIG_AVAIL) { >> > > Could the if condition be merged with the prior one? >> > > >> > >> + if ((ret = get_kernel_config("CONFIG_VMAP_STACK", >> NULL)) == IKCONFIG_Y){ >> > >> + if ((ret = >> get_kernel_config("CONFIG_ARM64_PAGE_SHIFT", &string)) == >> > >> > The "CONFIG_ARM64_PAGE_SHIFT " has been removed since kernel v6.9-rc1, >> > so this can not work on the latest kernel. See: >> > >> > d3e5bab923d3 ("arch: simplify architecture specific page size >> > configuration") >> > >> > >> > In addition, the IKCONFIG is not available in most distributions, it >> > can not cover this case. There is a similar discussion there: >> > >> > >> https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg00880.html >> > >> > Can you try to handle the current issue with a similar solution? >> > >> > >> > >> IKCONFIG_STR){ >> > >> + ARM64_PAGE_SHIFT = atol(string); >> > >> + } >> > >> + if (MIN_THREAD_SHIFT < ARM64_PAGE_SHIFT){ >> > >> + THREAD_SHIFT = ARM64_PAGE_SHIFT; >> > >> + } else { >> > >> + THREAD_SHIFT = MIN_THREAD_SHIFT; >> > >> + } >> > >> + } >> > >> + } >> > >> + >> > >> + if (THREAD_SHIFT == 0) { >> > >> + return -1; >> > >> + } >> > >> + >> > >> + THREAD_SIZE = ((1UL) << THREAD_SHIFT); >> > >> + return THREAD_SIZE; >> > >> +} >> > > I'm OK with the approach above, since it directly came from the kernel >> > > source. However I'm not a fan of checking kernel configs, there might >> > > be kernels which are compiled without CONFIG_IKCONFIG. >> > > >> > > Could we add an approach here, to get the value from disassembly when >> > > CONFIG_IKCONFIG is negative? >> > > >> > > kernel source: arch/arm64/kernel/entry.S: >> > > >> > > .macro kernel_ventry, el:req, ht:req, regsize:req, label:req >> > > .... >> > > add sp, sp, x0 // sp' = sp + x0 >> > > sub x0, sp, x0 // x0' = sp' - x0 = (sp + x0) - x0 = sp >> > > tbnz x0, #THREAD_SHIFT, 0f <<<<<<<< >> > > >> > > $ objdump -d vmlinux >> > > ... >> > > ffff800080010800 <vectors>: >> > > ffff800080010800: d10543ff sub sp, sp, #0x150 >> > > ffff800080010804: 8b2063ff add sp, sp, x0 >> > > ffff800080010808: cb2063e0 sub x0, sp, x0 >> > > ffff80008001080c: 37800080 tbnz w0, #16, >> > > ffff80008001081c <vectors+0x1c> <<<<<<<<<< >> > > >> > > It is easy to get the THREAD_SHIFT value by disassembling the tbnz >> > > instruction. What do you think @Lianbo Jiang >> > >> > This is a good idea, but it still relies on compiler. >> > >> > As we discussed, usually I would recommend finding its values via some >> > kernel symbols. >> >> Yeah, but I didn't find a suitable symbol so far... In addition, the >> > > It's true. For the KASAN, it is easy to find a suitable symbol, but it's > not easy for the PAGESIZE or PAGESHIFT. > > Given that I would suggest covering two cases: > [1] with vmcoreinfo(see mm/kasan/common.c) > ... > int min_thread_shift = 14; > if (kernel_symbol_exists("kasan_enable_current")) > min_thread_shift += 1; > > thread_shift = (min_thread_shift < machdep->pageshift) > ? machdep->pageshift : min_thread_shift; > ... > > [2] without vmcoreinfo > Use Tao's solution(disassemble kernel code) > > The first one is better than the second one in case of having vmcoreinfo, > If I understand correctly. > But we still need to cover the above two cases in the crash tools. > > BTW: can you help point out the current issue is with or without the > vmcoreinfo? Yeping > > Thanks > Lianbo > > above case is a little different here, the tbnz instruction is not >> generated from C code, it is written in assembly in >> arch/arm64/kernel/entry.S file. So I guess there would be a lower >> possibility if tbnz instruction gets replaced or eliminated by the >> compilers. Anyway, it's OK to me if you don't want to accept the >> approach. >> >> Thanks, >> Tao Liu >> >> >> > >> > >> > Thanks >> > >> > Lianbo >> > >> > > >> > > Thanks, >> > > Tao Liu >> > > >> > >> + >> > >> #endif /* ARM64 */ >> > >> >> > >> >> > >> -- >> > >> 2.25.1 >> > >> >> > >> >>
From b48cd074345673694c4b8bf3a6ad496760ea562c Mon Sep 17 00:00:00 2001 From: "yeping.zheng" <yeping.zh...@nio.com> Date: Mon, 29 Jul 2024 09:27:16 +0800 Subject: [PATCH] Subject: [PATCH] A segfault issue due to the incorrect irq_stack_size on ARM64 See the following stack trace: (gdb) bt #0 0x00005635ac2b166b in arm64_unwind_frame (frame=0x7ffdaf35cb70, bt=0x7ffdaf35d430) at arm64.c:2821 #1 arm64_back_trace_cmd (bt=0x7ffdaf35d430) at arm64.c:3306 #2 0x00005635ac27b108 in back_trace (bt=bt@entry=0x7ffdaf35d430) at kernel.c:3239 #3 0x00005635ac2880ae in cmd_bt () at kernel.c:2863 #4 0x00005635ac1f16dc in exec_command () at main.c:893 #5 0x00005635ac1f192a in main_loop () at main.c:840 #6 0x00005635ac50df81 in captured_main (data=<optimized out>) at main.c:1284 #7 gdb_main (args=<optimized out>) at main.c:1313 #8 0x00005635ac50e000 in gdb_main_entry (argc=<optimized out>, argv=<optimized out>) at main.c:1338 #9 0x00005635ac1ea2a5 in main (argc=5, argv=0x7ffdaf35dde8) at main.c:721 The issue may be encountered when thread_union symbol not found in vmlinux due to compiling optimization. This patch will try the following 2 methods to get the irq_stack_size when thread_union symbol unavailable: 1. Try getting the value from IKconfig. 2. Try getting the value from kernel code disassembly, to get THREAD_SHIFT directly from tbnz instruction. In arch/arm64/kernel/entry.S: .macro kernel_ventry, el:req, ht:req, regsize:req, label:req ... add sp, sp, x0 sub x0, sp, x0 tbnz x0, #THREAD_SHIFT, 0f $ gdb vmlinux (gdb) disass vectors Dump of assembler code for function vectors: ... 0xffff800080010804 <+4>: add sp, sp, x0 0xffff800080010808 <+8>: sub x0, sp, x0 0xffff80008001080c <+12>: tbnz w0, #16, 0xffff80008001081c <vectors+28> Signed-off-by: yeping.zheng <yeping.zheng(a)nio.com> --- arm64.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/arm64.c b/arm64.c index 78e6609..26f4153 100644 --- a/arm64.c +++ b/arm64.c @@ -94,6 +94,7 @@ static int arm64_is_uvaddr(ulong, struct task_context *); static void arm64_calc_KERNELPACMASK(void); static void arm64_recalc_KERNELPACMASK(void); static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char *label, int base); +static ulong arm64_set_irq_stack_size(void); struct kernel_range { unsigned long modules_vaddr, modules_end; @@ -2234,8 +2235,10 @@ arm64_irq_stack_init(void) if (MEMBER_EXISTS("thread_union", "stack")) { if ((sz = MEMBER_SIZE("thread_union", "stack")) > 0) ms->irq_stack_size = sz; - } else - ms->irq_stack_size = ARM64_IRQ_STACK_SIZE; + } else { + ulong res = arm64_set_irq_stack_size(); + ms->irq_stack_size = (res > 0) ? res : ARM64_IRQ_STACK_SIZE; + } machdep->flags |= IRQ_STACKS; @@ -4950,6 +4953,56 @@ static void arm64_recalc_KERNELPACMASK(void){ } } -#endif /* ARM64 */ +static ulong arm64_set_irq_stack_size(void) +{ + char *string; + int min_thread_shift = 14; + ulong thread_shift = 0; + char buf1[BUFSIZE]; + char *pos1, *pos2; + int errflag = 0; + + if (kernel_symbol_exists("kasan_enable_current")) { + min_thread_shift += 1; + thread_shift = (min_thread_shift < machdep->pageshift) ? machdep->pageshift : min_thread_shift; + } else { + sprintf(buf1, "x/32i vectors"); + open_tmpfile(); + if (!gdb_pass_through(buf1, pc->tmpfile, GNU_RETURN_ON_ERROR)) { + goto out; + } + rewind(pc->tmpfile); + while (fgets(buf1, BUFSIZE, pc->tmpfile)) { + if ((pos1 = strstr(buf1, "tbnz"))) { + if ((pos2 = strchr(pos1, '#'))) { + pos2 += 1; + pos1 = pos2; + while (*pos2 != '\0') { + if (!((*pos2 >= '0' && *pos2 <= '9') + || (*pos2 >= 'A' && *pos2 <= 'F') + || (*pos2 >= 'a' && *pos2 <= 'f'))) { + *pos2 = '\0'; + break; + } + ++pos2; + } + thread_shift = stol(pos1, RETURN_ON_ERROR|QUIET, &errflag); + if (errflag) { + thread_shift = 0; + } + break; + } + } + } +out: + close_tmpfile(); + } + + if (!thread_shift) + return 0; + else + return ((1UL) << thread_shift); +} +#endif /* ARM64 */ \ No newline at end of file -- 2.25.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