9:32 p.m.
From: lipengfei28 <lipengfei28(a)xiaomi.com>
Hi Tao,
Thanks for your review.
This v3 addresses your comment:
- remove redundant extra_stacks_regs[] allocation in
arm64_switch_stack_from_overflow()
- move variable declarations to block start for C89 style
- add braces to outer if for readability
Thanks,
Pengfei
---
On ARM64, crash's gdb bt relies on feeding a register snapshot (PC/SP/FP,
etc.) into gdb and letting gdb unwind from there. Before this change, the
register snapshot handling across exception frames / stack switching could be
broken in several ways, leading to truncated backtraces, garbage substacks, or
invalid addresses (e.g. -3 / 0xff...fd) showing up in gdb output.
Fixes included in this patch:
Fix out-of-bounds read when populating gdb registers from an exception frame
In arm64_print_exception_frame() the previous code copied
sizeof(struct arm64_pt_regs) bytes from a smaller stackframe-derived
buffer, which could read past valid data and poison the gdb registers.
Replace this with explicit field/register assignment and initialize the
bitmap accordingly.
Improve unwinding across IRQ/overflow stack transitions on newer kernels
When switching stacks (IRQ / overflow), gdb may stop at the trampoline (e.g.
call_on_irq_stack) because the discontinuity prevents it from recovering
the caller frame automatically. For UNW_4_14+, "peek" one frame ahead by
reading the saved FP/PC from the current frame, and register that as the
next gdb substack, so gdb can continue unwinding on the process stack.
Avoid creating invalid/empty substacks
Only add a gdb substack when the recovered PC is non-zero, preventing bogus
threads from being created.
This patch only changes ARM64 unwinding/register setup logic. It does not try
to reformat or merge gdb output.
Tested on: Android Linux 6.x, arm64
Changes in v2:
- Fix duplicate definition of arm64_gdb_add_next_frame_substack()
- Fix indentation issues in arm64_print_exception_frame()
- Minor cleanups based on Tao's review
Changes in v3:
- Remove redundant extra_stacks_regs[] allocation in
arm64_switch_stack_from_overflow(), as arm64_gdb_add_next_frame_substack()
already handles its own allocation internally
Signed-off-by: lipengfei28 <lipengfei28(a)xiaomi.com>
---
arm64.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 87 insertions(+), 17 deletions(-)
diff --git a/arm64.c b/arm64.c
index c125655..d177092 100644
--- a/arm64.c
+++ b/arm64.c
@@ -228,7 +228,7 @@ arm64_get_current_task_reg(int regno, const char *name,
return FALSE;
if (sid && sid <= extra_stacks_idx) {
- ur_bitmap = extra_stacks_regs[extra_stacks_idx - 1];
+ ur_bitmap = extra_stacks_regs[sid - 1];
goto get_sub;
}
@@ -3026,6 +3026,9 @@ static char *arm64_exception_functions[] = {
"do_el0_irq_bp_hardening",
"do_sp_pc_abort",
"handle_bad_stack",
+ "el1h_64_sync",
+ "el1h_64_irq",
+ "el1h_64_error",
NULL
};
@@ -3123,6 +3126,11 @@ arm64_print_stackframe_entry(struct bt_info *bt, int level, struct
arm64_stackfr
fprintf(ofp, "\n");
+ if (STREQ(name, "el1h_64_irq") || STREQ(name, "el1h_64_sync")) {
+ if (arm64_is_kernel_exception_frame(bt, frame->sp))
+ arm64_print_exception_frame(bt, frame->sp, KERNEL_MODE, ofp);
+ }
+
if (bt->flags & BT_LINE_NUMBERS) {
get_line_number(branch_pc, buf, FALSE);
if (strlen(buf))
@@ -3775,11 +3783,13 @@ arm64_back_trace_cmd(struct bt_info *bt)
REG_SEQ(arm64_pt_regs, pc));
SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap,
REG_SEQ(arm64_pt_regs, sp));
- if (!bt->machdep ||
+ if (extra_stacks_regs[extra_stacks_idx]->ur.pc &&
+ (bt->task != tt->panic_task) &&
+ (!bt->machdep ||
(extra_stacks_regs[extra_stacks_idx]->ur.sp !=
((struct user_regs_bitmap_struct *)(bt->machdep))->ur.sp &&
extra_stacks_regs[extra_stacks_idx]->ur.pc !=
- ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc)) {
+ ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc))) {
gdb_add_substack (extra_stacks_idx++);
}
}
@@ -3925,11 +3935,13 @@ arm64_back_trace_cmd_v2(struct bt_info *bt)
REG_SEQ(arm64_pt_regs, pc));
SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap,
REG_SEQ(arm64_pt_regs, sp));
- if (!bt->machdep ||
+ if (extra_stacks_regs[extra_stacks_idx]->ur.pc &&
+ (bt->task != tt->panic_task) &&
+ (!bt->machdep ||
(extra_stacks_regs[extra_stacks_idx]->ur.sp !=
((struct user_regs_bitmap_struct *)(bt->machdep))->ur.sp &&
extra_stacks_regs[extra_stacks_idx]->ur.pc !=
- ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc)) {
+ ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc))) {
gdb_add_substack (extra_stacks_idx++);
}
}
@@ -4232,6 +4244,46 @@ arm64_in_kdump_text_on_irq_stack(struct bt_info *bt)
return FALSE;
}
+static void
+arm64_gdb_add_next_frame_substack(struct bt_info *bt, const struct arm64_stackframe
*frame)
+{
+ struct machine_specific *ms = machdep->machspec;
+ struct user_regs_bitmap_struct *ur_ptr;
+ ulong next_fp, next_pc, next_sp;
+
+ if (!extra_stacks_regs[extra_stacks_idx]) {
+ extra_stacks_regs[extra_stacks_idx] = (struct user_regs_bitmap_struct *)
+ malloc(sizeof(struct user_regs_bitmap_struct));
+ }
+
+ memset(extra_stacks_regs[extra_stacks_idx], 0, sizeof(struct user_regs_bitmap_struct));
+ ur_ptr = extra_stacks_regs[extra_stacks_idx];
+
+ next_fp = GET_STACK_ULONG(frame->fp);
+ next_pc = GET_STACK_ULONG(frame->fp + 8);
+ next_sp = frame->fp + 16;
+
+ if (is_kernel_text(next_pc | ms->CONFIG_ARM64_KERNELPACMASK))
+ next_pc |= ms->CONFIG_ARM64_KERNELPACMASK;
+
+ ur_ptr->ur.pc = next_pc;
+ ur_ptr->ur.sp = next_sp;
+ ur_ptr->ur.regs[29] = next_fp;
+
+ SET_BIT(ur_ptr->bitmap, REG_SEQ(arm64_pt_regs, pc));
+ SET_BIT(ur_ptr->bitmap, REG_SEQ(arm64_pt_regs, sp));
+ SET_BIT(ur_ptr->bitmap, REG_SEQ(arm64_pt_regs, regs[0]) + 29);
+
+ if (ur_ptr->ur.pc &&
+ (!bt->machdep ||
+ (ur_ptr->ur.sp !=
+ ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.sp &&
+ ur_ptr->ur.pc !=
+ ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc))) {
+ gdb_add_substack(extra_stacks_idx++);
+ }
+}
+
static int
arm64_switch_stack(struct bt_info *bt, struct arm64_stackframe *frame, FILE *ofp)
{
@@ -4263,8 +4315,11 @@ arm64_switch_stack(struct bt_info *bt, struct arm64_stackframe
*frame, FILE *ofp
if (frame->fp == 0)
return USER_MODE;
- if (!(machdep->flags & UNW_4_14))
+ if (!(machdep->flags & UNW_4_14)) {
arm64_print_exception_frame(bt, frame->sp, KERNEL_MODE, ofp);
+ } else {
+ arm64_gdb_add_next_frame_substack(bt, frame);
+ }
return KERNEL_MODE;
}
@@ -4300,8 +4355,11 @@ arm64_switch_stack_from_overflow(struct bt_info *bt, struct
arm64_stackframe *fr
if (frame->fp == 0)
return USER_MODE;
- if (!(machdep->flags & UNW_4_14))
+ if (!(machdep->flags & UNW_4_14)) {
arm64_print_exception_frame(bt, frame->sp, KERNEL_MODE, ofp);
+ } else {
+ arm64_gdb_add_next_frame_substack(bt, frame);
+ }
return KERNEL_MODE;
}
@@ -4549,6 +4607,9 @@ arm64_print_exception_frame(struct bt_info *bt, ulong pt_regs, int
mode, FILE *o
(ulong)regs->orig_x0, (ulong)regs->syscallno);
fprintf(ofp, " PSTATE: %08lx\n", (ulong)regs->pstate);
} else if (!(bt->flags & BT_EFRAME_SEARCH)) {
+ struct user_regs_bitmap_struct *ur_ptr;
+ int i;
+
if (!extra_stacks_regs[extra_stacks_idx]) {
extra_stacks_regs[extra_stacks_idx] =
(struct user_regs_bitmap_struct *)
@@ -4556,17 +4617,26 @@ arm64_print_exception_frame(struct bt_info *bt, ulong pt_regs, int
mode, FILE *o
}
memset(extra_stacks_regs[extra_stacks_idx], 0,
sizeof(struct user_regs_bitmap_struct));
- memcpy(&extra_stacks_regs[extra_stacks_idx]->ur, regs,
- sizeof(struct arm64_pt_regs));
- for (int i = 0; i < sizeof(struct arm64_pt_regs)/sizeof(long); i++)
- SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap, i);
- if (!bt->machdep ||
- (extra_stacks_regs[extra_stacks_idx]->ur.sp !=
+
+ ur_ptr = extra_stacks_regs[extra_stacks_idx];
+
+ ur_ptr->ur.pc = regs->pc;
+ ur_ptr->ur.sp = regs->sp;
+ ur_ptr->ur.pstate = regs->pstate;
+ for (i = 0; i < 31; i++)
+ ur_ptr->ur.regs[i] = regs->regs[i];
+
+ for (i = 0; i < 34; i++)
+ SET_BIT(ur_ptr->bitmap, i);
+
+ if (ur_ptr->ur.pc &&
+ (!bt->machdep ||
+ (ur_ptr->ur.sp !=
((struct user_regs_bitmap_struct *)(bt->machdep))->ur.sp &&
- extra_stacks_regs[extra_stacks_idx]->ur.pc !=
- ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc)) {
- gdb_add_substack (extra_stacks_idx++);
- }
+ ur_ptr->ur.pc !=
+ ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc))) {
+ gdb_add_substack(extra_stacks_idx++);
+ }
}
}
--
2.34.1