| Message ID | 202006261358.3E8AA623A9@keescook (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/lkdtm: Use "comm" instead of "diff" for dmesg | expand |
Kees Cook <keescook@chromium.org> writes: > Instead of full GNU diff (which smaller boot environments may not have), > use "comm" which is more available. Although using "comm" requires CONFIG_PRINTK_TIME=y doesn't it? Which is probably fine, but should be mentioned. And I guess for completeness you could add: diff --git a/tools/testing/selftests/lkdtm/config b/tools/testing/selftests/lkdtm/config index d874990e442b..ae88bfb163ff 100644 --- a/tools/testing/selftests/lkdtm/config +++ b/tools/testing/selftests/lkdtm/config @@ -1 +1,2 @@ CONFIG_LKDTM=y +CONFIG_PRINTK_TIME=y cheers > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > Link: https://lore.kernel.org/lkml/CA+G9fYtHP+Gg+BrR_GkBMxu2oOi-_e9pATtpb6TVRswv1G1r1Q@mail.gmail.com > Fixes: f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running tests") > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > tools/testing/selftests/lkdtm/run.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh > index 8383eb89d88a..5fe23009ae13 100755 > --- a/tools/testing/selftests/lkdtm/run.sh > +++ b/tools/testing/selftests/lkdtm/run.sh > @@ -82,7 +82,7 @@ dmesg > "$DMESG" > ($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true > > # Record and dump the results > -dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$DMESG" - > "$LOG" || true > +dmesg | comm -13 "$DMESG" - > "$LOG" || true > > cat "$LOG" > # Check for expected output > -- > 2.25.1 > > > -- > Kees Cook
On Sat, Jun 27, 2020 at 09:51:31PM +1000, Michael Ellerman wrote: > Kees Cook <keescook@chromium.org> writes: > > Instead of full GNU diff (which smaller boot environments may not have), > > use "comm" which is more available. > > Although using "comm" requires CONFIG_PRINTK_TIME=y doesn't it? No, it doesn't seem to. "comm" doesn't carry about the line prefixes. AIUI, the only reason for a mention of "sort" is because of how "comm" does its line pairing. i.e. as soon as it goes out of sync, it starts accounting for the disjunction between files. But that's exactly what we want it doing, and the prefix doesn't matter.
Kees Cook <keescook@chromium.org> writes: > On Sat, Jun 27, 2020 at 09:51:31PM +1000, Michael Ellerman wrote: >> Kees Cook <keescook@chromium.org> writes: >> > Instead of full GNU diff (which smaller boot environments may not have), >> > use "comm" which is more available. >> >> Although using "comm" requires CONFIG_PRINTK_TIME=y doesn't it? > > No, it doesn't seem to. "comm" doesn't carry about the line prefixes. > AIUI, the only reason for a mention of "sort" is because of how "comm" > does its line pairing. i.e. as soon as it goes out of sync, it starts > accounting for the disjunction between files. But that's exactly what we > want it doing, and the prefix doesn't matter. OK, if it works. cheers
On Fri, Jun 26, 2020 at 01:59:43PM -0700, Kees Cook wrote: > Instead of full GNU diff (which smaller boot environments may not have), > use "comm" which is more available. > > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > Link: https://lore.kernel.org/lkml/CA+G9fYtHP+Gg+BrR_GkBMxu2oOi-_e9pATtpb6TVRswv1G1r1Q@mail.gmail.com > Fixes: f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running tests") > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > tools/testing/selftests/lkdtm/run.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh > index 8383eb89d88a..5fe23009ae13 100755 > --- a/tools/testing/selftests/lkdtm/run.sh > +++ b/tools/testing/selftests/lkdtm/run.sh > @@ -82,7 +82,7 @@ dmesg > "$DMESG" > ($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true > > # Record and dump the results > -dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$DMESG" - > "$LOG" || true > +dmesg | comm -13 "$DMESG" - > "$LOG" || true > > cat "$LOG" > # Check for expected output I'm not familiar with running lkdtm tests, but I copied the same fixup for the livepatching selftests and "comm" slides in nicely over there, so, Acked-by: Joe Lawrence <joe.lawrence@redhat.com> -- Joe
On Fri, Jun 26, 2020 at 01:59:43PM -0700, Kees Cook wrote: > Instead of full GNU diff (which smaller boot environments may not have), > use "comm" which is more available. > > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > Link: https://lore.kernel.org/lkml/CA+G9fYtHP+Gg+BrR_GkBMxu2oOi-_e9pATtpb6TVRswv1G1r1Q@mail.gmail.com > Fixes: f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running tests") > Signed-off-by: Kees Cook <keescook@chromium.org> Shuah, this really needs to land to fix lkdtm tests on busybox. Can you add this to -next? (Or is it better to direct this to Greg for the lkdtm tree?) Thanks! -Kees > --- > tools/testing/selftests/lkdtm/run.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh > index 8383eb89d88a..5fe23009ae13 100755 > --- a/tools/testing/selftests/lkdtm/run.sh > +++ b/tools/testing/selftests/lkdtm/run.sh > @@ -82,7 +82,7 @@ dmesg > "$DMESG" > ($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true > > # Record and dump the results > -dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$DMESG" - > "$LOG" || true > +dmesg | comm -13 "$DMESG" - > "$LOG" || true > > cat "$LOG" > # Check for expected output > -- > 2.25.1 > > > -- > Kees Cook
On 9/9/20 3:49 PM, Kees Cook wrote: > > On Fri, Jun 26, 2020 at 01:59:43PM -0700, Kees Cook wrote: >> Instead of full GNU diff (which smaller boot environments may not have), >> use "comm" which is more available. >> >> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> >> Link: https://lore.kernel.org/lkml/CA+G9fYtHP+Gg+BrR_GkBMxu2oOi-_e9pATtpb6TVRswv1G1r1Q@mail.gmail.com >> Fixes: f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running tests") >> Signed-off-by: Kees Cook <keescook@chromium.org> > > Shuah, this really needs to land to fix lkdtm tests on busybox. Can > you add this to -next? (Or is it better to direct this to Greg for the > lkdtm tree?) > > Thanks! > > -Kees > >> --- >> tools/testing/selftests/lkdtm/run.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh >> index 8383eb89d88a..5fe23009ae13 100755 >> --- a/tools/testing/selftests/lkdtm/run.sh >> +++ b/tools/testing/selftests/lkdtm/run.sh >> @@ -82,7 +82,7 @@ dmesg > "$DMESG" >> ($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true >> >> # Record and dump the results >> -dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$DMESG" - > "$LOG" || true >> +dmesg | comm -13 "$DMESG" - > "$LOG" || true >> >> cat "$LOG" >> # Check for expected output >> -- >> 2.25.1 >> >> >> -- >> Kees Cook > Hi Kees, You may want to consider a similar follow up to the one Miroslav made to the livepatching equivalent: https://lore.kernel.org/live-patching/nycvar.YFH.7.76.2008271528000.27422@cbobk.fhfr.pm/T/#m1c17812d2c005dd57e9a299a4a492026a156619e basically 'comm' will complain if two lines from dmesg have the same timestamp prefix and their text portions are not sorted. Regards, -- Joe
On 9/9/20 1:49 PM, Kees Cook wrote: > > On Fri, Jun 26, 2020 at 01:59:43PM -0700, Kees Cook wrote: >> Instead of full GNU diff (which smaller boot environments may not have), >> use "comm" which is more available. >> >> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> >> Link: https://lore.kernel.org/lkml/CA+G9fYtHP+Gg+BrR_GkBMxu2oOi-_e9pATtpb6TVRswv1G1r1Q@mail.gmail.com >> Fixes: f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running tests") >> Signed-off-by: Kees Cook <keescook@chromium.org> > > Shuah, this really needs to land to fix lkdtm tests on busybox. Can > you add this to -next? (Or is it better to direct this to Greg for the > lkdtm tree?) > Kees, Thanks for the ping. I can queue this up in -next Greg, would you like me to take this through selftest tree? In case you want to take this through lkdtm tree: Acked-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah
On Wed, Sep 09, 2020 at 04:29:50PM -0400, Joe Lawrence wrote: > On 9/9/20 3:49 PM, Kees Cook wrote: > > > > On Fri, Jun 26, 2020 at 01:59:43PM -0700, Kees Cook wrote: > > > Instead of full GNU diff (which smaller boot environments may not have), > > > use "comm" which is more available. > > > > > > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > > > Link: https://lore.kernel.org/lkml/CA+G9fYtHP+Gg+BrR_GkBMxu2oOi-_e9pATtpb6TVRswv1G1r1Q@mail.gmail.com > > > Fixes: f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running tests") > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > > Shuah, this really needs to land to fix lkdtm tests on busybox. Can > > you add this to -next? (Or is it better to direct this to Greg for the > > lkdtm tree?) > > > > Thanks! > > > > -Kees > > > > > --- > > > tools/testing/selftests/lkdtm/run.sh | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh > > > index 8383eb89d88a..5fe23009ae13 100755 > > > --- a/tools/testing/selftests/lkdtm/run.sh > > > +++ b/tools/testing/selftests/lkdtm/run.sh > > > @@ -82,7 +82,7 @@ dmesg > "$DMESG" > > > ($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true > > > # Record and dump the results > > > -dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$DMESG" - > "$LOG" || true > > > +dmesg | comm -13 "$DMESG" - > "$LOG" || true > > > cat "$LOG" > > > # Check for expected output > > > -- > > > 2.25.1 > > > > > > > > > -- > > > Kees Cook > > > > Hi Kees, > > You may want to consider a similar follow up to the one Miroslav made to the > livepatching equivalent: > > https://lore.kernel.org/live-patching/nycvar.YFH.7.76.2008271528000.27422@cbobk.fhfr.pm/T/#m1c17812d2c005dd57e9a299a4a492026a156619e > > basically 'comm' will complain if two lines from dmesg have the same > timestamp prefix and their text portions are not sorted. Ah-ha! Thank you. I will send a v2. :)
On 9/9/20 2:49 PM, Shuah Khan wrote: > On 9/9/20 1:49 PM, Kees Cook wrote: >> >> On Fri, Jun 26, 2020 at 01:59:43PM -0700, Kees Cook wrote: >>> Instead of full GNU diff (which smaller boot environments may not have), >>> use "comm" which is more available. >>> >>> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> >>> Link: >>> https://lore.kernel.org/lkml/CA+G9fYtHP+Gg+BrR_GkBMxu2oOi-_e9pATtpb6TVRswv1G1r1Q@mail.gmail.com >>> >>> Fixes: f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running >>> tests") >>> Signed-off-by: Kees Cook <keescook@chromium.org> >> >> Shuah, this really needs to land to fix lkdtm tests on busybox. Can >> you add this to -next? (Or is it better to direct this to Greg for the >> lkdtm tree?) >> > > Kees, Thanks for the ping. I can queue this up in -next > > Greg, would you like me to take this through selftest tree? > > In case you want to take this through lkdtm tree: > > Acked-by: Shuah Khan <skhan@linuxfoundation.org> > Kees, Just saw your reply. Will wait for v2. thanks, -- Shuah
diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh index 8383eb89d88a..5fe23009ae13 100755 --- a/tools/testing/selftests/lkdtm/run.sh +++ b/tools/testing/selftests/lkdtm/run.sh @@ -82,7 +82,7 @@ dmesg > "$DMESG" ($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true # Record and dump the results -dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$DMESG" - > "$LOG" || true +dmesg | comm -13 "$DMESG" - > "$LOG" || true cat "$LOG" # Check for expected output
Instead of full GNU diff (which smaller boot environments may not have), use "comm" which is more available. Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> Link: https://lore.kernel.org/lkml/CA+G9fYtHP+Gg+BrR_GkBMxu2oOi-_e9pATtpb6TVRswv1G1r1Q@mail.gmail.com Fixes: f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running tests") Signed-off-by: Kees Cook <keescook@chromium.org> --- tools/testing/selftests/lkdtm/run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)