[v3,3/3] selftests: livepatch: Test livepatching a heavily called syscall

  The test proves that a syscall can be livepatched. It is interesting
because syscalls are called a tricky way. Also the process gets
livepatched either when sleeping in the userspace or when entering
or leaving the kernel space.

The livepatch is a bit tricky:
  1. The syscall function name is architecture specific. Also
     ARCH_HAS_SYSCALL_WRAPPER must be taken in account.

  2. The syscall must stay working the same way for other processes
     on the system. It is solved by decrementing a counter only
     for PIDs of the test processes. It means that the test processes
     has to call the livepatched syscall at least once.

The test creates one userspace process per online cpu. The processes
are calling getpid in a busy loop. The intention is to create random
locations when the livepatch gets enabled. Nothing is guarantted.
The magic is in the randomness.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/livepatch/Makefile         |   4 +-
 tools/testing/selftests/livepatch/test-syscall.sh  |  53 ++++++++++
 .../selftests/livepatch/test_klp-call_getpid.c     |  44 ++++++++
 .../selftests/livepatch/test_modules/Makefile      |   3 +-
 .../livepatch/test_modules/test_klp_syscall.c      | 116 +++++++++++++++++++++
 5 files changed, 218 insertions(+), 2 deletions(-)
  

On 10/31/23 15:10, Marcos Paulo de Souza wrote:
> The test proves that a syscall can be livepatched. It is interesting
> because syscalls are called a tricky way. Also the process gets
> livepatched either when sleeping in the userspace or when entering
> or leaving the kernel space.
> 
> The livepatch is a bit tricky:
>    1. The syscall function name is architecture specific. Also
>       ARCH_HAS_SYSCALL_WRAPPER must be taken in account.
> 
>    2. The syscall must stay working the same way for other processes
>       on the system. It is solved by decrementing a counter only
>       for PIDs of the test processes. It means that the test processes
>       has to call the livepatched syscall at least once.
> 
> The test creates one userspace process per online cpu. The processes
> are calling getpid in a busy loop. The intention is to create random
> locations when the livepatch gets enabled. Nothing is guarantted.
> The magic is in the randomness.
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>   tools/testing/selftests/livepatch/Makefile         |   4 +-
>   tools/testing/selftests/livepatch/test-syscall.sh  |  53 ++++++++++
>   .../selftests/livepatch/test_klp-call_getpid.c     |  44 ++++++++
>   .../selftests/livepatch/test_modules/Makefile      |   3 +-
>   .../livepatch/test_modules/test_klp_syscall.c      | 116 +++++++++++++++++++++
>   5 files changed, 218 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
> index 119e2bbebe5d..35418a4790be 100644
> --- a/tools/testing/selftests/livepatch/Makefile
> +++ b/tools/testing/selftests/livepatch/Makefile
> @@ -1,5 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0
>   
> +TEST_GEN_FILES := test_klp-call_getpid
>   TEST_GEN_MODS_DIR := test_modules
>   TEST_PROGS_EXTENDED := functions.sh
>   TEST_PROGS := \
> @@ -8,7 +9,8 @@ TEST_PROGS := \
>   	test-shadow-vars.sh \
>   	test-state.sh \
>   	test-ftrace.sh \
> -	test-sysfs.sh
> +	test-sysfs.sh \
> +	test-syscall.sh
>   
>   TEST_FILES := settings
>   
> diff --git a/tools/testing/selftests/livepatch/test-syscall.sh b/tools/testing/selftests/livepatch/test-syscall.sh
> new file mode 100755
> index 000000000000..b76a881d4013
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test-syscall.sh
> @@ -0,0 +1,53 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2023 SUSE
> +# Author: Marcos Paulo de Souza <mpdesouza@suse.com>
> +
> +. $(dirname $0)/functions.sh
> +
> +MOD_SYSCALL=test_klp_syscall
> +
> +setup_config
> +
> +# - Start _NRPROC processes calling getpid and load a livepatch to patch the
> +#   getpid syscall. Check if all the processes transitioned to the livepatched
> +#   state.
> +
> +start_test "patch getpid syscall while being heavily hammered"
> +
> +for i in $(seq 1 $(getconf _NPROCESSORS_ONLN)); do
> +	./test_klp-call_getpid &
> +	pids[$i]="$!"
> +done
> +
> +pid_list=$(echo ${pids[@]} | tr ' ' ',')
> +load_lp $MOD_SYSCALL klp_pids=$pid_list
> +
> +# wait for all tasks to transition to patched state
> +loop_until 'grep -q '^0$' /sys/kernel/test_klp_syscall/npids'
> +
> +pending_pids=$(cat /sys/kernel/test_klp_syscall/npids)
> +log "$MOD_SYSCALL: Remaining not livepatched processes: $pending_pids"
> +
> +for pid in ${pids[@]}; do
> +	kill $pid || true
> +done
> +
> +disable_lp $MOD_SYSCALL
> +unload_lp $MOD_SYSCALL
> +
> +check_result "% insmod test_modules/$MOD_SYSCALL.ko klp_pids=$pid_list
> +livepatch: enabling patch '$MOD_SYSCALL'
> +livepatch: '$MOD_SYSCALL': initializing patching transition
> +livepatch: '$MOD_SYSCALL': starting patching transition
> +livepatch: '$MOD_SYSCALL': completing patching transition
> +livepatch: '$MOD_SYSCALL': patching complete
> +$MOD_SYSCALL: Remaining not livepatched processes: 0
> +% echo 0 > /sys/kernel/livepatch/$MOD_SYSCALL/enabled
> +livepatch: '$MOD_SYSCALL': initializing unpatching transition
> +livepatch: '$MOD_SYSCALL': starting unpatching transition
> +livepatch: '$MOD_SYSCALL': completing unpatching transition
> +livepatch: '$MOD_SYSCALL': unpatching complete
> +% rmmod $MOD_SYSCALL"
> +
> +exit 0
> diff --git a/tools/testing/selftests/livepatch/test_klp-call_getpid.c b/tools/testing/selftests/livepatch/test_klp-call_getpid.c
> new file mode 100644
> index 000000000000..ce321a2d7308
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test_klp-call_getpid.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 SUSE
> + * Authors: Libor Pechacek <lpechacek@suse.cz>
> + *          Marcos Paulo de Souza <mpdesouza@suse.com>
> + */
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/syscall.h>
> +#include <sys/types.h>
> +#include <signal.h>
> +
> +static int stop;
> +static int sig_int;
> +
> +void hup_handler(int signum)
> +{
> +	stop = 1;
> +}
> +
> +void int_handler(int signum)
> +{
> +	stop = 1;
> +	sig_int = 1;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	long count = 0;
> +
> +	signal(SIGHUP, &hup_handler);
> +	signal(SIGINT, &int_handler);
> +
> +	while (!stop) {
> +		(void)syscall(SYS_getpid);
> +		count++;
> +	}
> +
> +	if (sig_int)
> +		printf("%ld iterations done\n", count);
> +
> +	return 0;
> +}
> diff --git a/tools/testing/selftests/livepatch/test_modules/Makefile b/tools/testing/selftests/livepatch/test_modules/Makefile
> index 6f7c2103d27d..f5e880269bff 100644
> --- a/tools/testing/selftests/livepatch/test_modules/Makefile
> +++ b/tools/testing/selftests/livepatch/test_modules/Makefile
> @@ -10,7 +10,8 @@ obj-m += test_klp_atomic_replace.o \
>   	test_klp_state.o \
>   	test_klp_state2.o \
>   	test_klp_state3.o \
> -	test_klp_shadow_vars.o
> +	test_klp_shadow_vars.o \
> +	test_klp_syscall.o
>   
>   modules:
>   	$(Q)$(MAKE) -C $(KDIR) modules KBUILD_EXTMOD=$(TESTMODS_DIR)
> diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> new file mode 100644
> index 000000000000..619496cc3481
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017-2023 SUSE
> + * Authors: Libor Pechacek <lpechacek@suse.cz>
> + *          Nicolai Stange <nstange@suse.de>
> + *          Marcos Paulo de Souza <mpdesouza@suse.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/livepatch.h>
> +
> +#if defined(__x86_64__)
> +#define FN_PREFIX __x64_
> +#elif defined(__s390x__)
> +#define FN_PREFIX __s390x_
> +#elif defined(__aarch64__)
> +#define FN_PREFIX __arm64_
> +#else
> +/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> +#define FN_PREFIX
> +#endif
> +
> +/* Protects klp_pids */
> +static DEFINE_MUTEX(kpid_mutex);
> +
> +static unsigned int npids, npids_pending;
> +static int klp_pids[NR_CPUS];
> +module_param_array(klp_pids, int, &npids_pending, 0);
> +MODULE_PARM_DESC(klp_pids, "Array of pids to be transitioned to livepatched state.");
> +
> +static ssize_t npids_show(struct kobject *kobj, struct kobj_attribute *attr,
> +			  char *buf)
> +{
> +	return sprintf(buf, "%u\n", npids_pending);
> +}
> +
> +static struct kobj_attribute klp_attr = __ATTR_RO(npids);
> +static struct kobject *klp_kobj;
> +
> +asmlinkage long lp_sys_getpid(void)
> +{
> +	int i;
> +
> +	mutex_lock(&kpid_mutex);
> +	if (npids_pending > 0) {
> +		for (i = 0; i < npids; i++) {
> +			if (current->pid == klp_pids[i]) {
> +				klp_pids[i] = 0;
> +				npids_pending--;
> +				break;
> +			}
> +		}
> +	}
> +	mutex_unlock(&kpid_mutex);
> +
> +	return task_tgid_vnr(current);
> +}
> +
> +static struct klp_func vmlinux_funcs[] = {
> +	{
> +		.old_name = __stringify(FN_PREFIX) "sys_getpid",
> +		.new_func = lp_sys_getpid,
> +	}, {}
> +};
> +
> +static struct klp_object objs[] = {
> +	{
> +		/* name being NULL means vmlinux */
> +		.funcs = vmlinux_funcs,
> +	}, {}
> +};
> +
> +static struct klp_patch patch = {
> +	.mod = THIS_MODULE,
> +	.objs = objs,
> +};
> +
> +static int livepatch_init(void)
> +{
> +	int ret;
> +
> +	klp_kobj = kobject_create_and_add("test_klp_syscall", kernel_kobj);
> +	if (!klp_kobj)
> +		return -ENOMEM;
> +
> +	ret = sysfs_create_file(klp_kobj, &klp_attr.attr);
> +	if (ret) {
> +		kobject_put(klp_kobj);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Save the number pids to transition to livepatched state before the
> +	 * number of pending pids is decremented.
> +	 */
> +	npids = npids_pending;
> +
> +	return klp_enable_patch(&patch);
> +}
> +
> +static void livepatch_exit(void)
> +{
> +	kobject_put(klp_kobj);
> +}
> +
> +module_init(livepatch_init);
> +module_exit(livepatch_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_INFO(livepatch, "Y");
> +MODULE_AUTHOR("Libor Pechacek <lpechacek@suse.cz>");
> +MODULE_AUTHOR("Nicolai Stange <nstange@suse.de>");
> +MODULE_AUTHOR("Marcos Paulo de Souza <mpdesouza@suse.com>");
> +MODULE_DESCRIPTION("Livepatch test: syscall transition");

Missing module name? Is there a reason to not name this module?

thanks,
-- Shuah
  

On Thu, Nov 30, 2023 at 04:24:26PM -0700, Shuah Khan wrote:
> On 10/31/23 15:10, Marcos Paulo de Souza wrote:
> > The test proves that a syscall can be livepatched. It is interesting
> > because syscalls are called a tricky way. Also the process gets
> > livepatched either when sleeping in the userspace or when entering
> > or leaving the kernel space.
> > 
> > The livepatch is a bit tricky:
> >    1. The syscall function name is architecture specific. Also
> >       ARCH_HAS_SYSCALL_WRAPPER must be taken in account.
> > 
> >    2. The syscall must stay working the same way for other processes
> >       on the system. It is solved by decrementing a counter only
> >       for PIDs of the test processes. It means that the test processes
> >       has to call the livepatched syscall at least once.
> > 
> > The test creates one userspace process per online cpu. The processes
> > are calling getpid in a busy loop. The intention is to create random
> > locations when the livepatch gets enabled. Nothing is guarantted.
> > The magic is in the randomness.
> > 
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> >   tools/testing/selftests/livepatch/Makefile         |   4 +-
> >   tools/testing/selftests/livepatch/test-syscall.sh  |  53 ++++++++++
> >   .../selftests/livepatch/test_klp-call_getpid.c     |  44 ++++++++
> >   .../selftests/livepatch/test_modules/Makefile      |   3 +-
> >   .../livepatch/test_modules/test_klp_syscall.c      | 116 +++++++++++++++++++++
> >   5 files changed, 218 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
> > index 119e2bbebe5d..35418a4790be 100644
> > --- a/tools/testing/selftests/livepatch/Makefile
> > +++ b/tools/testing/selftests/livepatch/Makefile
> > @@ -1,5 +1,6 @@
> >   # SPDX-License-Identifier: GPL-2.0
> > +TEST_GEN_FILES := test_klp-call_getpid
> >   TEST_GEN_MODS_DIR := test_modules
> >   TEST_PROGS_EXTENDED := functions.sh
> >   TEST_PROGS := \
> > @@ -8,7 +9,8 @@ TEST_PROGS := \
> >   	test-shadow-vars.sh \
> >   	test-state.sh \
> >   	test-ftrace.sh \
> > -	test-sysfs.sh
> > +	test-sysfs.sh \
> > +	test-syscall.sh
> >   TEST_FILES := settings
> > diff --git a/tools/testing/selftests/livepatch/test-syscall.sh b/tools/testing/selftests/livepatch/test-syscall.sh
> > new file mode 100755
> > index 000000000000..b76a881d4013
> > --- /dev/null
> > +++ b/tools/testing/selftests/livepatch/test-syscall.sh
> > @@ -0,0 +1,53 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2023 SUSE
> > +# Author: Marcos Paulo de Souza <mpdesouza@suse.com>
> > +
> > +. $(dirname $0)/functions.sh
> > +
> > +MOD_SYSCALL=test_klp_syscall
> > +
> > +setup_config
> > +
> > +# - Start _NRPROC processes calling getpid and load a livepatch to patch the
> > +#   getpid syscall. Check if all the processes transitioned to the livepatched
> > +#   state.
> > +
> > +start_test "patch getpid syscall while being heavily hammered"
> > +
> > +for i in $(seq 1 $(getconf _NPROCESSORS_ONLN)); do
> > +	./test_klp-call_getpid &
> > +	pids[$i]="$!"
> > +done
> > +
> > +pid_list=$(echo ${pids[@]} | tr ' ' ',')
> > +load_lp $MOD_SYSCALL klp_pids=$pid_list
> > +
> > +# wait for all tasks to transition to patched state
> > +loop_until 'grep -q '^0$' /sys/kernel/test_klp_syscall/npids'
> > +
> > +pending_pids=$(cat /sys/kernel/test_klp_syscall/npids)
> > +log "$MOD_SYSCALL: Remaining not livepatched processes: $pending_pids"
> > +
> > +for pid in ${pids[@]}; do
> > +	kill $pid || true
> > +done
> > +
> > +disable_lp $MOD_SYSCALL
> > +unload_lp $MOD_SYSCALL
> > +
> > +check_result "% insmod test_modules/$MOD_SYSCALL.ko klp_pids=$pid_list
> > +livepatch: enabling patch '$MOD_SYSCALL'
> > +livepatch: '$MOD_SYSCALL': initializing patching transition
> > +livepatch: '$MOD_SYSCALL': starting patching transition
> > +livepatch: '$MOD_SYSCALL': completing patching transition
> > +livepatch: '$MOD_SYSCALL': patching complete
> > +$MOD_SYSCALL: Remaining not livepatched processes: 0
> > +% echo 0 > /sys/kernel/livepatch/$MOD_SYSCALL/enabled
> > +livepatch: '$MOD_SYSCALL': initializing unpatching transition
> > +livepatch: '$MOD_SYSCALL': starting unpatching transition
> > +livepatch: '$MOD_SYSCALL': completing unpatching transition
> > +livepatch: '$MOD_SYSCALL': unpatching complete
> > +% rmmod $MOD_SYSCALL"
> > +
> > +exit 0
> > diff --git a/tools/testing/selftests/livepatch/test_klp-call_getpid.c b/tools/testing/selftests/livepatch/test_klp-call_getpid.c
> > new file mode 100644
> > index 000000000000..ce321a2d7308
> > --- /dev/null
> > +++ b/tools/testing/selftests/livepatch/test_klp-call_getpid.c
> > @@ -0,0 +1,44 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2023 SUSE
> > + * Authors: Libor Pechacek <lpechacek@suse.cz>
> > + *          Marcos Paulo de Souza <mpdesouza@suse.com>
> > + */
> > +
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +#include <sys/syscall.h>
> > +#include <sys/types.h>
> > +#include <signal.h>
> > +
> > +static int stop;
> > +static int sig_int;
> > +
> > +void hup_handler(int signum)
> > +{
> > +	stop = 1;
> > +}
> > +
> > +void int_handler(int signum)
> > +{
> > +	stop = 1;
> > +	sig_int = 1;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	long count = 0;
> > +
> > +	signal(SIGHUP, &hup_handler);
> > +	signal(SIGINT, &int_handler);
> > +
> > +	while (!stop) {
> > +		(void)syscall(SYS_getpid);
> > +		count++;
> > +	}
> > +
> > +	if (sig_int)
> > +		printf("%ld iterations done\n", count);
> > +
> > +	return 0;
> > +}
> > diff --git a/tools/testing/selftests/livepatch/test_modules/Makefile b/tools/testing/selftests/livepatch/test_modules/Makefile
> > index 6f7c2103d27d..f5e880269bff 100644
> > --- a/tools/testing/selftests/livepatch/test_modules/Makefile
> > +++ b/tools/testing/selftests/livepatch/test_modules/Makefile
> > @@ -10,7 +10,8 @@ obj-m += test_klp_atomic_replace.o \
> >   	test_klp_state.o \
> >   	test_klp_state2.o \
> >   	test_klp_state3.o \
> > -	test_klp_shadow_vars.o
> > +	test_klp_shadow_vars.o \
> > +	test_klp_syscall.o
> >   modules:
> >   	$(Q)$(MAKE) -C $(KDIR) modules KBUILD_EXTMOD=$(TESTMODS_DIR)
> > diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > new file mode 100644
> > index 000000000000..619496cc3481
> > --- /dev/null
> > +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > @@ -0,0 +1,116 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2017-2023 SUSE
> > + * Authors: Libor Pechacek <lpechacek@suse.cz>
> > + *          Nicolai Stange <nstange@suse.de>
> > + *          Marcos Paulo de Souza <mpdesouza@suse.com>
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/sched.h>
> > +#include <linux/slab.h>
> > +#include <linux/livepatch.h>
> > +
> > +#if defined(__x86_64__)
> > +#define FN_PREFIX __x64_
> > +#elif defined(__s390x__)
> > +#define FN_PREFIX __s390x_
> > +#elif defined(__aarch64__)
> > +#define FN_PREFIX __arm64_
> > +#else
> > +/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > +#define FN_PREFIX
> > +#endif
> > +
> > +/* Protects klp_pids */
> > +static DEFINE_MUTEX(kpid_mutex);
> > +
> > +static unsigned int npids, npids_pending;
> > +static int klp_pids[NR_CPUS];
> > +module_param_array(klp_pids, int, &npids_pending, 0);
> > +MODULE_PARM_DESC(klp_pids, "Array of pids to be transitioned to livepatched state.");
> > +
> > +static ssize_t npids_show(struct kobject *kobj, struct kobj_attribute *attr,
> > +			  char *buf)
> > +{
> > +	return sprintf(buf, "%u\n", npids_pending);
> > +}
> > +
> > +static struct kobj_attribute klp_attr = __ATTR_RO(npids);
> > +static struct kobject *klp_kobj;
> > +
> > +asmlinkage long lp_sys_getpid(void)
> > +{
> > +	int i;
> > +
> > +	mutex_lock(&kpid_mutex);
> > +	if (npids_pending > 0) {
> > +		for (i = 0; i < npids; i++) {
> > +			if (current->pid == klp_pids[i]) {
> > +				klp_pids[i] = 0;
> > +				npids_pending--;
> > +				break;
> > +			}
> > +		}
> > +	}
> > +	mutex_unlock(&kpid_mutex);
> > +
> > +	return task_tgid_vnr(current);
> > +}
> > +
> > +static struct klp_func vmlinux_funcs[] = {
> > +	{
> > +		.old_name = __stringify(FN_PREFIX) "sys_getpid",
> > +		.new_func = lp_sys_getpid,
> > +	}, {}
> > +};
> > +
> > +static struct klp_object objs[] = {
> > +	{
> > +		/* name being NULL means vmlinux */
> > +		.funcs = vmlinux_funcs,
> > +	}, {}
> > +};
> > +
> > +static struct klp_patch patch = {
> > +	.mod = THIS_MODULE,
> > +	.objs = objs,
> > +};
> > +
> > +static int livepatch_init(void)
> > +{
> > +	int ret;
> > +
> > +	klp_kobj = kobject_create_and_add("test_klp_syscall", kernel_kobj);
> > +	if (!klp_kobj)
> > +		return -ENOMEM;
> > +
> > +	ret = sysfs_create_file(klp_kobj, &klp_attr.attr);
> > +	if (ret) {
> > +		kobject_put(klp_kobj);
> > +		return ret;
> > +	}
> > +
> > +	/*
> > +	 * Save the number pids to transition to livepatched state before the
> > +	 * number of pending pids is decremented.
> > +	 */
> > +	npids = npids_pending;
> > +
> > +	return klp_enable_patch(&patch);
> > +}
> > +
> > +static void livepatch_exit(void)
> > +{
> > +	kobject_put(klp_kobj);
> > +}
> > +
> > +module_init(livepatch_init);
> > +module_exit(livepatch_exit);
> > +MODULE_LICENSE("GPL");
> > +MODULE_INFO(livepatch, "Y");
> > +MODULE_AUTHOR("Libor Pechacek <lpechacek@suse.cz>");
> > +MODULE_AUTHOR("Nicolai Stange <nstange@suse.de>");
> > +MODULE_AUTHOR("Marcos Paulo de Souza <mpdesouza@suse.com>");
> > +MODULE_DESCRIPTION("Livepatch test: syscall transition");
> 
> Missing module name? Is there a reason to not name this module?

Can you please elaborate? This new module use the same MODULE_ macros used by
the current livepatch selftests. What do you mean by module name?

Thanks in advance,
  Marcos

> 
> thanks,
> -- Shuah
  

On 12/1/23 06:13, Marcos Paulo de Souza wrote:
> On Thu, Nov 30, 2023 at 04:24:26PM -0700, Shuah Khan wrote:

>>
>> Missing module name? Is there a reason to not name this module?
> 
> Can you please elaborate? This new module use the same MODULE_ macros used by
> the current livepatch selftests. What do you mean by module name?
> 

Pre-commit checpatch script spdx check complained about the module name.
Please run it to see the message.

thanks,
-- Shuah
  

On Fri, 2023-12-01 at 16:38 +0000, Shuah Khan wrote:
> On 12/1/23 06:13, Marcos Paulo de Souza wrote:
> > On Thu, Nov 30, 2023 at 04:24:26PM -0700, Shuah Khan wrote:
> 
> > > 
> > > Missing module name? Is there a reason to not name this module?
> > 
> > Can you please elaborate? This new module use the same MODULE_
> > macros used by
> > the current livepatch selftests. What do you mean by module name?
> > 
> 
> Pre-commit checpatch script spdx check complained about the module
> name.
> Please run it to see the message.

I've ran checkpatch on all the tree patches, and so far it complained
about MAINTANERS file needing to be updated (since we removed
lib/livepatch and it's mentioned on MAINTAINERS file):

$ ./scripts/checkpatch.pl 000*.patch                    
-----------------------------------------------------------
0001-kselftests-lib.mk-Add-TEST_GEN_MODS_DIR-variable.patch
-----------------------------------------------------------
total: 0 errors, 0 warnings, 68 lines checked

0001-kselftests-lib.mk-Add-TEST_GEN_MODS_DIR-variable.patch has no
obvious style problems and is ready for submission.
---------------------------------------------------------------
0002-livepatch-Move-tests-from-lib-livepatch-to-selftests.patch
---------------------------------------------------------------
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit
description?)
#17: 
debug and rebuild the tests by running make on the selftests/livepatch
directory,

WARNING: added, moved or deleted file(s), does MAINTAINERS need
updating?
#82: 
 rename {lib/livepatch =>
tools/testing/selftests/livepatch/test_modules}/test_klp_atomic_replace
.c (100%)

total: 0 errors, 2 warnings, 519 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-
inplace.

0002-livepatch-Move-tests-from-lib-livepatch-to-selftests.patch has
style problems, please review.
---------------------------------------------------------------
0003-selftests-livepatch-Test-livepatching-a-heavily-call.patch
---------------------------------------------------------------
WARNING: added, moved or deleted file(s), does MAINTAINERS need
updating?
#60: 
new file mode 100755

total: 0 errors, 1 warnings, 237 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-
inplace.

0003-selftests-livepatch-Test-livepatching-a-heavily-call.patch has
style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

I couldn't find any mention about "missing module name". Is your script
showing more warnings than these ones? Can you please share your
output?

I'll fix MAINTAINERS file but I'll wait until I understand what's
missing in your checkpatch script to resend the patchset.

Thanks,
  Marcos 

> 
> thanks,
> -- Shuah
>
  

On 12/5/23 05:52, mpdesouza@suse.com wrote:
> On Fri, 2023-12-01 at 16:38 +0000, Shuah Khan wrote:

> 0003-selftests-livepatch-Test-livepatching-a-heavily-call.patch has
> style problems, please review.
> 
> NOTE: If any of the errors are false positives, please report
>        them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> I couldn't find any mention about "missing module name". Is your script
> showing more warnings than these ones? Can you please share your
> output?
> 
> I'll fix MAINTAINERS file but I'll wait until I understand what's
> missing in your checkpatch script to resend the patchset.
> 

Looks like it is coming a script - still my question stands on
whether or not you would need a module name for this module?

I am not too concerned about MAINTAINERS file warns.

I am assuming you will be sending a new version to address
Joe Lawrence's comments?

thanks,
-- Shuah
  

Hi,

On Tue, 5 Dec 2023, Shuah Khan wrote:

> On 12/5/23 05:52, mpdesouza@suse.com wrote:
> > On Fri, 2023-12-01 at 16:38 +0000, Shuah Khan wrote:
> 
> > 0003-selftests-livepatch-Test-livepatching-a-heavily-call.patch has
> > style problems, please review.
> > 
> > NOTE: If any of the errors are false positives, please report
> >        them to the maintainer, see CHECKPATCH in MAINTAINERS.
> > 
> > I couldn't find any mention about "missing module name". Is your script
> > showing more warnings than these ones? Can you please share your
> > output?
> > 
> > I'll fix MAINTAINERS file but I'll wait until I understand what's
> > missing in your checkpatch script to resend the patchset.
> > 
> 
> Looks like it is coming a script - still my question stands on
> whether or not you would need a module name for this module?

I admit I am also clueless here. The module name is given in Makefile. In 
this case in test_modules/Makefile. I do not know of anything else. There 
is no MODULE_NAME macro. Could you elaborate, please?

Miroslav
  

On 12/6/23 07:39, Miroslav Benes wrote:
> Hi,
> 
> On Tue, 5 Dec 2023, Shuah Khan wrote:
> 
>> On 12/5/23 05:52, mpdesouza@suse.com wrote:
>>> On Fri, 2023-12-01 at 16:38 +0000, Shuah Khan wrote:
>>
>>> 0003-selftests-livepatch-Test-livepatching-a-heavily-call.patch has
>>> style problems, please review.
>>>
>>> NOTE: If any of the errors are false positives, please report
>>>         them to the maintainer, see CHECKPATCH in MAINTAINERS.
>>>
>>> I couldn't find any mention about "missing module name". Is your script
>>> showing more warnings than these ones? Can you please share your
>>> output?
>>>
>>> I'll fix MAINTAINERS file but I'll wait until I understand what's
>>> missing in your checkpatch script to resend the patchset.
>>>
>>
>> Looks like it is coming a script - still my question stands on
>> whether or not you would need a module name for this module?
> 
> I admit I am also clueless here. The module name is given in Makefile. In
> this case in test_modules/Makefile. I do not know of anything else. There
> is no MODULE_NAME macro. Could you elaborate, please?
> 

I see that now.

thanks,
-- Shuah
  

On 12/11/23 16:53, Shuah Khan wrote:
> On 12/6/23 07:39, Miroslav Benes wrote:
>> Hi,
>>
>> On Tue, 5 Dec 2023, Shuah Khan wrote:
>>
>>> On 12/5/23 05:52, mpdesouza@suse.com wrote:
>>>> On Fri, 2023-12-01 at 16:38 +0000, Shuah Khan wrote:
>>>
>>>> 0003-selftests-livepatch-Test-livepatching-a-heavily-call.patch has
>>>> style problems, please review.
>>>>
>>>> NOTE: If any of the errors are false positives, please report
>>>>         them to the maintainer, see CHECKPATCH in MAINTAINERS.
>>>>
>>>> I couldn't find any mention about "missing module name". Is your script
>>>> showing more warnings than these ones? Can you please share your
>>>> output?
>>>>
>>>> I'll fix MAINTAINERS file but I'll wait until I understand what's
>>>> missing in your checkpatch script to resend the patchset.
>>>>
>>>
>>> Looks like it is coming a script - still my question stands on
>>> whether or not you would need a module name for this module?
>>
>> I admit I am also clueless here. The module name is given in Makefile. In
>> this case in test_modules/Makefile. I do not know of anything else. There
>> is no MODULE_NAME macro. Could you elaborate, please?
>>
> 
> I see that now.
> 

Hi Shuah,

In the other replies to this thread, Marcos noted that he would add some
text to the commit / documentation on running and building the selftests
directly in the kernel tree (that would get my Ack) ... is there
anything else to be updated for a hopefully final v4 (for your Ack)?

Thanks,
  

On 12/15/23 13:36, Joe Lawrence wrote:
> On 12/11/23 16:53, Shuah Khan wrote:
>> On 12/6/23 07:39, Miroslav Benes wrote:
>>> Hi,
>>>
>>> On Tue, 5 Dec 2023, Shuah Khan wrote:
>>>
>>>> On 12/5/23 05:52, mpdesouza@suse.com wrote:
>>>>> On Fri, 2023-12-01 at 16:38 +0000, Shuah Khan wrote:
>>>>
>>>>> 0003-selftests-livepatch-Test-livepatching-a-heavily-call.patch has
>>>>> style problems, please review.
>>>>>
>>>>> NOTE: If any of the errors are false positives, please report
>>>>>          them to the maintainer, see CHECKPATCH in MAINTAINERS.
>>>>>
>>>>> I couldn't find any mention about "missing module name". Is your script
>>>>> showing more warnings than these ones? Can you please share your
>>>>> output?
>>>>>
>>>>> I'll fix MAINTAINERS file but I'll wait until I understand what's
>>>>> missing in your checkpatch script to resend the patchset.
>>>>>
>>>>
>>>> Looks like it is coming a script - still my question stands on
>>>> whether or not you would need a module name for this module?
>>>
>>> I admit I am also clueless here. The module name is given in Makefile. In
>>> this case in test_modules/Makefile. I do not know of anything else. There
>>> is no MODULE_NAME macro. Could you elaborate, please?
>>>
>>
>> I see that now.
>>
> 
> Hi Shuah,
> 
> In the other replies to this thread, Marcos noted that he would add some
> text to the commit / documentation on running and building the selftests
> directly in the kernel tree (that would get my Ack) ... is there
> anything else to be updated for a hopefully final v4 (for your Ack)?
> 


I am waiting for v4 with your comments are addressed. I can take
this through kselftest tree.

thanks,
-- Shuah
  

On Mon, 2023-12-18 at 13:47 -0700, Shuah Khan wrote:
> On 12/15/23 13:36, Joe Lawrence wrote:
> > On 12/11/23 16:53, Shuah Khan wrote:
> > > On 12/6/23 07:39, Miroslav Benes wrote:
> > > > Hi,
> > > > 
> > > > On Tue, 5 Dec 2023, Shuah Khan wrote:
> > > > 
> > > > > On 12/5/23 05:52, mpdesouza@suse.com wrote:
> > > > > > On Fri, 2023-12-01 at 16:38 +0000, Shuah Khan wrote:
> > > > > 
> > > > > > 0003-selftests-livepatch-Test-livepatching-a-heavily-
> > > > > > call.patch has
> > > > > > style problems, please review.
> > > > > > 
> > > > > > NOTE: If any of the errors are false positives, please
> > > > > > report
> > > > > >          them to the maintainer, see CHECKPATCH in
> > > > > > MAINTAINERS.
> > > > > > 
> > > > > > I couldn't find any mention about "missing module name". Is
> > > > > > your script
> > > > > > showing more warnings than these ones? Can you please share
> > > > > > your
> > > > > > output?
> > > > > > 
> > > > > > I'll fix MAINTAINERS file but I'll wait until I understand
> > > > > > what's
> > > > > > missing in your checkpatch script to resend the patchset.
> > > > > > 
> > > > > 
> > > > > Looks like it is coming a script - still my question stands
> > > > > on
> > > > > whether or not you would need a module name for this module?
> > > > 
> > > > I admit I am also clueless here. The module name is given in
> > > > Makefile. In
> > > > this case in test_modules/Makefile. I do not know of anything
> > > > else. There
> > > > is no MODULE_NAME macro. Could you elaborate, please?
> > > > 
> > > 
> > > I see that now.
> > > 
> > 
> > Hi Shuah,
> > 
> > In the other replies to this thread, Marcos noted that he would add
> > some
> > text to the commit / documentation on running and building the
> > selftests
> > directly in the kernel tree (that would get my Ack) ... is there
> > anything else to be updated for a hopefully final v4 (for your
> > Ack)?
> > 
> 
> 
> I am waiting for v4 with your comments are addressed. I can take
> this through kselftest tree.

Ok. I'm already preparing the v4. The plan is to send in the next few
days.

Thanks,
  Marcos

> 
> thanks,
> -- Shuah
>