[PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Kees Cook]

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


-- 
Kees Cook

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Michael Ellerman]

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

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[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

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Michael Ellerman]

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

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Joe Lawrence]

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

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Kees Cook]

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

-- 
Kees Cook

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Joe Lawrence]

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

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Kees Cook]

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

-- 
Kees Cook

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Shuah Khan]

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

Re: [PATCH] selftests/lkdtm: Use "comm" instead of "diff" for dmesg

[Shuah Khan]

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