2019-10-16 15:04:57

by Miroslav Benes

[permalink] [raw]
Subject: [PATCH v3 0/3] ftrace: Introduce PERMANENT ftrace_ops flag

Livepatch uses ftrace for redirection to new patched functions. It means
that if ftrace is disabled, all live patched functions are disabled as
well. Toggling global 'ftrace_enabled' sysctl thus affect it directly.
It is not a problem per se, because only administrator can set sysctl
values, but it still may be surprising.

Introduce PERMANENT ftrace_ops flag to amend this. If the
FTRACE_OPS_FL_PERMANENT is set on any ftrace ops, the tracing cannot be
disabled by disabling ftrace_enabled. Equally, a callback with the flag
set cannot be registered if ftrace_enabled is disabled.

v2->v3:
- ftrace_enabled explicitly set to true
- selftest from Joe Lawrence (I just split it to two patches)
- typo fix

v1->v2:
- different logic, proposed by Joe Lawrence

Joe Lawrence (2):
selftests/livepatch: Make dynamic debug setup and restore generic
selftests/livepatch: Test interaction with ftrace_enabled

Miroslav Benes (1):
ftrace: Introduce PERMANENT ftrace_ops flag

Documentation/trace/ftrace-uses.rst | 8 +++
Documentation/trace/ftrace.rst | 4 +-
include/linux/ftrace.h | 3 +
kernel/livepatch/patch.c | 3 +-
kernel/trace/ftrace.c | 23 ++++++-
tools/testing/selftests/livepatch/Makefile | 3 +-
.../testing/selftests/livepatch/functions.sh | 34 +++++++---
.../selftests/livepatch/test-callbacks.sh | 2 +-
.../selftests/livepatch/test-ftrace.sh | 65 +++++++++++++++++++
.../selftests/livepatch/test-livepatch.sh | 2 +-
.../selftests/livepatch/test-shadow-vars.sh | 2 +-
11 files changed, 132 insertions(+), 17 deletions(-)
create mode 100755 tools/testing/selftests/livepatch/test-ftrace.sh

--
2.23.0


2019-10-16 15:06:26

by Miroslav Benes

[permalink] [raw]
Subject: [PATCH v3 2/3] selftests/livepatch: Make dynamic debug setup and restore generic

From: Joe Lawrence <[email protected]>

Livepatch selftests currently save the current dynamic debug config and
tweak it for the selftests. The config is restored at the end. Make the
infrastructure generic, so that more variables can be saved and
restored.

Signed-off-by: Joe Lawrence <[email protected]>
Signed-off-by: Miroslav Benes <[email protected]>
---
.../testing/selftests/livepatch/functions.sh | 22 +++++++++++--------
.../selftests/livepatch/test-callbacks.sh | 2 +-
.../selftests/livepatch/test-livepatch.sh | 2 +-
.../selftests/livepatch/test-shadow-vars.sh | 2 +-
4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index 79b0affd21fb..b7e5a67ae434 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -29,29 +29,33 @@ function die() {
exit 1
}

-function push_dynamic_debug() {
- DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \
- awk -F'[: ]' '{print "file " $1 " line " $2 " " $4}')
+function push_config() {
+ DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \
+ awk -F'[: ]' '{print "file " $1 " line " $2 " " $4}')
}

-function pop_dynamic_debug() {
+function pop_config() {
if [[ -n "$DYNAMIC_DEBUG" ]]; then
echo -n "$DYNAMIC_DEBUG" > /sys/kernel/debug/dynamic_debug/control
fi
}

-# set_dynamic_debug() - save the current dynamic debug config and tweak
-# it for the self-tests. Set a script exit trap
-# that restores the original config.
function set_dynamic_debug() {
- push_dynamic_debug
- trap pop_dynamic_debug EXIT INT TERM HUP
cat <<-EOF > /sys/kernel/debug/dynamic_debug/control
file kernel/livepatch/* +p
func klp_try_switch_task -p
EOF
}

+# setup_config - save the current config and set a script exit trap that
+# restores the original config. Setup the dynamic debug
+# for verbose livepatching output.
+function setup_config() {
+ push_config
+ set_dynamic_debug
+ trap pop_config EXIT INT TERM HUP
+}
+
# loop_until(cmd) - loop a command until it is successful or $MAX_RETRIES,
# sleep $RETRY_INTERVAL between attempts
# cmd - command and its arguments to run
diff --git a/tools/testing/selftests/livepatch/test-callbacks.sh b/tools/testing/selftests/livepatch/test-callbacks.sh
index e97a9dcb73c7..a35289b13c9c 100755
--- a/tools/testing/selftests/livepatch/test-callbacks.sh
+++ b/tools/testing/selftests/livepatch/test-callbacks.sh
@@ -9,7 +9,7 @@ MOD_LIVEPATCH2=test_klp_callbacks_demo2
MOD_TARGET=test_klp_callbacks_mod
MOD_TARGET_BUSY=test_klp_callbacks_busy

-set_dynamic_debug
+setup_config


# TEST: target module before livepatch
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index f05268aea859..493e3df415a1 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -7,7 +7,7 @@
MOD_LIVEPATCH=test_klp_livepatch
MOD_REPLACE=test_klp_atomic_replace

-set_dynamic_debug
+setup_config


# TEST: basic function patching
diff --git a/tools/testing/selftests/livepatch/test-shadow-vars.sh b/tools/testing/selftests/livepatch/test-shadow-vars.sh
index 04a37831e204..1aae73299114 100755
--- a/tools/testing/selftests/livepatch/test-shadow-vars.sh
+++ b/tools/testing/selftests/livepatch/test-shadow-vars.sh
@@ -6,7 +6,7 @@

MOD_TEST=test_klp_shadow_vars

-set_dynamic_debug
+setup_config


# TEST: basic shadow variable API
--
2.23.0

2019-10-16 15:06:33

by Miroslav Benes

[permalink] [raw]
Subject: [PATCH v3 3/3] selftests/livepatch: Test interaction with ftrace_enabled

From: Joe Lawrence <[email protected]>

Since livepatching depends upon ftrace handlers to implement "patched"
code functionality, verify that the ftrace_enabled sysctl value
interacts with livepatch registration as expected. At the same time,
ensure that ftrace_enabled is set and part of the test environment
configuration that is saved and restored when running the selftests.

Signed-off-by: Joe Lawrence <[email protected]>
Signed-off-by: Miroslav Benes <[email protected]>
---
tools/testing/selftests/livepatch/Makefile | 3 +-
.../testing/selftests/livepatch/functions.sh | 14 +++-
.../selftests/livepatch/test-ftrace.sh | 65 +++++++++++++++++++
3 files changed, 80 insertions(+), 2 deletions(-)
create mode 100755 tools/testing/selftests/livepatch/test-ftrace.sh

diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
index fd405402c3ff..1886d9d94b88 100644
--- a/tools/testing/selftests/livepatch/Makefile
+++ b/tools/testing/selftests/livepatch/Makefile
@@ -4,6 +4,7 @@ TEST_PROGS_EXTENDED := functions.sh
TEST_PROGS := \
test-livepatch.sh \
test-callbacks.sh \
- test-shadow-vars.sh
+ test-shadow-vars.sh \
+ test-ftrace.sh

include ../lib.mk
diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index b7e5a67ae434..31eb09e38729 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -32,12 +32,16 @@ function die() {
function push_config() {
DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \
awk -F'[: ]' '{print "file " $1 " line " $2 " " $4}')
+ FTRACE_ENABLED=$(sysctl --values kernel.ftrace_enabled)
}

function pop_config() {
if [[ -n "$DYNAMIC_DEBUG" ]]; then
echo -n "$DYNAMIC_DEBUG" > /sys/kernel/debug/dynamic_debug/control
fi
+ if [[ -n "$FTRACE_ENABLED" ]]; then
+ sysctl kernel.ftrace_enabled="$FTRACE_ENABLED" &> /dev/null
+ fi
}

function set_dynamic_debug() {
@@ -47,12 +51,20 @@ function set_dynamic_debug() {
EOF
}

+function set_ftrace_enabled() {
+ local sysctl="$1"
+ result=$(sysctl kernel.ftrace_enabled="$1" 2>&1 | paste --serial --delimiters=' ')
+ echo "livepatch: $result" > /dev/kmsg
+}
+
# setup_config - save the current config and set a script exit trap that
# restores the original config. Setup the dynamic debug
-# for verbose livepatching output.
+# for verbose livepatching output and turn on
+# the ftrace_enabled sysctl.
function setup_config() {
push_config
set_dynamic_debug
+ set_ftrace_enabled 1
trap pop_config EXIT INT TERM HUP
}

diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh
new file mode 100755
index 000000000000..e2a76887f40a
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test-ftrace.sh
@@ -0,0 +1,65 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2019 Joe Lawrence <[email protected]>
+
+. $(dirname $0)/functions.sh
+
+MOD_LIVEPATCH=test_klp_livepatch
+
+setup_config
+
+
+# TEST: livepatch interaction with ftrace_enabled sysctl
+# - turn ftrace_enabled OFF and verify livepatches can't load
+# - turn ftrace_enabled ON and verify livepatch can load
+# - verify that ftrace_enabled can't be turned OFF while a livepatch is loaded
+
+echo -n "TEST: livepatch interaction with ftrace_enabled sysctl ... "
+dmesg -C
+
+set_ftrace_enabled 0
+load_failing_mod $MOD_LIVEPATCH
+
+set_ftrace_enabled 1
+load_lp $MOD_LIVEPATCH
+if [[ "$(cat /proc/cmdline)" != "$MOD_LIVEPATCH: this has been live patched" ]] ; then
+ echo -e "FAIL\n\n"
+ die "livepatch kselftest(s) failed"
+fi
+
+set_ftrace_enabled 0
+if [[ "$(cat /proc/cmdline)" != "$MOD_LIVEPATCH: this has been live patched" ]] ; then
+ echo -e "FAIL\n\n"
+ die "livepatch kselftest(s) failed"
+fi
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
+
+check_result "livepatch: kernel.ftrace_enabled = 0
+% modprobe $MOD_LIVEPATCH
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+livepatch: failed to register ftrace handler for function 'cmdline_proc_show' (-16)
+livepatch: failed to patch object 'vmlinux'
+livepatch: failed to enable patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': canceling patching transition, going to unpatch
+livepatch: '$MOD_LIVEPATCH': completing unpatching transition
+livepatch: '$MOD_LIVEPATCH': unpatching complete
+modprobe: ERROR: could not insert '$MOD_LIVEPATCH': Device or resource busy
+livepatch: kernel.ftrace_enabled = 1
+% modprobe $MOD_LIVEPATCH
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+livepatch: '$MOD_LIVEPATCH': starting patching transition
+livepatch: '$MOD_LIVEPATCH': completing patching transition
+livepatch: '$MOD_LIVEPATCH': patching complete
+livepatch: sysctl: setting key \"kernel.ftrace_enabled\": Device or resource busy kernel.ftrace_enabled = 0
+% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH/enabled
+livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
+livepatch: '$MOD_LIVEPATCH': starting unpatching transition
+livepatch: '$MOD_LIVEPATCH': completing unpatching transition
+livepatch: '$MOD_LIVEPATCH': unpatching complete
+% rmmod $MOD_LIVEPATCH"
+
+
+exit 0
--
2.23.0

2019-10-16 16:39:22

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] ftrace: Introduce PERMANENT ftrace_ops flag

On Wed, 16 Oct 2019 13:33:12 +0200
Miroslav Benes <[email protected]> wrote:

> Livepatch uses ftrace for redirection to new patched functions. It means
> that if ftrace is disabled, all live patched functions are disabled as
> well. Toggling global 'ftrace_enabled' sysctl thus affect it directly.
> It is not a problem per se, because only administrator can set sysctl
> values, but it still may be surprising.
>
> Introduce PERMANENT ftrace_ops flag to amend this. If the
> FTRACE_OPS_FL_PERMANENT is set on any ftrace ops, the tracing cannot be
> disabled by disabling ftrace_enabled. Equally, a callback with the flag
> set cannot be registered if ftrace_enabled is disabled.
>
> v2->v3:
> - ftrace_enabled explicitly set to true
> - selftest from Joe Lawrence (I just split it to two patches)
> - typo fix
>
> v1->v2:
> - different logic, proposed by Joe Lawrence
>
> Joe Lawrence (2):
> selftests/livepatch: Make dynamic debug setup and restore generic
> selftests/livepatch: Test interaction with ftrace_enabled
>
> Miroslav Benes (1):
> ftrace: Introduce PERMANENT ftrace_ops flag
>

Would you like me to take all three patches through my tree?

-- Steve

2019-10-16 16:42:42

by Miroslav Benes

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] ftrace: Introduce PERMANENT ftrace_ops flag

On Wed, 16 Oct 2019, Steven Rostedt wrote:

> On Wed, 16 Oct 2019 13:33:12 +0200
> Miroslav Benes <[email protected]> wrote:
>
> > Livepatch uses ftrace for redirection to new patched functions. It means
> > that if ftrace is disabled, all live patched functions are disabled as
> > well. Toggling global 'ftrace_enabled' sysctl thus affect it directly.
> > It is not a problem per se, because only administrator can set sysctl
> > values, but it still may be surprising.
> >
> > Introduce PERMANENT ftrace_ops flag to amend this. If the
> > FTRACE_OPS_FL_PERMANENT is set on any ftrace ops, the tracing cannot be
> > disabled by disabling ftrace_enabled. Equally, a callback with the flag
> > set cannot be registered if ftrace_enabled is disabled.
> >
> > v2->v3:
> > - ftrace_enabled explicitly set to true
> > - selftest from Joe Lawrence (I just split it to two patches)
> > - typo fix
> >
> > v1->v2:
> > - different logic, proposed by Joe Lawrence
> >
> > Joe Lawrence (2):
> > selftests/livepatch: Make dynamic debug setup and restore generic
> > selftests/livepatch: Test interaction with ftrace_enabled
> >
> > Miroslav Benes (1):
> > ftrace: Introduce PERMANENT ftrace_ops flag
> >
>
> Would you like me to take all three patches through my tree?

I think that would be the easiest, yes.

Thanks
Miroslav

2019-10-16 16:55:01

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] selftests/livepatch: Test interaction with ftrace_enabled

On Wed 2019-10-16 13:33:15, Miroslav Benes wrote:
> From: Joe Lawrence <[email protected]>
>
> Since livepatching depends upon ftrace handlers to implement "patched"
> code functionality, verify that the ftrace_enabled sysctl value
> interacts with livepatch registration as expected. At the same time,
> ensure that ftrace_enabled is set and part of the test environment
> configuration that is saved and restored when running the selftests.
>
> Signed-off-by: Joe Lawrence <[email protected]>
> Signed-off-by: Miroslav Benes <[email protected]>
> ---
> tools/testing/selftests/livepatch/Makefile | 3 +-
> .../testing/selftests/livepatch/functions.sh | 14 +++-
> .../selftests/livepatch/test-ftrace.sh | 65 +++++++++++++++++++
> 3 files changed, 80 insertions(+), 2 deletions(-)
> create mode 100755 tools/testing/selftests/livepatch/test-ftrace.sh
>
> diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
> index fd405402c3ff..1886d9d94b88 100644
> --- a/tools/testing/selftests/livepatch/Makefile
> +++ b/tools/testing/selftests/livepatch/Makefile
> @@ -4,6 +4,7 @@ TEST_PROGS_EXTENDED := functions.sh
> TEST_PROGS := \
> test-livepatch.sh \
> test-callbacks.sh \
> - test-shadow-vars.sh
> + test-shadow-vars.sh \
> + test-ftrace.sh
>
> include ../lib.mk
> diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
> index b7e5a67ae434..31eb09e38729 100644
> --- a/tools/testing/selftests/livepatch/functions.sh
> +++ b/tools/testing/selftests/livepatch/functions.sh
> @@ -32,12 +32,16 @@ function die() {
> function push_config() {
> DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \
> awk -F'[: ]' '{print "file " $1 " line " $2 " " $4}')
> + FTRACE_ENABLED=$(sysctl --values kernel.ftrace_enabled)
> }
>
> function pop_config() {
> if [[ -n "$DYNAMIC_DEBUG" ]]; then
> echo -n "$DYNAMIC_DEBUG" > /sys/kernel/debug/dynamic_debug/control
> fi
> + if [[ -n "$FTRACE_ENABLED" ]]; then
> + sysctl kernel.ftrace_enabled="$FTRACE_ENABLED" &> /dev/null
> + fi
> }
>
> function set_dynamic_debug() {
> @@ -47,12 +51,20 @@ function set_dynamic_debug() {
> EOF
> }
>
> +function set_ftrace_enabled() {
> + local sysctl="$1"

The variable is not later used.

> + result=$(sysctl kernel.ftrace_enabled="$1" 2>&1 | paste --serial --delimiters=' ')
> + echo "livepatch: $result" > /dev/kmsg
> +}

Otherwise, the patch looks good to me. After removing or using the
variable:

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr

2019-10-16 17:01:00

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] selftests/livepatch: Make dynamic debug setup and restore generic

On Wed 2019-10-16 13:33:14, Miroslav Benes wrote:
> From: Joe Lawrence <[email protected]>
>
> Livepatch selftests currently save the current dynamic debug config and
> tweak it for the selftests. The config is restored at the end. Make the
> infrastructure generic, so that more variables can be saved and
> restored.
>
> Signed-off-by: Joe Lawrence <[email protected]>
> Signed-off-by: Miroslav Benes <[email protected]>

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr

2019-10-17 12:43:34

by Kamalesh Babulal

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] selftests/livepatch: Test interaction with ftrace_enabled

On 10/16/19 5:03 PM, Miroslav Benes wrote:
> From: Joe Lawrence <[email protected]>
>
> Since livepatching depends upon ftrace handlers to implement "patched"
> code functionality, verify that the ftrace_enabled sysctl value
> interacts with livepatch registration as expected. At the same time,
> ensure that ftrace_enabled is set and part of the test environment
> configuration that is saved and restored when running the selftests.
>
> Signed-off-by: Joe Lawrence <[email protected]>
> Signed-off-by: Miroslav Benes <[email protected]>

[...]
> diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh
> new file mode 100755
> index 000000000000..e2a76887f40a
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test-ftrace.sh

This test fails due to wrong file permissions, with the warning:

# Warning: file test-ftrace.sh is not executable, correct this.
not ok 4 selftests: livepatch: test-ftrace.sh

--
Kamalesh

2019-10-17 12:45:47

by Kamalesh Babulal

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] selftests/livepatch: Make dynamic debug setup and restore generic

On 10/16/19 5:03 PM, Miroslav Benes wrote:
> From: Joe Lawrence <[email protected]>
>
> Livepatch selftests currently save the current dynamic debug config and
> tweak it for the selftests. The config is restored at the end. Make the
> infrastructure generic, so that more variables can be saved and
> restored.
>
> Signed-off-by: Joe Lawrence <[email protected]>
> Signed-off-by: Miroslav Benes <[email protected]>
> ---
> .../testing/selftests/livepatch/functions.sh | 22 +++++++++++--------
> .../selftests/livepatch/test-callbacks.sh | 2 +-
> .../selftests/livepatch/test-livepatch.sh | 2 +-
> .../selftests/livepatch/test-shadow-vars.sh | 2 +-

A minor nit pick, should the README also updated with the setup_config()?

--
Kamalesh

2019-10-17 14:05:32

by Joe Lawrence

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] selftests/livepatch: Make dynamic debug setup and restore generic

On 10/16/19 1:10 PM, Kamalesh Babulal wrote:
> On 10/16/19 5:03 PM, Miroslav Benes wrote:
>> From: Joe Lawrence <[email protected]>
>>
>> Livepatch selftests currently save the current dynamic debug config and
>> tweak it for the selftests. The config is restored at the end. Make the
>> infrastructure generic, so that more variables can be saved and
>> restored.
>>
>> Signed-off-by: Joe Lawrence <[email protected]>
>> Signed-off-by: Miroslav Benes <[email protected]>
>> ---
>> .../testing/selftests/livepatch/functions.sh | 22 +++++++++++--------
>> .../selftests/livepatch/test-callbacks.sh | 2 +-
>> .../selftests/livepatch/test-livepatch.sh | 2 +-
>> .../selftests/livepatch/test-shadow-vars.sh | 2 +-
>
> A minor nit pick, should the README also updated with the setup_config()?
>

Yup, good eye. I think it should be a simple matter of
s/set_dynamic_debug/setup_config/g

-- Joe

2019-10-17 14:06:05

by Joe Lawrence

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] selftests/livepatch: Test interaction with ftrace_enabled

On 10/16/19 1:06 PM, Kamalesh Babulal wrote:
> On 10/16/19 5:03 PM, Miroslav Benes wrote:
>> From: Joe Lawrence <[email protected]>
>>
>> Since livepatching depends upon ftrace handlers to implement "patched"
>> code functionality, verify that the ftrace_enabled sysctl value
>> interacts with livepatch registration as expected. At the same time,
>> ensure that ftrace_enabled is set and part of the test environment
>> configuration that is saved and restored when running the selftests.
>>
>> Signed-off-by: Joe Lawrence <[email protected]>
>> Signed-off-by: Miroslav Benes <[email protected]>
>
> [...]
>> diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh
>> new file mode 100755
>> index 000000000000..e2a76887f40a
>> --- /dev/null
>> +++ b/tools/testing/selftests/livepatch/test-ftrace.sh
>
> This test fails due to wrong file permissions, with the warning:
>
> # Warning: file test-ftrace.sh is not executable, correct this.
> not ok 4 selftests: livepatch: test-ftrace.sh
>

Hi Kamalesh,

Thanks for testing. This error is weird as the git log says new file
mode: 100755. 'git am' of Miroslav's patchset gives me a new
test-ftrace.sh with "Access: (0775/-rwxrwxr-x)" Did you apply the mbox
directly or.. ?

-- Joe

2019-10-18 10:10:12

by Kamalesh Babulal

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] selftests/livepatch: Test interaction with ftrace_enabled

On 10/17/19 3:17 AM, Joe Lawrence wrote:
> On 10/16/19 1:06 PM, Kamalesh Babulal wrote:
>> On 10/16/19 5:03 PM, Miroslav Benes wrote:
>>> From: Joe Lawrence <[email protected]>
>>>
>>> Since livepatching depends upon ftrace handlers to implement "patched"
>>> code functionality, verify that the ftrace_enabled sysctl value
>>> interacts with livepatch registration as expected.  At the same time,
>>> ensure that ftrace_enabled is set and part of the test environment
>>> configuration that is saved and restored when running the selftests.
>>>
>>> Signed-off-by: Joe Lawrence <[email protected]>
>>> Signed-off-by: Miroslav Benes <[email protected]>
>>
>> [...]
>>> diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh
>>> new file mode 100755
>>> index 000000000000..e2a76887f40a
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/livepatch/test-ftrace.sh
>>
>> This test fails due to wrong file permissions, with the warning:
>>
>> # Warning: file test-ftrace.sh is not executable, correct this.
>> not ok 4 selftests: livepatch: test-ftrace.sh
>>
>
> Hi Kamalesh,
>
> Thanks for testing.  This error is weird as the git log says new file mode: 100755.  'git am' of Miroslav's patchset gives me a new test-ftrace.sh with "Access: (0775/-rwxrwxr-x)"  Did you apply the mbox directly or.. ?
>

Hi Joe,

Thanks, I was using patch command to apply the patches and using git am
helped. Sorry for the noise. The test cases passes now, without the issue
I previously reported:

[...]
# TEST: livepatch interaction with ftrace_enabled sysctl ... ok
ok 4 selftests: livepatch: test-ftrace.sh

Long story is that the patch command version installed on the test machine
doesn't understand new file mode permission from the git diff, updating
the patch version creates the patch with 755 mode.

--
Kamalesh