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&gt;
---
 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

Reply via email to