Re: [PATCH] selftests: livepatch: test livepatching a kprobed function

[mpdesouza@suse.com]

On Fri, 2024-09-13 at 14:58 +0200, Michael Vetter wrote:
> The test proves that a function that is being kprobed and uses a
> post_handler cannot be livepatched.
> 
> Only one ftrace_ops with FTRACE_OPS_FL_IPMODIFY set may be registered
> to any given function at a time.

First of all, thanks a lot for sending the patch!

> 
> Signed-off-by: Michael Vetter <mvetter@suse.com>
> ---
>  tools/testing/selftests/livepatch/Makefile    |  3 +-
>  .../selftests/livepatch/test-kprobe.sh        | 62
> +++++++++++++++++++
>  .../selftests/livepatch/test_modules/Makefile |  3 +-
>  .../livepatch/test_modules/test_klp_kprobe.c  | 38 ++++++++++++
>  4 files changed, 104 insertions(+), 2 deletions(-)
>  create mode 100755 tools/testing/selftests/livepatch/test-kprobe.sh
>  create mode 100644
> tools/testing/selftests/livepatch/test_modules/test_klp_kprobe.c
> 
> diff --git a/tools/testing/selftests/livepatch/Makefile
> b/tools/testing/selftests/livepatch/Makefile
> index 35418a4790be..a080eb54a215 100644
> --- a/tools/testing/selftests/livepatch/Makefile
> +++ b/tools/testing/selftests/livepatch/Makefile
> @@ -10,7 +10,8 @@ TEST_PROGS := \
>  	test-state.sh \
>  	test-ftrace.sh \
>  	test-sysfs.sh \
> -	test-syscall.sh
> +	test-syscall.sh \
> +	test-kprobe.sh
>  
>  TEST_FILES := settings
>  
> diff --git a/tools/testing/selftests/livepatch/test-kprobe.sh
> b/tools/testing/selftests/livepatch/test-kprobe.sh
> new file mode 100755
> index 000000000000..0c62c6b81e18
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test-kprobe.sh
> @@ -0,0 +1,62 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2024 SUSE
> +# Author: Michael Vetter <mvetter@suse.com>
> +
> +. $(dirname $0)/functions.sh
> +
> +MOD_LIVEPATCH=test_klp_livepatch
> +MOD_KPROBE=test_klp_kprobe
> +
> +setup_config
> +
> +# Kprobe a function and verify that we can't livepatch that same
> function
> +# when it uses a post_handler since only one IPMODIFY maybe be
> registered
> +# to any given function at a time.
> +
> +start_test "livepatch interaction with kprobed function with
> post_handler"
> +
> +echo 1 > /sys/kernel/debug/kprobes/enabled

I believe that in order to keep the system in the same as it was before
running the tests we might want to save the current kprobe enabled
state, enable it (what you are doing) and then restoring to the initial
state.

Everything else is fine, thank you so much!

With this fixed,

Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>

> +
> +load_mod $MOD_KPROBE has_post_handler=true
> +load_failing_mod $MOD_LIVEPATCH
> +unload_mod $MOD_KPROBE
> +
> +check_result "% insmod test_modules/test_klp_kprobe.ko
> has_post_handler=true
> +% insmod test_modules/$MOD_LIVEPATCH.ko
> +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
> +insmod: ERROR: could not insert module
> test_modules/$MOD_LIVEPATCH.ko: Device or resource busy
> +% rmmod test_klp_kprobe"
> +
> +start_test "livepatch interaction with kprobed function without
> post_handler"
> +
> +load_mod $MOD_KPROBE has_post_handler=false
> +load_lp $MOD_LIVEPATCH
> +
> +unload_mod $MOD_KPROBE
> +disable_lp $MOD_LIVEPATCH
> +unload_lp $MOD_LIVEPATCH
> +
> +check_result "% insmod test_modules/test_klp_kprobe.ko
> has_post_handler=false
> +% 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
> +% rmmod test_klp_kprobe
> +% 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
> diff --git a/tools/testing/selftests/livepatch/test_modules/Makefile
> b/tools/testing/selftests/livepatch/test_modules/Makefile
> index e6e638c4bcba..4981d270f128 100644
> --- a/tools/testing/selftests/livepatch/test_modules/Makefile
> +++ b/tools/testing/selftests/livepatch/test_modules/Makefile
> @@ -11,7 +11,8 @@ obj-m += test_klp_atomic_replace.o \
>  	test_klp_state2.o \
>  	test_klp_state3.o \
>  	test_klp_shadow_vars.o \
> -	test_klp_syscall.o
> +	test_klp_syscall.o \
> +	test_klp_kprobe.o
>  
>  # Ensure that KDIR exists, otherwise skip the compilation
>  modules:
> diff --git
> a/tools/testing/selftests/livepatch/test_modules/test_klp_kprobe.c
> b/tools/testing/selftests/livepatch/test_modules/test_klp_kprobe.c
> new file mode 100644
> index 000000000000..49b579ea1054
> --- /dev/null
> +++
> b/tools/testing/selftests/livepatch/test_modules/test_klp_kprobe.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2024 Marcos Paulo de Souza <mpdesouza@suse.com>
> +// Copyright (C) 2024 Michael Vetter <mvetter@suse.com>
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/kprobes.h>
> +
> +static bool has_post_handler = true;
> +module_param(has_post_handler, bool, 0444);
> +
> +static void __kprobes post_handler(struct kprobe *p, struct pt_regs
> *regs,
> +				unsigned long flags)
> +{
> +}
> +
> +static struct kprobe kp = {
> +	.symbol_name = "cmdline_proc_show",
> +};
> +
> +static int __init kprobe_init(void)
> +{
> +	if (has_post_handler)
> +		kp.post_handler = post_handler;
> +
> +	return register_kprobe(&kp);
> +}
> +
> +static void __exit kprobe_exit(void)
> +{
> +	unregister_kprobe(&kp);
> +}
> +
> +module_init(kprobe_init)
> +module_exit(kprobe_exit)
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Michael Vetter <mvetter@suse.com>");
> +MODULE_DESCRIPTION("Livepatch test: livepatch kprobed function");