Re: [PATCH] selftests: livepatch: Fix it to do root uid check and skip

[Shuah Khan <skhan@linuxfoundation.org>]

On 12/13/19 1:34 AM, Petr Mladek wrote:
> On Thu 2019-12-12 18:56:17, Shuah Khan wrote:
>> livepatch test configures the system and debug environment to run
>> tests. Some of these actions fail without root access and test
>> dumps several permission denied messages before it exits.
>>
>> Fix it to check root uid and exit with skip code instead.
> 
> It works when I run the tests directly, e.g.
> 
> $> cd tools/testing/selftests/livepatch
> $> ./test-livepatch.sh
> 
> But I still get an error from the selftest framework when running
> make run_tests:
> 
> $> make run_tests
> TAP version 13
> 1..5
> # selftests: livepatch: test-livepatch.sh
> /mnt/kernel/linux/tools/testing/selftests/kselftest/runner.sh: line 43: /dev/stdout: Permission denied
> not ok 1 selftests: livepatch: test-livepatch.sh # exit=1
> # selftests: livepatch: test-callbacks.sh
> /mnt/kernel/linux/tools/testing/selftests/kselftest/runner.sh: line 43: /dev/stdout: Permission denied
> not ok 2 selftests: livepatch: test-callbacks.sh # exit=1
> # selftests: livepatch: test-shadow-vars.sh
> /mnt/kernel/linux/tools/testing/selftests/kselftest/runner.sh: line 43: /dev/stdout: Permission denied
> not ok 3 selftests: livepatch: test-shadow-vars.sh # exit=1
> # selftests: livepatch: test-state.sh
> /mnt/kernel/linux/tools/testing/selftests/kselftest/runner.sh: line 43: /dev/stdout: Permission denied
> not ok 4 selftests: livepatch: test-state.sh # exit=1
> # selftests: livepatch: test-ftrace.sh
> /mnt/kernel/linux/tools/testing/selftests/kselftest/runner.sh: line 43: /dev/stdout: Permission denied
> not ok 5 selftests: livepatch: test-ftrace.sh # exit=1
> 
> The same problem is also in linux-next. Is this a know problem, please?
> 
> 

This isn't a known issue.

I am not seeing this problem on 5.5-rc1 and on linux-next with top
commit 32b8acf85223448973ca0bf0ee8149a01410f3a0 (HEAD -> master, tag: 
next-20191213

I am curious what could be diffent in your env. that is causing it.

>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
>> ---
>>   tools/testing/selftests/livepatch/functions.sh | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
>> index 31eb09e38729..014b587692f0 100644
>> --- a/tools/testing/selftests/livepatch/functions.sh
>> +++ b/tools/testing/selftests/livepatch/functions.sh
>> @@ -45,6 +57,7 @@ function pop_config() {
>>   }
>>   
>>   function set_dynamic_debug() {
>> +	is_root
>>           cat <<-EOF > /sys/kernel/debug/dynamic_debug/control
>>   		file kernel/livepatch/* +p
>>   		func klp_try_switch_task -p
> 
> This test is superfluous.

> 
> I guess that it was added because of test-state.sh. But it calls
> set_dynamic_debug() instead of config_setup() by mistake.
> Please, use the patch below instead of the above hunk.
> 
> Otherwise, this patch looks good. Thanks for fixing this.
> Without the hunk above, and with the patch below, feel free to use:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> 
> Here is the fix of test-state.sh:
> 
>  From 01ca8fd71fc964b892e54aea198537d007d33b4f Mon Sep 17 00:00:00 2001
> From: Petr Mladek <pmladek@suse.com>
> Date: Fri, 13 Dec 2019 09:26:45 +0100
> Subject: [PATCH] selftests/livepatch: Use setup_config() also in test-state.sh
> 
> The commit 35c9e74cff4c798d0 ("selftests/livepatch: Make dynamic debug
> setup and restore generic") introduced setup_config() to prepare
> the testing environment. All selftests should call it instead
> of set_dynamic_debug().
> 
> test-state.sh has been developed in parallel and was not converted
> by mistake.
> 

Thanks for suggesting the right fix. I will send v2 with your
suggested -by tag.

thanks,
-- Shuah