diff mbox series

[v2,6/6] selftests: livepatch: Test livepatching function using an external symbol

Message ID 20240516133009.20224-7-lhruska@suse.cz (mailing list archive)
State New
Headers show
Series livepatch: klp-convert tool - Minimal version | expand

Commit Message

Lukas Hruska May 16, 2024, 1:30 p.m. UTC
The test proves that klp-convert works as intended and it is possible to
livepatch a function that use an external symbol.

Signed-off-by: Lukas Hruska <lhruska@suse.cz>
---
 .../testing/selftests/livepatch/functions.sh  | 16 +++++-
 .../selftests/livepatch/test-extern.sh        | 57 +++++++++++++++++++
 .../selftests/livepatch/test_modules/Makefile |  2 +
 .../livepatch/test_modules/test_klp_extern.c  | 51 +++++++++++++++++
 .../test_modules/test_klp_extern_hello.c      | 36 ++++++++++++
 5 files changed, 161 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/livepatch/test-extern.sh
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_extern.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_extern_hello.c

Comments

Petr Mladek May 22, 2024, 11:44 a.m. UTC | #1
On Thu 2024-05-16 15:30:09, Lukas Hruska wrote:
> The test proves that klp-convert works as intended and it is possible to
> livepatch a function that use an external symbol.
> 
> Signed-off-by: Lukas Hruska <lhruska@suse.cz>

> --- a/tools/testing/selftests/livepatch/functions.sh
> +++ b/tools/testing/selftests/livepatch/functions.sh
> @@ -7,6 +7,7 @@
>  MAX_RETRIES=600
>  RETRY_INTERVAL=".1"	# seconds
>  KLP_SYSFS_DIR="/sys/kernel/livepatch"
> +MODULE_SYSFS_DIR="/sys/module"
>  
>  # Kselftest framework requirement - SKIP code is 4
>  ksft_skip=4
> @@ -299,7 +300,7 @@ function check_result {
>  	result=$(dmesg | awk -v last_dmesg="$LAST_DMESG" 'p; $0 == last_dmesg { p=1 }' | \
>  		 grep -e 'livepatch:' -e 'test_klp' | \
>  		 grep -v '\(tainting\|taints\) kernel' | \
> -		 sed 's/^\[[ 0-9.]*\] //')
> +		 sed 's/^\[[ 0-9.]*\] //' | sed 's/^test_klp_log: //')

The prefix "test_klp_log:" is not used anywhere. It seems that this
change is not needed in the final version.

>  
>  	if [[ "$expect" == "$result" ]] ; then
>  		echo "ok"

Otherwise, it looks and works nice. With the hunk removed:

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr
kernel test robot May 23, 2024, 7:21 a.m. UTC | #2
Hi Lukas,

kernel test robot noticed the following build errors:

[auto build test ERROR on masahiroy-kbuild/fixes]
[also build test ERROR on shuah-kselftest/next shuah-kselftest/fixes linus/master v6.9]
[cannot apply to masahiroy-kbuild/for-next next-20240517]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lukas-Hruska/livepatch-Add-klp-convert-tool/20240516-223103
base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git fixes
patch link:    https://lore.kernel.org/r/20240516133009.20224-7-lhruska%40suse.cz
patch subject: [PATCH v2 6/6] selftests: livepatch: Test livepatching function using an external symbol
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce:
  $ clang --version
  clang version 18.1.5 (git://gitmirror/llvm_project 617a15a9eac96088ae5e9134248d8236e34b91b1)
  Target: x86_64-unknown-linux-gnu

  $ cd linux

  # download the attached config file and save it as .config

  $ make ARCH=x86_64 LLVM=1 -j$(nproc) olddefconfig
  $ make ARCH=x86_64 LLVM=1 -j$(nproc)
  $ export KDIR=$PWD
  $ make ARCH=x86_64 LLVM=1 -j$(nproc) -C tools/testing/selftests/livepatch

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <yujie.liu@intel.com>
| Closes: https://lore.kernel.org/r/202405190214.M93ZmZJb-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

  make: Entering directory 'tools/testing/selftests/livepatch'
  make -C test_modules
  make[1]: Entering directory 'tools/testing/selftests/livepatch/test_modules'
  make -C /root/linux modules KBUILD_EXTMOD=tools/testing/selftests/livepatch/test_modules
  make[2]: Entering directory '/root/linux'
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_busy.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo2.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_mod.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_extern.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_extern_hello.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_state.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_state2.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_state3.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_shadow_vars.o
    CC [M]  tools/testing/selftests/livepatch/test_modules/test_klp_syscall.o
    MODPOST tools/testing/selftests/livepatch/test_modules/Module.symvers
>> ERROR: modpost: "".klp.sym.rela.test_klp_extern_hello.test_klp_extern_hello.hello_msg,0"" [tools/testing/selftests/livepatch/test_modules/test_klp_extern.ko] undefined!
  make[4]: *** [scripts/Makefile.modpost:146: tools/testing/selftests/livepatch/test_modules/Module.symvers] Error 1
  make[3]: *** [Makefile:1873: modpost] Error 2
  make[2]: *** [Makefile:240: __sub-make] Error 2
  make[2]: Leaving directory 'linux'
  make[1]: *** [Makefile:21: modules] Error 2
  make[1]: Leaving directory 'tools/testing/selftests/livepatch/test_modules'
  make: *** [../lib.mk:107: gen_mods_dir] Error 2
  make: Leaving directory 'tools/testing/selftests/livepatch'


FYI, this build error is captured when using clang compiler, while
there is no such error when using gcc, so this may be a
compiler specific issue.
diff mbox series

Patch

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index fc4c6a016d38..801d55dc06ac 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -7,6 +7,7 @@ 
 MAX_RETRIES=600
 RETRY_INTERVAL=".1"	# seconds
 KLP_SYSFS_DIR="/sys/kernel/livepatch"
+MODULE_SYSFS_DIR="/sys/module"
 
 # Kselftest framework requirement - SKIP code is 4
 ksft_skip=4
@@ -299,7 +300,7 @@  function check_result {
 	result=$(dmesg | awk -v last_dmesg="$LAST_DMESG" 'p; $0 == last_dmesg { p=1 }' | \
 		 grep -e 'livepatch:' -e 'test_klp' | \
 		 grep -v '\(tainting\|taints\) kernel' | \
-		 sed 's/^\[[ 0-9.]*\] //')
+		 sed 's/^\[[ 0-9.]*\] //' | sed 's/^test_klp_log: //')
 
 	if [[ "$expect" == "$result" ]] ; then
 		echo "ok"
@@ -344,3 +345,16 @@  function check_sysfs_value() {
 		die "Unexpected value in $path: $expected_value vs. $value"
 	fi
 }
+
+# read_module_param_value(modname, param) - read module parameter value
+#  modname - livepatch module creating the sysfs interface
+#  param - parameter name
+function read_module_param() {
+   local mod="$1"; shift
+   local param="$1"; shift
+
+   local path="$MODULE_SYSFS_DIR/$mod/parameters/$param"
+
+   log "% echo \"$mod/parameters/$param: \$(cat $path)\""
+   log "$mod/parameters/$param: $(cat $path)"
+}
diff --git a/tools/testing/selftests/livepatch/test-extern.sh b/tools/testing/selftests/livepatch/test-extern.sh
new file mode 100755
index 000000000000..3dde6cabb07c
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test-extern.sh
@@ -0,0 +1,57 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2024 Lukas Hruska <lhruska@suse.cz>
+
+. $(dirname $0)/functions.sh
+
+MOD_LIVEPATCH=test_klp_extern
+MOD_HELLO=test_klp_extern_hello
+PARAM_HELLO=hello
+
+setup_config
+
+# - load a module to be livepatched
+# - load a livepatch that modifies the output from 'hello' parameter
+#   of the previously loaded module and verify correct behaviour
+# - unload the livepatch and make sure the patch was removed
+# - unload the module that was livepatched
+
+start_test "livepatch with external symbol"
+
+load_mod $MOD_HELLO
+
+read_module_param $MOD_HELLO $PARAM_HELLO
+
+load_lp $MOD_LIVEPATCH
+
+read_module_param $MOD_HELLO $PARAM_HELLO
+
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
+
+read_module_param $MOD_HELLO $PARAM_HELLO
+
+unload_mod $MOD_HELLO
+
+check_result "% insmod test_modules/$MOD_HELLO.ko
+% echo \"$MOD_HELLO/parameters/$PARAM_HELLO: \$(cat /sys/module/$MOD_HELLO/parameters/$PARAM_HELLO)\"
+$MOD_HELLO/parameters/$PARAM_HELLO: Hello from kernel module.
+% insmod test_modules/$MOD_LIVEPATCH.ko
+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 \"$MOD_HELLO/parameters/$PARAM_HELLO: \$(cat /sys/module/$MOD_HELLO/parameters/$PARAM_HELLO)\"
+$MOD_HELLO/parameters/$PARAM_HELLO: Hello from livepatched module.
+% 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
+% echo \"$MOD_HELLO/parameters/$PARAM_HELLO: \$(cat /sys/module/$MOD_HELLO/parameters/$PARAM_HELLO)\"
+$MOD_HELLO/parameters/$PARAM_HELLO: Hello from kernel module.
+% rmmod $MOD_HELLO"
+
+exit 0
diff --git a/tools/testing/selftests/livepatch/test_modules/Makefile b/tools/testing/selftests/livepatch/test_modules/Makefile
index e6e638c4bcba..0d6df14787da 100644
--- a/tools/testing/selftests/livepatch/test_modules/Makefile
+++ b/tools/testing/selftests/livepatch/test_modules/Makefile
@@ -6,6 +6,8 @@  obj-m += test_klp_atomic_replace.o \
 	test_klp_callbacks_demo.o \
 	test_klp_callbacks_demo2.o \
 	test_klp_callbacks_mod.o \
+	test_klp_extern.o \
+	test_klp_extern_hello.o \
 	test_klp_livepatch.o \
 	test_klp_state.o \
 	test_klp_state2.o \
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_extern.c b/tools/testing/selftests/livepatch/test_modules/test_klp_extern.c
new file mode 100644
index 000000000000..2a88ae289668
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_extern.c
@@ -0,0 +1,51 @@ 
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2024 Lukas Hruska <lhruska@suse.cz>
+
+#define pr_fmt(fmt) "test_klp_extern_hello: " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+
+extern const char *hello_msg \
+		   KLP_RELOC_SYMBOL(test_klp_extern_hello, test_klp_extern_hello, hello_msg);
+
+static int hello_get(char *buffer, const struct kernel_param *kp)
+{
+	return sysfs_emit(buffer, "%s livepatched module.\n", hello_msg);
+}
+
+static struct klp_func funcs[] = {
+	{
+		.old_name = "hello_get",
+		.new_func = hello_get,
+	}, { }
+};
+
+static struct klp_object objs[] = {
+	{
+		.name = "test_klp_extern_hello",
+		.funcs = funcs,
+	}, { }
+};
+
+static struct klp_patch patch = {
+	.mod = THIS_MODULE,
+	.objs = objs,
+};
+
+static int test_klp_extern_init(void)
+{
+	return klp_enable_patch(&patch);
+}
+
+static void test_klp_extern_exit(void)
+{
+}
+
+module_init(test_klp_extern_init);
+module_exit(test_klp_extern_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
+MODULE_AUTHOR("Lukas Hruska <lhruska@suse.cz>");
+MODULE_DESCRIPTION("Livepatch test: external symbol relocation");
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_extern_hello.c b/tools/testing/selftests/livepatch/test_modules/test_klp_extern_hello.c
new file mode 100644
index 000000000000..58ce4e655eee
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_extern_hello.c
@@ -0,0 +1,36 @@ 
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2024 Lukas Hruska <lhruska@suse.cz>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+const char *hello_msg = "Hello from";
+
+static int hello_get(char *buffer, const struct kernel_param *kp)
+{
+	return sysfs_emit(buffer, "%s kernel module.\n", hello_msg);
+}
+
+static const struct kernel_param_ops hello_ops = {
+	.get	= hello_get
+};
+
+module_param_cb(hello, &hello_ops, NULL, 0400);
+MODULE_PARM_DESC(hello, "Read only parameter greeting the reader.");
+
+static int test_klp_extern_hello_init(void)
+{
+	return 0;
+}
+
+static void test_klp_extern_hello_exit(void)
+{
+}
+
+module_init(test_klp_extern_hello_init);
+module_exit(test_klp_extern_hello_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Lukas Hruska <lhruska@suse.cz>");
+MODULE_DESCRIPTION("Livepatch test: external symbol relocation - test module");