[Crash-utility] Re: Fix irq_stack_size on ARM64

[yp z <wonderzyp@xxxxxxxxx>]

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 <dyoung@xxxxxxxxxx> 于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.zheng@xxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki