[Crash-utility] Re: [PATCH v2] arm64: fix gdb register feeding for exception frames and stack switching

From: lipengfei28 <lipengfei28(a)xiaomi.com&gt; Thanks for your review. This v2 addresses your comments: - fix duplicated function definition - fix indentation issue - minor cleanups 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 Signed-off-by: lipengfei28 <lipengfei28(a)xiaomi.com&gt; --- arm64.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 89 insertions(+), 17 deletions(-) diff --git a/arm64.c b/arm64.c index c125655..aadd2af 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; } @@ -4278,6 +4333,11 @@ arm64_switch_stack_from_overflow(struct bt_info *bt, struct arm64_stackframe *fr char buf[BUFSIZE]; struct machine_specific *ms = machdep->machspec; + 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)); + } +

if (bt->flags & BT_FULL) { stacktop = ms->overflow_stacks[bt->tc->processor] + ms->overflow_stack_size; words = (stacktop - bt->bptr) / sizeof(ulong); @@ -4300,8 +4360,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; } @@ -4556,17 +4619,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 != + struct user_regs_bitmap_struct *ur_ptr = extra_stacks_regs[extra_stacks_idx]; + int i; + + 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 -- Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io To unsubscribe send an email to devel-leave(a)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