-----Original Message----- 在 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: + 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?

Thanks. Lianbo > ulonglong uptime_jiffies; > ulong uptime_sec; > get_uptime(NULL, &uptime_jiffies); > -- 2.27.0 -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility

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.

This is easily reproducible on any vmcore you can see for yourself if you want. But here is the backtrace in GDB: crash> log -T [Detaching after fork from child process 2669642] log: WARNING: log: option 'T' not available in minimal mode Program received signal SIGFPE, Arithmetic exception. 0x00000000004e5ecf in cmd_log () at kernel.c:4950 4950 kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec; Missing separate debuginfos, use: yum debuginfo-install zlib-1.2.11-13.el8.x86_64 (gdb) bt #0 0x00000000004e5ecf in cmd_log () at kernel.c:4950 #1 0x0000000000463ea9 in exec_command () at main.c:880 #2 0x00000000004640da in main_loop () at main.c:827 #3 0x00000000006c0ce3 in captured_command_loop (data=<optimized out>) at main.c:258 #4 0x00000000006bf5ce in catch_errors (func=0x6c0cd0 <captured_command_loop>, func_args=0x0, errstring=0x91b926 "", mask=6) at exceptions.c:557 #5 0x00000000006c1bb6 in captured_main (data=<optimized out>) at main.c:1064 #6 0x00000000006bf5ce in catch_errors (func=0x6c0df0 <captured_main>, func_args=0x7fffffffe260, errstring=0x91b926 "", mask=6) at exceptions.c:557 #7 0x00000000006c0ae4 in gdb_main (args=<optimized out>) at main.c:1079 #8 0x00000000006c0b1e in gdb_main_entry (argc=<optimized out>, argv=<optimized out>, argv@entry=0x7fffffffe3c8) at main.c:1099 #9 0x00000000004f86d4 in gdb_main_loop (argc=<optimized out>, argc@entry=4, argv=argv@entry=0x7fffffffe3c8) at gdb_interface.c:76 #10 0x0000000000462707 in main (argc=4, argv=0x7fffffffe3c8) at main.c:708 (gdb) c Continuing. Program received signal SIGFPE, Arithmetic exception. 0x00000000004e5ecf in cmd_log () at kernel.c:4950 4950 kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec; (gdb) bt #0 0x00000000004e5ecf in cmd_log () at kernel.c:4950 #1 0x0000000000463ea9 in exec_command () at main.c:880 #2 0x00000000004640da in main_loop () at main.c:827 #3 0x00000000006c0ce3 in captured_command_loop (data=<optimized out>) at main.c:258 #4 0x00000000006bf5ce in catch_errors (func=0x6c0cd0 <captured_command_loop>, func_args=0x0, errstring=0x91b926 "", mask=6) at exceptions.c:557 #5 0x00000000006c1bb6 in captured_main (data=<optimized out>) at main.c:1064 #6 0x00000000006bf5ce in catch_errors (func=0x6c0df0 <captured_main>, func_args=0x7fffffffe260, errstring=0x91b926 "", mask=6) at exceptions.c:557 #7 0x00000000006c0ae4 in gdb_main (args=<optimized out>) at main.c:1079 #8 0x00000000006c0b1e in gdb_main_entry (argc=<optimized out>, argv=<optimized out>, argv@entry=0x7fffffffe3c8) at main.c:1099 #9 0x00000000004f86d4 in gdb_main_loop (argc=<optimized out>, argc@entry=4, argv=argv@entry=0x7fffffffe3c8) at gdb_interface.c:76 #10 0x0000000000462707 in main (argc=4, argv=0x7fffffffe3c8) at main.c:708 (gdb) quit A debugging session is active. Inferior 1 [process 2669636] will be killed. Quit anyway? (y or n) y

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.

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?

-----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.

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