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

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

[Shuah Khan]

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.

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
@@ -7,6 +7,9 @@
 MAX_RETRIES=600
 RETRY_INTERVAL=".1"	# seconds
 
+# Kselftest framework requirement - SKIP code is 4
+ksft_skip=4
+
 # log(msg) - write message to kernel log
 #	msg - insightful words
 function log() {
@@ -18,7 +21,16 @@ function log() {
 function skip() {
 	log "SKIP: $1"
 	echo "SKIP: $1" >&2
-	exit 4
+	exit $ksft_skip
+}
+
+# root test
+function is_root() {
+	uid=$(id -u)
+	if [ $uid -ne 0 ]; then
+		echo "skip all tests: must be run as root" >&2
+		exit $ksft_skip
+	fi
 }
 
 # die(msg) - game over, man
@@ -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
@@ -62,6 +75,7 @@ function set_ftrace_enabled() {
 #		 for verbose livepatching output and turn on
 #		 the ftrace_enabled sysctl.
 function setup_config() {
+	is_root
 	push_config
 	set_dynamic_debug
 	set_ftrace_enabled 1
-- 
2.20.1

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

[Petr Mladek]

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?


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

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 tools/testing/selftests/livepatch/test-state.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/livepatch/test-state.sh b/tools/testing/selftests/livepatch/test-state.sh
index dc2908c22c26..5c80e51fca55 100755
--- a/tools/testing/selftests/livepatch/test-state.sh
+++ b/tools/testing/selftests/livepatch/test-state.sh
@@ -8,7 +8,7 @@ MOD_LIVEPATCH=test_klp_state
 MOD_LIVEPATCH2=test_klp_state2
 MOD_LIVEPATCH3=test_klp_state3
 
-set_dynamic_debug
+setup_config
 
 
 # TEST: Loading and removing a module that modifies the system state
-- 
2.16.4

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

[Shuah Khan]

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

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

[Petr Mladek]

On Fri 2019-12-13 10:52:32, Shuah Khan wrote:
> 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.

I did the test in kvm. I was connected there via ssh as "root".
I tested the normal user with "su - user".

It seems that in this case /dev/stdout still points to a
pseudo-terminal that is accessible only by root:

#> su - user
$> echo hello >/dev/stdout
-bash: /dev/stdout: Permission denied
$> ls -l /dev/stdout
lrwxrwxrwx 1 root root 15 Dec 13 16:08 /dev/stdout -> /proc/self/fd/1
$> ls -l /proc/self/fd/1
lrwx------. 1 user users 64 Dec 16 09:27 /proc/self/fd/1 -> /dev/pts/1
$> ls -l /dev/pts/1
crw--w---- 1 root tty 136, 1 Dec 16 09:27 /dev/pts/1

I do not see this problem when I ssh to the machine as
the normal "user".


Best Regards,
Petr