2022-09-02 21:03:41

by Song Liu

[permalink] [raw]
Subject: [PATCH v3 0/2] add sysfs entry "patched" for each klp_object

I was debugging an issue that a livepatch appears to be attached, but
actually not. It turns out that there is a mismatch in module name
(abc-xyz vs. abc_xyz), klp_find_object_module failed to find the module.
Add a sysfs entry for each klp_object, so that it is easier to debug
such issues.

Changes v2 => v3:
1. Improve selftest. (Petr Mladek and Joe Lawrence)

Changes v1 => v2:
1. Add selftest. (Petr Mladek)
2. Update documentation. (Petr Mladek)
3. Use sysfs_emit. (Petr Mladek)

Song Liu (2):
livepatch: add sysfs entry "patched" for each klp_object
selftests/livepatch: add sysfs test

.../ABI/testing/sysfs-kernel-livepatch | 8 ++
kernel/livepatch/core.c | 18 ++++
tools/testing/selftests/livepatch/Makefile | 3 +-
.../testing/selftests/livepatch/functions.sh | 34 ++++++++
.../testing/selftests/livepatch/test-sysfs.sh | 86 +++++++++++++++++++
5 files changed, 148 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/livepatch/test-sysfs.sh

--
2.30.2


2022-09-02 21:03:52

by Song Liu

[permalink] [raw]
Subject: [PATCH v3 2/2] selftests/livepatch: add sysfs test

Add a test for livepatch sysfs entries.

Signed-off-by: Song Liu <[email protected]>
---
tools/testing/selftests/livepatch/Makefile | 3 +-
.../testing/selftests/livepatch/functions.sh | 34 ++++++++
.../testing/selftests/livepatch/test-sysfs.sh | 86 +++++++++++++++++++
3 files changed, 122 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/livepatch/test-sysfs.sh

diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
index 1acc9e1fa3fb..02fadc9d55e0 100644
--- a/tools/testing/selftests/livepatch/Makefile
+++ b/tools/testing/selftests/livepatch/Makefile
@@ -6,7 +6,8 @@ TEST_PROGS := \
test-callbacks.sh \
test-shadow-vars.sh \
test-state.sh \
- test-ftrace.sh
+ test-ftrace.sh \
+ test-sysfs.sh

TEST_FILES := settings

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index 9230b869371d..433e23dd9dcb 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -6,6 +6,7 @@

MAX_RETRIES=600
RETRY_INTERVAL=".1" # seconds
+KLP_SYSFS_DIR="/sys/kernel/livepatch"

# Kselftest framework requirement - SKIP code is 4
ksft_skip=4
@@ -308,3 +309,36 @@ function check_result {

cleanup_dmesg_file
}
+
+# check_sysfs_rights(modname, rel_path, expected_rights) - check sysfs
+# path permissions
+# modname - livepatch module creating the sysfs interface
+# rel_path - relative path of the sysfs interface
+# expected_rights - expected access rights
+function check_sysfs_rights() {
+ local mod="$1"; shift
+ local rel_path="$1"; shift
+ local expected_rights="$1"; shift
+
+ local path="$KLP_SYSFS_DIR/$mod/$rel_path"
+ local rights=$(/bin/stat --format '%A' "$path")
+ if test "$rights" != "$expected_rights" ; then
+ die "Unexpected access rights of $path: $expected_rights vs. $rights"
+ fi
+}
+
+# check_sysfs_value(modname, rel_path, expected_value) - check sysfs value
+# modname - livepatch module creating the sysfs interface
+# rel_path - relative path of the sysfs interface
+# expected_value - expected value read from the file
+function check_sysfs_value() {
+ local mod="$1"; shift
+ local rel_path="$1"; shift
+ local expected_value="$1"; shift
+
+ local path="$KLP_SYSFS_DIR/$mod/$rel_path"
+ local value=`cat $path`
+ if test "$value" != "$expected_value" ; then
+ die "Unexpected value in $path: $expected_value vs. $value"
+ fi
+}
diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh
new file mode 100755
index 000000000000..7f76f280189a
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test-sysfs.sh
@@ -0,0 +1,86 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2022 Song Liu <[email protected]>
+
+. $(dirname $0)/functions.sh
+
+MOD_LIVEPATCH=test_klp_livepatch
+
+setup_config
+
+# - load a livepatch and verifies the sysfs entries work as expected
+
+start_test "sysfs test"
+
+load_lp $MOD_LIVEPATCH
+
+check_sysfs_rights "$MOD_LIVEPATCH" "" "drwxr-xr-x"
+check_sysfs_rights "$MOD_LIVEPATCH" "enabled" "-rw-r--r--"
+check_sysfs_value "$MOD_LIVEPATCH" "enabled" "1"
+check_sysfs_rights "$MOD_LIVEPATCH" "force" "--w-------"
+check_sysfs_rights "$MOD_LIVEPATCH" "transition" "-r--r--r--"
+check_sysfs_value "$MOD_LIVEPATCH" "transition" "0"
+check_sysfs_rights "$MOD_LIVEPATCH" "vmlinux/patched" "-r--r--r--"
+check_sysfs_value "$MOD_LIVEPATCH" "vmlinux/patched" "1"
+
+disable_lp $MOD_LIVEPATCH
+
+unload_lp $MOD_LIVEPATCH
+
+check_result "% 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
+% 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"
+
+start_test "sysfs test object/patched"
+
+MOD_LIVEPATCH=test_klp_callbacks_demo
+MOD_TARGET=test_klp_callbacks_mod
+load_lp $MOD_LIVEPATCH
+
+# check the "patch" file changes as target module loads/unloads
+check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
+load_mod $MOD_TARGET
+check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "1"
+unload_mod $MOD_TARGET
+check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
+
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
+
+check_result "% modprobe test_klp_callbacks_demo
+livepatch: enabling patch 'test_klp_callbacks_demo'
+livepatch: 'test_klp_callbacks_demo': initializing patching transition
+test_klp_callbacks_demo: pre_patch_callback: vmlinux
+livepatch: 'test_klp_callbacks_demo': starting patching transition
+livepatch: 'test_klp_callbacks_demo': completing patching transition
+test_klp_callbacks_demo: post_patch_callback: vmlinux
+livepatch: 'test_klp_callbacks_demo': patching complete
+% modprobe test_klp_callbacks_mod
+livepatch: applying patch 'test_klp_callbacks_demo' to loading module 'test_klp_callbacks_mod'
+test_klp_callbacks_demo: pre_patch_callback: test_klp_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
+test_klp_callbacks_demo: post_patch_callback: test_klp_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
+test_klp_callbacks_mod: test_klp_callbacks_mod_init
+% rmmod test_klp_callbacks_mod
+test_klp_callbacks_mod: test_klp_callbacks_mod_exit
+test_klp_callbacks_demo: pre_unpatch_callback: test_klp_callbacks_mod -> [MODULE_STATE_GOING] Going away
+livepatch: reverting patch 'test_klp_callbacks_demo' on unloading module 'test_klp_callbacks_mod'
+test_klp_callbacks_demo: post_unpatch_callback: test_klp_callbacks_mod -> [MODULE_STATE_GOING] Going away
+% echo 0 > /sys/kernel/livepatch/test_klp_callbacks_demo/enabled
+livepatch: 'test_klp_callbacks_demo': initializing unpatching transition
+test_klp_callbacks_demo: pre_unpatch_callback: vmlinux
+livepatch: 'test_klp_callbacks_demo': starting unpatching transition
+livepatch: 'test_klp_callbacks_demo': completing unpatching transition
+test_klp_callbacks_demo: post_unpatch_callback: vmlinux
+livepatch: 'test_klp_callbacks_demo': unpatching complete
+% rmmod test_klp_callbacks_demo"
+
+exit 0
--
2.30.2

2022-09-02 21:12:21

by Song Liu

[permalink] [raw]
Subject: [PATCH v3 1/2] livepatch: add sysfs entry "patched" for each klp_object

Add per klp_object sysfs entry "patched". It makes it easier to debug
typos in the module name.

Reviewed-by: Joe Lawrence <[email protected]>
Signed-off-by: Song Liu <[email protected]>
---
I was debugging an issue that a livepatch appears to be attached, but
actually not. It turns out that there is a mismatch in module name
(abc-xyz vs. abc_xyz), klp_find_object_module failed to find the module.
---
.../ABI/testing/sysfs-kernel-livepatch | 8 ++++++++
kernel/livepatch/core.c | 18 ++++++++++++++++++
2 files changed, 26 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
index bea7bd5a1d5f..ebe39a3cea39 100644
--- a/Documentation/ABI/testing/sysfs-kernel-livepatch
+++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
@@ -55,6 +55,14 @@ Description:
The object directory contains subdirectories for each function
that is patched within the object.

+What: /sys/kernel/livepatch/<patch>/<object>/patched
+Date: August 2022
+KernelVersion: 6.0.0
+Contact: [email protected]
+Description:
+ An attribute which indicates whether the object is currently
+ patched.
+
What: /sys/kernel/livepatch/<patch>/<object>/<function,sympos>
Date: Nov 2014
KernelVersion: 3.19.0
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index bc475e62279d..67eb9f9168f3 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -325,6 +325,7 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
* /sys/kernel/livepatch/<patch>/transition
* /sys/kernel/livepatch/<patch>/force
* /sys/kernel/livepatch/<patch>/<object>
+ * /sys/kernel/livepatch/<patch>/<object>/patched
* /sys/kernel/livepatch/<patch>/<object>/<function,sympos>
*/
static int __klp_disable_patch(struct klp_patch *patch);
@@ -431,6 +432,22 @@ static struct attribute *klp_patch_attrs[] = {
};
ATTRIBUTE_GROUPS(klp_patch);

+static ssize_t patched_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct klp_object *obj;
+
+ obj = container_of(kobj, struct klp_object, kobj);
+ return sysfs_emit(buf, "%d\n", obj->patched);
+}
+
+static struct kobj_attribute patched_kobj_attr = __ATTR_RO(patched);
+static struct attribute *klp_object_attrs[] = {
+ &patched_kobj_attr.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(klp_object);
+
static void klp_free_object_dynamic(struct klp_object *obj)
{
kfree(obj->name);
@@ -576,6 +593,7 @@ static void klp_kobj_release_object(struct kobject *kobj)
static struct kobj_type klp_ktype_object = {
.release = klp_kobj_release_object,
.sysfs_ops = &kobj_sysfs_ops,
+ .default_groups = klp_object_groups,
};

static void klp_kobj_release_func(struct kobject *kobj)
--
2.30.2

2022-09-23 15:46:30

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] add sysfs entry "patched" for each klp_object

On Fri 2022-09-02 13:52:06, Song Liu wrote:
> I was debugging an issue that a livepatch appears to be attached, but
> actually not. It turns out that there is a mismatch in module name
> (abc-xyz vs. abc_xyz), klp_find_object_module failed to find the module.
> Add a sysfs entry for each klp_object, so that it is easier to debug
> such issues.
>
> Changes v2 => v3:
> 1. Improve selftest. (Petr Mladek and Joe Lawrence)
>
> Changes v1 => v2:
> 1. Add selftest. (Petr Mladek)
> 2. Update documentation. (Petr Mladek)
> 3. Use sysfs_emit. (Petr Mladek)
>
> Song Liu (2):
> livepatch: add sysfs entry "patched" for each klp_object
> selftests/livepatch: add sysfs test

For both patches:

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

And I have pushed the patchset into livepatching.git,
for-6.1/sysfs-patched-object.

Best Regards,
Petr

2022-09-23 15:47:46

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] livepatch: add sysfs entry "patched" for each klp_object

On Fri 2022-09-02 13:52:07, Song Liu wrote:
> Add per klp_object sysfs entry "patched". It makes it easier to debug
> typos in the module name.
>
> Reviewed-by: Joe Lawrence <[email protected]>
> Signed-off-by: Song Liu <[email protected]>

Just a note. Signed-off-by of the author is always the first entry.
It is followed by Acked-by, Reviewed-by, Tested-by, ... in the order
in which they arrived.

I have fixed the ordering when pushing the patch.

> --- a/Documentation/ABI/testing/sysfs-kernel-livepatch
> +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
> @@ -55,6 +55,14 @@ Description:
> The object directory contains subdirectories for each function
> that is patched within the object.
>
> +What: /sys/kernel/livepatch/<patch>/<object>/patched
> +Date: August 2022
> +KernelVersion: 6.0.0

Also I have updated the version to 6.1.0. It should be the version
produced by the upcoming merge window.

Best Regards,
Petr