[PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Joe Lawrence]

The livepatch selftests compare expected dmesg output to verify kernel
behavior.  They currently filter out "tainting kernel with
TAINT_LIVEPATCH" messages which may be logged when loading livepatch
modules.

Further filter the log to also drop "loading out-of-tree module taints
kernel" messages in case the klp_test modules have been build without
the in-tree module flag.

Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---

Note: I stumbled across this in a testing scenario and thought it might
be generally useful to extend this admittedly fragile mechanism.  Since
there are no related livepatch-core changes, this can go through Shuah's
kselftest tree if she prefers.  -- Joe

 tools/testing/selftests/livepatch/functions.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index 79b0affd21fb..57975c323542 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -221,7 +221,7 @@ function check_result {
 	local expect="$*"
 	local result
 
-	result=$(dmesg | grep -v 'tainting' | grep -e 'livepatch:' -e 'test_klp' | sed 's/^\[[ 0-9.]*\] //')
+	result=$(dmesg | grep -ve '\<taints\>' -ve '\<tainting\>' | grep -e 'livepatch:' -e 'test_klp' | sed 's/^\[[ 0-9.]*\] //')
 
 	if [[ "$expect" == "$result" ]] ; then
 		echo "ok"
-- 
2.21.0

Re: [PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Miroslav Benes]

On Wed, 6 Nov 2019, Joe Lawrence wrote:

> The livepatch selftests compare expected dmesg output to verify kernel
> behavior.  They currently filter out "tainting kernel with
> TAINT_LIVEPATCH" messages which may be logged when loading livepatch
> modules.
> 
> Further filter the log to also drop "loading out-of-tree module taints
> kernel" messages in case the klp_test modules have been build without
> the in-tree module flag.

That is true, but "tainting kernel with TAINT_LIVEPATCH" should be printed 
out even in this case. check_modinfo_livepatch() is called for all modules 
and relies on MODINFO(livepatch, Y).

So either the bug is elsewhere or I need one more cup of tea.

Miroslav
 
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> ---
> 
> Note: I stumbled across this in a testing scenario and thought it might
> be generally useful to extend this admittedly fragile mechanism.  Since
> there are no related livepatch-core changes, this can go through Shuah's
> kselftest tree if she prefers.  -- Joe
> 
>  tools/testing/selftests/livepatch/functions.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
> index 79b0affd21fb..57975c323542 100644
> --- a/tools/testing/selftests/livepatch/functions.sh
> +++ b/tools/testing/selftests/livepatch/functions.sh
> @@ -221,7 +221,7 @@ function check_result {
>  	local expect="$*"
>  	local result
>  
> -	result=$(dmesg | grep -v 'tainting' | grep -e 'livepatch:' -e 'test_klp' | sed 's/^\[[ 0-9.]*\] //')
> +	result=$(dmesg | grep -ve '\<taints\>' -ve '\<tainting\>' | grep -e 'livepatch:' -e 'test_klp' | sed 's/^\[[ 0-9.]*\] //')
>  
>  	if [[ "$expect" == "$result" ]] ; then
>  		echo "ok"
> -- 
> 2.21.0
> 

Re: [PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Joe Lawrence]

On 11/7/19 3:42 AM, Miroslav Benes wrote:
> On Wed, 6 Nov 2019, Joe Lawrence wrote:
> 
>> The livepatch selftests compare expected dmesg output to verify kernel
>> behavior.  They currently filter out "tainting kernel with
>> TAINT_LIVEPATCH" messages which may be logged when loading livepatch
>> modules.
>>
>> Further filter the log to also drop "loading out-of-tree module taints
>> kernel" messages in case the klp_test modules have been build without
>> the in-tree module flag.
> 
> That is true, but "tainting kernel with TAINT_LIVEPATCH" should be printed
> out even in this case. check_modinfo_livepatch() is called for all modules
> and relies on MODINFO(livepatch, Y).
>  > So either the bug is elsewhere or I need one more cup of tea.
 >

I'm only half a cup in this morning myself, but...

In my scenario, I saw in the kernel log:

   % modprobe test_klp_livepatch
   test_klp_livepatch: loading out-of-tree module taints kernel.
   test_klp_livepatch: module verification failed: signature and/or 
required key missing - tainting kernel
   ...

and because check_result() only removes 'tainting' with grep -v, the 
expected log message failed to match with the actual filtered message 
because of the first 'taints' message.

So this change just adds more to the filtered out strings:

   result = dmesg | grep -v <filtered out strings> | grep <interesting 
strings>

BTW, none of the callers of check_result() bother to include either 
taint message since I think they are only ever emitted on the first 
occurance.

-- Joe

Re: [PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Miroslav Benes]

On Thu, 7 Nov 2019, Joe Lawrence wrote:

> On 11/7/19 3:42 AM, Miroslav Benes wrote:
> > On Wed, 6 Nov 2019, Joe Lawrence wrote:
> > 
> >> The livepatch selftests compare expected dmesg output to verify kernel
> >> behavior.  They currently filter out "tainting kernel with
> >> TAINT_LIVEPATCH" messages which may be logged when loading livepatch
> >> modules.
> >>
> >> Further filter the log to also drop "loading out-of-tree module taints
> >> kernel" messages in case the klp_test modules have been build without
> >> the in-tree module flag.
> > 
> > That is true, but "tainting kernel with TAINT_LIVEPATCH" should be printed
> > out even in this case. check_modinfo_livepatch() is called for all modules
> > and relies on MODINFO(livepatch, Y).
> >  > So either the bug is elsewhere or I need one more cup of tea.

Ok, the above is not relevant here.

I'm only wondering about the execution steps, because supporting modules 
lib/livepatch/ should be built as in-tree. No?
 
> I'm only half a cup in this morning myself, but...
> 
> In my scenario, I saw in the kernel log:
> 
>   % modprobe test_klp_livepatch
>   test_klp_livepatch: loading out-of-tree module taints kernel.
>   test_klp_livepatch: module verification failed: signature and/or 
> required key missing - tainting kernel
>   ...

So modprobe here should not print anything like this.

> and because check_result() only removes 'tainting' with grep -v, the expected
> log message failed to match with the actual filtered message because of the
> first 'taints' message.
> 
> So this change just adds more to the filtered out strings:
> 
>   result = dmesg | grep -v <filtered out strings> | grep <interesting 
> strings>

The code is definitely correct. You can add my 

Acked-by: Miroslav Benes <mbenes@suse.cz>

M

Re: [PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Joe Lawrence]

On 11/7/19 9:53 AM, Miroslav Benes wrote:
> On Thu, 7 Nov 2019, Joe Lawrence wrote:
> 
>> On 11/7/19 3:42 AM, Miroslav Benes wrote:
>>> On Wed, 6 Nov 2019, Joe Lawrence wrote:
>>>
>>>> The livepatch selftests compare expected dmesg output to verify kernel
>>>> behavior.  They currently filter out "tainting kernel with
>>>> TAINT_LIVEPATCH" messages which may be logged when loading livepatch
>>>> modules.
>>>>
>>>> Further filter the log to also drop "loading out-of-tree module taints
>>>> kernel" messages in case the klp_test modules have been build without
>>>> the in-tree module flag.
>>>
>>> That is true, but "tainting kernel with TAINT_LIVEPATCH" should be printed
>>> out even in this case. check_modinfo_livepatch() is called for all modules
>>> and relies on MODINFO(livepatch, Y).
>>>   > So either the bug is elsewhere or I need one more cup of tea.
> 
> Ok, the above is not relevant here.
> 
> I'm only wondering about the execution steps, because supporting modules
> lib/livepatch/ should be built as in-tree. No?
>   

Ah, I see.  Well as you noted they are modprobed, so theoretically they 
could come from anywhere OOT, right?

In my test, I had a kernel tree, but only wanted to build the test 
modules.  Once I did a 'make modules SUBDIR=' or 'make M= ...' 
KBUILD_EXTMOD got flipped on and the modules lost in-tree status.  No 
amount of googling could tell me how to build a single in-tree directory 
of modules :(  And then it seemed that opening the tests for OOT modules 
was reasonable anyway.

-- Joe

Re: [PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Miroslav Benes]

On Thu, 7 Nov 2019, Joe Lawrence wrote:

> On 11/7/19 9:53 AM, Miroslav Benes wrote:
> > On Thu, 7 Nov 2019, Joe Lawrence wrote:
> > 
> >> On 11/7/19 3:42 AM, Miroslav Benes wrote:
> >>> On Wed, 6 Nov 2019, Joe Lawrence wrote:
> >>>
> >>>> The livepatch selftests compare expected dmesg output to verify kernel
> >>>> behavior.  They currently filter out "tainting kernel with
> >>>> TAINT_LIVEPATCH" messages which may be logged when loading livepatch
> >>>> modules.
> >>>>
> >>>> Further filter the log to also drop "loading out-of-tree module taints
> >>>> kernel" messages in case the klp_test modules have been build without
> >>>> the in-tree module flag.
> >>>
> >>> That is true, but "tainting kernel with TAINT_LIVEPATCH" should be printed
> >>> out even in this case. check_modinfo_livepatch() is called for all modules
> >>> and relies on MODINFO(livepatch, Y).
> >>>   > So either the bug is elsewhere or I need one more cup of tea.
> > 
> > Ok, the above is not relevant here.
> > 
> > I'm only wondering about the execution steps, because supporting modules
> > lib/livepatch/ should be built as in-tree. No?
> >   
> 
> Ah, I see.  Well as you noted they are modprobed, so theoretically they could
> come from anywhere OOT, right?

Yes.
 
> In my test, I had a kernel tree, but only wanted to build the test modules.
> Once I did a 'make modules SUBDIR=' or 'make M= ...' KBUILD_EXTMOD got flipped
> on and the modules lost in-tree status.  No amount of googling could tell me
> how to build a single in-tree directory of modules :(

"make lib/livepatch/test_klp_livepatch.ko" should do the trick. "make 
lib/livepatch/" only builds the object files and I haven't found a way to 
make it link .ko modules other than specifying them one by one directly.

> And then it seemed that
> opening the tests for OOT modules was reasonable anyway.

That's an interesting idea. If a module is in tree, it is under our 
control. So we know what "testing capabilities" it offers. I guess that 
with OOT testing modules the selftests would have to be smarter.

Miroslav

Re: [PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Joe Lawrence]

On 11/7/19 10:24 AM, Miroslav Benes wrote:
>> In my test, I had a kernel tree, but only wanted to build the test modules.
>> Once I did a 'make modules SUBDIR=' or 'make M= ...' KBUILD_EXTMOD got flipped
>> on and the modules lost in-tree status.  No amount of googling could tell me
>> how to build a single in-tree directory of modules :(
> 
> "make lib/livepatch/test_klp_livepatch.ko" should do the trick. "make
> lib/livepatch/" only builds the object files and I haven't found a way to
> make it link .ko modules other than specifying them one by one directly.
> 

Forgot to mention that this works too, but I was looking to script it 
and not have each .ko hardcoded in a series of make commands.  Anyway, 
it's a strange use-case and it was something I was only cooking up for 
an in-house continuous testing scenario.

>> And then it seemed that
>> opening the tests for OOT modules was reasonable anyway.
> 
> That's an interesting idea. If a module is in tree, it is under our
> control. So we know what "testing capabilities" it offers. I guess that
> with OOT testing modules the selftests would have to be smarter.
> 

It would probably go hand in hand with custom test scripts that would 
understand the OOT module capabilities, I think.  I doubt anyone will 
try it (besides me), but the grep filter was there and it was an easy tweak.

-- Joe

Re: [PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Kamalesh Babulal]

On 11/7/19 3:58 AM, Joe Lawrence wrote:
> The livepatch selftests compare expected dmesg output to verify kernel
> behavior.  They currently filter out "tainting kernel with
> TAINT_LIVEPATCH" messages which may be logged when loading livepatch
> modules.
> 
> Further filter the log to also drop "loading out-of-tree module taints
> kernel" messages in case the klp_test modules have been build without
> the in-tree module flag.
> 
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>

Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>

-- 
Kamalesh

Re: [PATCH] selftests/livepatch: filter 'taints' from dmesg comparison

[Petr Mladek]

On Wed 2019-11-06 17:28:01, Joe Lawrence wrote:
> The livepatch selftests compare expected dmesg output to verify kernel
> behavior.  They currently filter out "tainting kernel with
> TAINT_LIVEPATCH" messages which may be logged when loading livepatch
> modules.
> 
> Further filter the log to also drop "loading out-of-tree module taints
> kernel" messages in case the klp_test modules have been build without
> the in-tree module flag.

It needs a special way how to build the selftest modules. But it is
clearly possible.

> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>

Reviewed-by: Petr Mladek <pmladek@suse.com>

> ---
> 
> Since there are no related livepatch-core changes, this can go
> through Shuah's kselftest tree if she prefers.

Shuah, would you like to take it via kselftest tree, please?

Or I could add it into livepatch tree if you would prefer it.

Best Regards,
Petr