Instead of full GNU diff (which smaller boot environments may not have),
use "comm" which is more available.
Reported-by: Naresh Kamboju <[email protected]>
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 <[email protected]>
---
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 <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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 <[email protected]> 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
Kees Cook <[email protected]> writes:
> On Sat, Jun 27, 2020 at 09:51:31PM +1000, Michael Ellerman wrote:
>> Kees Cook <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
-- 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 <[email protected]>
> 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 <[email protected]>
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
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 <[email protected]>
>> 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 <[email protected]>
>
> 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/[email protected]/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 <[email protected]>
>> 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 <[email protected]>
>
> 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 <[email protected]>
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 <[email protected]>
> > > 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 <[email protected]>
> >
> > 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/[email protected]/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
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 <[email protected]>
>>> 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 <[email protected]>
>>
>> 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 <[email protected]>
>
Kees,
Just saw your reply. Will wait for v2.
thanks,
-- Shuah