Hi Lianbo, > Hi, sending a patch as attachment is bad, you can refer to the kernel doc:
Thank to Dave for pointing that out. I apologize for not realizing this before, and resend it as following. > I think you are right, the else branch can work well regardless of whether vmcoreinfo is present or not. So I changed the execution order of the code: > Step 1. Try to get THREAD_SHIFT from tbnz instruction. > Step 2. If cannot get THREAD_SHIFT by step 1, change the thread_shift when KASAN is enabled and with vmcoreinfo. > Please help review this new patch. --- arm64.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/arm64.c b/arm64.c index 78e6609..49799e6 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,48 @@ static void arm64_recalc_KERNELPACMASK(void){ } } +static ulong arm64_set_irq_stack_size(void) +{ + int min_thread_shift = 14; + ulong thread_shift = 0; + char buf1[BUFSIZE]; + char *pos1, *pos2; + int errflag = 0; + + 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; + for(pos1=pos2; *pos2!='\0' && *pos2!=','; pos2++); + *pos2 = '\0'; + thread_shift = stol(pos1, RETURN_ON_ERROR|QUIET, &errflag); + if (errflag) { + thread_shift = 0; + } + break; + } + } + } +out: + close_tmpfile(); + + if ((!thread_shift) && kernel_symbol_exists("kasan_enable_current")) { + min_thread_shift += 1; + thread_shift = (min_thread_shift < machdep->pageshift) ? machdep->pageshift : min_thread_shift; + } + + if (!thread_shift) + return 0; + else + return ((1UL) << thread_shift); +} + #endif /* ARM64 */ Thanks yeping. Dave Young <dyo...@redhat.com> 于2024年7月31日周三 14:58写道: > > Please help review this new patch. > > Hi, sending a patch as attachment is bad, you can refer to the kernel doc: > https://www.kernel.org/doc/html/v4.14/process/email-clients.html > > "Patches for the Linux kernel are submitted via email, preferably as > inline text in the body of the email. Some maintainers accept > attachments, but then the attachments should have content-type > text/plain. However, attachments are generally frowned upon because it > makes quoting portions of the patch more difficult in the patch review > process." > > Crash is different from kernel but the email process is similar, I > think you can resend it with a proper format instead of maintainers > manually copying them in the email body for review. > > Thanks > Dave > >
From 2e4d11da6db27952a5bcfd7e3475bd42d230f6b8 Mon Sep 17 00:00:00 2001 From: "yeping.zheng" <yeping.zh...@nio.com> Date: Tue, 30 Jul 2024 14:16:54 +0800 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 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> 2. If cannot get THREAD_SHIFT by step 1, change the thread_shift when KASAN is enabled and with vmcoreinfo. In arm64/include/asm/memory.h: #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) ... #define IRQ_STACK_SIZE THREAD_SIZE Since enabling the KASAN will affect the final value, this patch reset IRQ_STACK_SIZE according to the calculation process in kernel code. Signed-off-by: yeping.zheng <yeping.zheng(a)nio.com> --- arm64.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/arm64.c b/arm64.c index 78e6609..49799e6 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,48 @@ static void arm64_recalc_KERNELPACMASK(void){ } } +static ulong arm64_set_irq_stack_size(void) +{ + int min_thread_shift = 14; + ulong thread_shift = 0; + char buf1[BUFSIZE]; + char *pos1, *pos2; + int errflag = 0; + + 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; + for(pos1=pos2; *pos2!='\0' && *pos2!=','; pos2++); + *pos2 = '\0'; + thread_shift = stol(pos1, RETURN_ON_ERROR|QUIET, &errflag); + if (errflag) { + thread_shift = 0; + } + break; + } + } + } +out: + close_tmpfile(); + + if ((!thread_shift) && kernel_symbol_exists("kasan_enable_current")) { + min_thread_shift += 1; + thread_shift = (min_thread_shift < machdep->pageshift) ? machdep->pageshift : min_thread_shift; + } + + if (!thread_shift) + return 0; + else + return ((1UL) << thread_shift); +} + #endif /* ARM64 */ -- 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