Re: arm64: Support overflow stack panic

[Hong YANG3 杨红 <hong.yang3@xxxxxxx>]

Hi Lianbo, Kazu

Thanks for your great help to review and merge the patch, it looks far better than the original version.

Happy hacking!

Best regards

Hong

---

From: lijiang <lijiang@xxxxxxxxxx>
Sent: Wednesday, December 1, 2021 20:06
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!

On Mon, Nov 29, 2021 at 6:58 PM Hong YANG3 杨红 <hong.yang3@xxxxxxx> wrote:
>
> Hi Lianbo
>
> Please help to review the new patch v7 with only one change for removing the redundant code.
>

Thank you for the update, Hong. Applied with the patch log modifications.
https://github.com/crash-utility/crash/commit/995db8ab88916b6397676b67be98c0a4f82cca49

> Patch is patch, what's  the better way attach it into an email? Copy & paste would not applicable for a large patch file.
>
I usually use the command "git send-email --to xxx xxx" to post a
patch with plain text mode.

> I have no vmcore file, but there is a kernel module which would help to trigger an overflow stack panic for testing, please download the module form link [1] and compile it as a module to load it into your test box, please read the README.txt and the source code for more details.
>
> [1] https://github.com/yangh/crash/tree/pso-kernel-module/kernel-modules/panic-stack-overflow
>

Thank you for sharing this. I have reproduced it with your demo.

Thanks.
Lianbo

> Best regards
> Hong
> ________________________________
> From: Hong YANG3 杨红 <hong.yang3@xxxxxxx>
> Sent: Monday, November 29, 2021 11:40
> To: lijiang <lijiang@xxxxxxxxxx>; Discussion list for crash utility usage, maintenance and development <crash-utility@xxxxxxxxxx>
> Subject: Re: arm64: Support overflow stack panic
>
> Hi Lianbo
>
> I'm using outlook to send mail to this list, I'll try to find a better way to send out patch and mails more friendly for all reader, .
>
> I'll send out a demo kernel module which can trigger an overflow panic for testing, and also the patch will be updated as your comment in previous mail.
>
> Thanks for your quickly reply.
>
> Best regards
> Hong
> ________________________________
> From: lijiang <lijiang@xxxxxxxxxx>
> Sent: Monday, November 29, 2021 10:58
> To: Hong YANG3 杨红 <hong.yang3@xxxxxxx>; 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.  I added the comments below, other changes look good to me.
>
> @@ -1978,7 +2028,10 @@ arm64_in_exception_text(ulong ptr)
>                 if ((ptr >= ms->__exception_text_start) &&
>                     (ptr < ms->__exception_text_end))
>                         return TRUE;
> -       } else if ((name = closest_symbol(ptr))) {  /* Linux 5.5 and later */
> +       }
> +
> +       name = closest_symbol(ptr);
> +       if (name != NULL) { /* Linux 5.5 and later */
>
> The above changes are irrelevant to your patch itself. But anyway this looks more readable to me.
>
>                 for (func = &arm64_exception_functions[0]; *func; func++) {
>                         if (STREQ(name, *func))
>                                 return TRUE;
> @@ -2255,12 +2308,14 @@ arm64_unwind_frame(struct bt_info *bt, struct arm64_stackframe *frame)
>         if (!(machdep->flags & IRQ_STACKS))
>                 return TRUE;
>
> -       if (!(machdep->flags & IRQ_STACKS))
> +       if (!(machdep->flags & OVERFLOW_STACKS))
>                 return TRUE;
>
> Originally, it had two same(repeated) statements, one of which must be redundant.  This time, can it be changed to a statement as below?
>
> if (!(machdep->flags & (IRQ_STACKS | OVERFLOW_STACKS)))
>         return TRUE;
>
> BTW:  this patch was sent as an attachment, which is inconvenient for other reviewers to add comments.
>
> In addition, I  have a request: can you share the vmcore with me if it doesn't have confidential data? I'm collecting the specific vmcore
> for the test, at least I haven't reproduced it.
>
> Thanks.
> Lianbo

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility