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@xxxxxxxxxxxxxxxxxxx>
> > ---
> >   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@xxxxxxxx>
>
>
> Here is the fix of test-state.sh:
>
>  From 01ca8fd71fc964b892e54aea198537d007d33b4f Mon Sep 17 00:00:00 2001
> From: Petr Mladek <pmladek@xxxxxxxx>
> 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