Re: [Crash-utility] [PATCH v3] Fix "log" command when crash is, started with "--minimal" option

Send Crash-utility mailing list submissions to crash-utility(a)redhat.com To subscribe or unsubscribe via the World Wide Web, visit https://www.redhat.com/mailman/listinfo/crash-utility or, via email, send a message with subject or body 'help' to crash-utility-request(a)redhat.com You can reach the person managing the list at crash-utility-owner(a)redhat.com When replying, please edit your Subject line so it is more specific than "Re: Contents of Crash-utility digest..." Today's Topics: 1. Re: Crash-utility Digest, Vol 179, Issue 4 (David Wysochanski) ---------------------------------------------------------------------- Message: 1 Date: Sat, 15 Aug 2020 04:45:49 -0400 From: David Wysochanski <dwysocha(a)redhat.com&gt; To: HAGIO KAZUHITO(?? ??) <k-hagio-ab(a)nec.com&gt; Cc: lijiang <lijiang(a)redhat.com&gt;, "Discussion list for crash utility usage, maintenance and development" <crash-utility(a)redhat.com&gt; Subject: Re: [Crash-utility] Crash-utility Digest, Vol 179, Issue 4 Message-ID: <CALF+zOkMQoyDn8cP5fBpMrrusvcqytMUW+tzs=npKUWwG=nH1g(a)mail.gmail.com&gt; Content-Type: text/plain; charset="UTF-8" On Thu, Aug 13, 2020 at 8:47 PM HAGIO KAZUHITO(?????) <k-hagio-ab(a)nec.com&gt; wrote: > > -----Original Message----- >> From: crash-utility-bounces(a)redhat.com <crash-utility-bounces(a)redhat.com&gt; On Behalf Of lijiang >> Sent: Friday, August 14, 2020 8:31 AM >> To: David Wysochanski <dwysocha(a)redhat.com&gt; >> Cc: Discussion list for crash utility usage, maintenance and development <crash-utility(a)redhat.com&gt; >> Subject: Re: [Crash-utility] Crash-utility Digest, Vol 179, Issue 4 >> >> ? 2020?08?13? 22:58, David Wysochanski ??: >>> On Thu, Aug 13, 2020 at 9:08 AM lijiang <lijiang(a)redhat.com&gt; wrote: >>>> >>>> ? 2020?08?13? 16:33, David Wysochanski ??: >>>>> Hi Lianbo >>>>> >>>>> On Sat, Aug 8, 2020 at 10:46 PM lijiang <lijiang(a)redhat.com&gt; wrote: >>>>>> >>>>>> ? 2020?08?07? 00:00, crash-utility-request(a)redhat.com ??: >>>>>>> Message: 5 >>>>>>> Date: Thu, 6 Aug 2020 09:30:22 -0400 >>>>>>> From: Dave Wysochanski <dwysocha(a)redhat.com&gt; >>>>>>> To: crash-utility(a)redhat.com >>>>>>> Subject: [Crash-utility] [PATCH v3] Fix "log" command when crash is >>>>>>> started with "--minimal" option >>>>>>> Message-ID: <20200806133022.2127538-1-dwysocha(a)redhat.com&gt; >>>>>>> >>>>>>> Commit c86250bce29f introduced the useful '-T' option to print the >>>>>>> log timestamp in human-readable form. However, this option does >>>>>>> not work when crash is invoked with '--minimal' mode, and if tried, >>>>>>> crash will spin at 100% and continuously crash at a divide by 0 >>>>>>> because machdep->hz == 0. >>>>>>> >>>>>>> Fix this by disallowing this option in minimal mode. In addition, >>>>>>> only calculate the logic to calculate kt->boot_date.tv_sec >>>>>>> when this option is enabled. >>>>>>> >>>>>> Hi, Dave Wysochanski >>>>>> >>>>>> Thank you for the patch. >>>>>> >>>>>>> Fixes: c86250bce29f ("Introduction of the "log -T" option...") >>>>>>> Signed-off-by: Dave Wysochanski <dwysocha(a)redhat.com&gt; >>>>>>> Reviewed-by: Wang Long <w(a)laoqinren.net&gt; >>>>>>> Tested-by: Mathias Krause <minipli(a)grsecurity.net&gt; >>>>>>> --- >>>>>>> kernel.c | 5 ++++- >>>>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/kernel.c b/kernel.c >>>>>>> index 5ed6021..95119f3 100644 >>>>>>> --- a/kernel.c >>>>>>> +++ b/kernel.c >>>>>>> @@ -4939,7 +4939,10 @@ cmd_log(void) >>>>>>> if (argerrs) >>>>>>> cmd_usage(pc->curcmd, SYNOPSIS); >>>>>>> >>>>>>> - if (kt->boot_date.tv_sec == 0) { >>>>>>> + if (msg_flags & SHOW_LOG_CTIME && pc->flags & MINIMAL_MODE) >>>>>>> + error(FATAL, "log: option 'T' not available in minimal mode\n"); >>>>>>> + >>>>>>> + if (msg_flags & SHOW_LOG_CTIME && kt->boot_date.tv_sec == 0) { >>>>>> >>>>>> The above two 'if' statements have the same checking condition, would you mind putting them together >>>>>> as a statement block? E.g: >>>>>> >>>>> Sure I can resubmit a fixup of v4 patch once there are no more changes needed. >>>>> >>>>>> + if (msg_flags & SHOW_LOG_CTIME) { >>>>>> + if (pc->flags & MINIMAL_MODE) { >>>>>> + error(WARNING, "the option '-T' not available in minimal mode\n"); >>>>>> + return; >>>>>> + } >>>>>> + >>>>>> + if (kt->boot_date.tv_sec == 0) { >>>>>> ... >>>>>> + } >>>>>> } >>>>>> >>>>>> In addition, might it be more reasonable to issue a warning instead of a fatal error? >>>>>> >>>>> >>>>> If you use WARNING it will not fix the infinite loop / CPU spin at >>>>> 100%. You have to CTRL-C the crash program to get the prompt back. >>>>> So I do not think this is a good idea. >>>>> >>>> How did you reproduce it? Can you help to confirm if you have applied the correct patch >>>> as below? >>>> >>>> [root@intel-sharkbay-mb-03 crash]# git diff kernel.c >>>> diff --git a/kernel.c b/kernel.c >>>> index 5ed6021..6375b24 100644 >>>> --- a/kernel.c >>>> +++ b/kernel.c >>>> @@ -4939,13 +4939,20 @@ cmd_log(void) >>>> if (argerrs) >>>> cmd_usage(pc->curcmd, SYNOPSIS); >>>> >>>> - if (kt->boot_date.tv_sec == 0) { >>>> - ulonglong uptime_jiffies; >>>> - ulong uptime_sec; >>>> - get_uptime(NULL, &uptime_jiffies); >>>> - uptime_sec = (uptime_jiffies)/(ulonglong)machdep->hz; >>>> - kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec; >>>> - kt->boot_date.tv_nsec = 0; >>>> + if (msg_flags & SHOW_LOG_CTIME) { >>>> + if (pc->flags & MINIMAL_MODE) { >>>> + error(WARNING, "the option '-T' not available in minimal mode\n"); >>>> + return; >>>> + } >>>> + >>>> + if (kt->boot_date.tv_sec == 0) { >>>> + ulonglong uptime_jiffies; >>>> + ulong uptime_sec; >>>> + get_uptime(NULL, &uptime_jiffies); >>>> + uptime_sec = (uptime_jiffies)/(ulonglong)machdep->hz; >>>> + kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec; >>>> + kt->boot_date.tv_nsec = 0; >>>> + } >>>> } >>>> >>>> if (msg_flags & SHOW_LOG_AUDIT) { >>>> >>>> >>>> I didn't see any problems, it's strange, this is my test steps. >>>> >>> >>> You are right - I missed the 'return;' in your patch. The WARNING is fine. >>> >> Thanks for your confirmation. >> >>> How do you want to handle this? Do you want to take the original header >>> and add your signed-off-by line and commit your patch? Or do you want >>> me to resubmit with review-by or signed-off-by lines? >>> >> No, please do not add my signed-off-by and review-by line. >> >> If you and Kazu have no objection, you could post it again with the above changes. > > No objection. I can ack a new one with the above change. > I would suggest taking the v3 patch as is because Lianbo has said not to add his signed-off-by line because I did not write that portion. I'm not going to modify something written by someone else and omit where it came from.

Thanks! > Thanks, > Kazu > >> Otherwise Kazu can help to merge your last patch, because it can also work. >> >> Thanks. >> Lianbo >> >> -- >> Crash-utility mailing list >> Crash-utility(a)redhat.com >> https://www.redhat.com/mailman/listinfo/crash-utility ------------------------------ -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility End of Crash-utility Digest, Vol 179, Issue 19 **********************************************