Hi Kazu
Thanks for your time to review the patches and all the detailed comments, the 3rd patch attached with all the refinement.
By the way, do I need to squash all the 3 patches as one before them get accepted?
Thanks
Hong
From: HAGIO KAZUHITO(?尾 一仁) <k-hagio-ab@xxxxxxx>
Sent: Friday, November 19, 2021 16:21 To: Hong Yang3 杨红 <hong.yang3@xxxxxxx> Cc: Discussion list for crash utility usage, maintenance and development <crash-utility@xxxxxxxxxx> Subject: RE: arm64: Support overflow stack panic 注意:此封邮件来自于公司外部,请注意信息安全!
Attention: This email comes from outside of the company, please pay attention to the information security! Hi Hong, Thanks for the refinement. > + if (symbol_exists("overflow_stack") && > + (sp = per_cpu_symbol_search("overflow_stack")) && > + get_symbol_type("overflow_stack", NULL, req)) { > + if (CRASHDEBUG(1)) { > + fprintf(fp, "overflow_stack: \n"); > + fprintf(fp, " type: %x, %s\n", > + (int)req->typecode, > + (req->typecode == TYPE_CODE_PTR) ? > + "TYPE_CODE_PTR" : "other"); Given that overflow_stack is array, TYPE_CODE_ARRAY would be better. > @@ -2673,6 +2724,12 @@ arm64_back_trace_cmd(struct bt_info *bt) > bt->hp->eip : GET_STACK_ULONG(bt->hp->esp); > stackframe.sp = bt->hp->esp + 8; > bt->flags &= ~BT_REGS_NOT_FOUND; > + } else if (arm64_on_overflow_stack(bt->tc->processor, bt->frameptr)) { > + arm64_set_overflow_stack(bt); > + bt->flags |= BT_OVERFLOW_STACK; I would prefer to place this into the else block below > + stackframe.sp = bt->stkptr; > + stackframe.pc = bt->instptr; > + stackframe.fp = bt->frameptr; then we can remove these three lines. > } else { > if (arm64_on_irq_stack(bt->tc->processor, bt->frameptr)) { > arm64_set_irq_stack(bt); > +static int > +arm64_in_alternate_stack(int cpu, ulong stkptr) > +{ > + struct machine_specific *ms = machdep->machspec; > + > + return arm64_in_alternate_stackv(cpu, stkptr, > + ms->irq_stacks, ms->irq_stack_size); > +} Given that in_alternate_stack() should check if it's in non-process stacks, I think we should check also for overflow_stacks here. So how about making these function like this? arm64_in_alternate_stack arm64_on_irq_stack arm64_on_overflow_stack arm64_on_irq_stack arm64_in_alternate_stackv(irq_stacks) arm64_on_overflow_stack arm64_in_alternate_stackv(overflow_stacks) Thanks, Kazu -----Original Message----- > Hi Kazu > > Here are the latest patches for supporting to run bt command against a core dump with kernel stack overflow > exception for arm64. > > Please help to review and advise if any further change needed. > > Tested bt command with options: > > bt > bt -a > bt -c 3 > > By the way, 'mach' command also updated to show overflow stacks info as same as IRQ stacks. > > Thanks > Hong > ________________________________ > > From: HAGIO KAZUHITO(?尾 一仁) <k-hagio-ab@xxxxxxx> > Sent: Wednesday, November 17, 2021 15:23 > To: Hong Yang3 杨红 <hong.yang3@xxxxxxx> > Cc: Discussion list for crash utility usage, maintenance and development <crash-utility@xxxxxxxxxx> > Subject: RE: arm64: Support overflow stack panic > > 注意:此封邮件来自于公司外部,请注意信息安全! > Attention: This email comes from outside of the company, please pay attention to the information security! > > Hi Hong, > > Thank you for the patch and sending it to this list. > > -----Original Message----- > > Hi Crash > > > > I'll keep refining the patch before it get approved: > > OK, so we will wait for the refined patch. > > Thanks, > Kazu > > > > > > > 1. Fix the error in arm64_overflow_stack_init() which saved the overflow stack address into > > ms->irqstacks[], which would cause bt command crash on other cpus. The normal IRQ stacks should be used > > for bt command for other cpus. > > 2. In addition to unwind on the overflow stack, try to go through the IRQ stack to find more useful > > information > > > > Kernel stack overflow case would be rarely but I'd like to sharp the crash to cover this kind of issue. > > > > Best regards > > Hong > > ________________________________ > > > > From: Hong Yang3 杨红 > > Sent: Tuesday, November 16, 2021 9:55 > > To: crash-utility@xxxxxxxxxx <crash-utility@xxxxxxxxxx> > > Subject: arm64: Support overflow stack panic > > > > Hi All > > > > When I was trying to open a core of an overflow stack panic result, the bt command caused a segment fault, > > after a while I figured out the overflow stack is not supported by crash utility. > > > > This patch is trying to initialize the overflow stack information on startup stage, and the bt command > works > > as expected to dump the correct call trace in the overflow stack, currently it only apply to arm64 target. > > > > I'm not sure if any other sub command also need to be fixed for full support for the overflow stack, please > > advise and I'll try to improve the patch. > > > > Thanks > > Hong YANG |
From 2a5e482ef4592c1328d7ff88f3fa9b3f593d4d09 Mon Sep 17 00:00:00 2001 From: Hong YANG <hong.yang3@xxxxxxx> Date: Fri, 19 Nov 2021 18:27:01 +0800 Subject: [PATCH 3/3] arm64: Refine overflow stack checking funcs Signed-off-by: Hong YANG <hong.yang3@xxxxxxx> --- arm64.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/arm64.c b/arm64.c index 54a1037..af32e2c 100644 --- a/arm64.c +++ b/arm64.c @@ -1742,8 +1742,8 @@ arm64_overflow_stack_init(void) fprintf(fp, "overflow_stack: \n"); fprintf(fp, " type: %x, %s\n", (int)req->typecode, - (req->typecode == TYPE_CODE_PTR) ? - "TYPE_CODE_PTR" : "other"); + (req->typecode == TYPE_CODE_ARRAY) ? + "TYPE_CODE_ARRAY" : "other"); fprintf(fp, " target_typecode: %x, %s\n", (int)req->target_typecode, req->target_typecode == TYPE_CODE_INT ? @@ -2728,16 +2728,13 @@ arm64_back_trace_cmd(struct bt_info *bt) bt->hp->eip : GET_STACK_ULONG(bt->hp->esp); stackframe.sp = bt->hp->esp + 8; bt->flags &= ~BT_REGS_NOT_FOUND; - } else if (arm64_on_overflow_stack(bt->tc->processor, bt->frameptr)) { - arm64_set_overflow_stack(bt); - bt->flags |= BT_OVERFLOW_STACK; - stackframe.sp = bt->stkptr; - stackframe.pc = bt->instptr; - stackframe.fp = bt->frameptr; } else { if (arm64_on_irq_stack(bt->tc->processor, bt->frameptr)) { arm64_set_irq_stack(bt); bt->flags |= BT_IRQSTACK; + } else if (arm64_on_overflow_stack(bt->tc->processor, bt->frameptr)) { + arm64_set_overflow_stack(bt); + bt->flags |= BT_OVERFLOW_STACK; } stackframe.sp = bt->stkptr; stackframe.pc = bt->instptr; @@ -3990,16 +3987,10 @@ arm64_on_process_stack(struct bt_info *bt, ulong stkptr) return FALSE; } -static int -arm64_on_irq_stack(int cpu, ulong stkptr) -{ - return arm64_in_alternate_stack(cpu, stkptr); -} - static int arm64_in_alternate_stackv(int cpu, ulong stkptr, ulong *stacks, ulong stack_size) { - if (!stack_size || (cpu >= kt->cpus)) + if ((cpu >= kt->cpus) || (stacks == NULL) || !stack_size) return FALSE; if ((stkptr >= stacks[cpu]) && @@ -4011,6 +4002,13 @@ arm64_in_alternate_stackv(int cpu, ulong stkptr, ulong *stacks, ulong stack_size static int arm64_in_alternate_stack(int cpu, ulong stkptr) +{ + return (arm64_on_irq_stack(cpu, stkptr) || + arm64_on_overflow_stack(cpu, stkptr)); +} + +static int +arm64_on_irq_stack(int cpu, ulong stkptr) { struct machine_specific *ms = machdep->machspec; -- 2.25.1
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility