Live Patching
 help / color / mirror / Atom feed
* Re: [PATCH bpf-next 0/3] Fix ftrace for livepatch + BPF fexit programs
From: Alexei Starovoitov @ 2025-10-24 16:42 UTC (permalink / raw)
  To: Song Liu
  Cc: bpf, linux-trace-kernel, live-patching, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Steven Rostedt,
	Andrey Grodzovsky, Masami Hiramatsu, Kernel Team
In-Reply-To: <20251024071257.3956031-1-song@kernel.org>

On Fri, Oct 24, 2025 at 12:13 AM Song Liu <song@kernel.org> wrote:
>
> livepatch and BPF trampoline are two special users of ftrace. livepatch
> uses ftrace with IPMODIFY flag and BPF trampoline uses ftrace direct
> functions. When livepatch and BPF trampoline with fexit programs attach to
> the same kernel function, BPF trampoline needs to call into the patched
> version of the kernel function.

This sounds serious. Pls target bpf tree in respin after addressing
the comments.

pw-bot: cr

^ permalink raw reply

* Re: [PATCH bpf-next 2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
From: Steven Rostedt @ 2025-10-24 16:21 UTC (permalink / raw)
  To: Song Liu
  Cc: Jiri Olsa, Song Liu, bpf@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org, live-patching@vger.kernel.org,
	ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	andrey.grodzovsky@crowdstrike.com, mhiramat@kernel.org,
	Kernel Team
In-Reply-To: <D4EEB2BC-E87F-4F85-B043-867D4E1ED573@meta.com>

On Fri, 24 Oct 2025 15:47:04 +0000
Song Liu <songliubraving@meta.com> wrote:

> >> --- a/kernel/trace/ftrace.c
> >> +++ b/kernel/trace/ftrace.c
> >> @@ -2020,8 +2020,6 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
> >> if (is_ipmodify)
> >> goto rollback;
> >> 
> >> - FTRACE_WARN_ON(rec->flags & FTRACE_FL_DIRECT);  
> > 
> > why is this needed?  
> 
> This is needed for the modify_ftrace_direct case, because 
> the record already have a direct function (BPF trampoline)
> attached. 

I don't like the fact that it's removing a check for other cases.

It needs to be denoted that this use case is "OK" where as other use cases
are not. That way the check is still there for other cases and only "OK"
for this use case.

Something like this:

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 370f620734cf..51b205bafe80 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1971,7 +1971,8 @@ static void ftrace_hash_rec_enable_modify(struct ftrace_ops *ops)
  */
 static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
 					 struct ftrace_hash *old_hash,
-					 struct ftrace_hash *new_hash)
+					 struct ftrace_hash *new_hash,
+					 bool update)
 {
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec, *end = NULL;
@@ -2020,6 +2021,16 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
 				if (is_ipmodify)
 					goto rollback;
 
+				/*
+				 * If this is called by __modify_ftrace_direct()
+				 * then it is only chaning where the direct
+				 * pointer is jumping to, and the record already
+				 * points to a direct trampoline. If it isn't
+				 * then it is a bug to update ipmodify on a direct
+				 * caller.
+				 */
+				FTRACE_WARN_ON(!update &&
+					       (rec->flags & FTRACE_FL_DIRECT));
 				/*
 				 * Another ops with IPMODIFY is already
 				 * attached. We are now attaching a direct
@@ -2067,14 +2078,14 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
 	return -EBUSY;
 }
 
-static int ftrace_hash_ipmodify_enable(struct ftrace_ops *ops)
+static int ftrace_hash_ipmodify_enable(struct ftrace_ops *ops, bool update)
 {
 	struct ftrace_hash *hash = ops->func_hash->filter_hash;
 
 	if (ftrace_hash_empty(hash))
 		hash = NULL;
 
-	return __ftrace_hash_update_ipmodify(ops, EMPTY_HASH, hash);
+	return __ftrace_hash_update_ipmodify(ops, EMPTY_HASH, hash, update);
 }
 
 /* Disabling always succeeds */
@@ -2085,7 +2096,7 @@ static void ftrace_hash_ipmodify_disable(struct ftrace_ops *ops)
 	if (ftrace_hash_empty(hash))
 		hash = NULL;
 
-	__ftrace_hash_update_ipmodify(ops, hash, EMPTY_HASH);
+	__ftrace_hash_update_ipmodify(ops, hash, EMPTY_HASH, false);
 }
 
 static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
@@ -2099,7 +2110,7 @@ static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
 	if (ftrace_hash_empty(new_hash))
 		new_hash = NULL;
 
-	return __ftrace_hash_update_ipmodify(ops, old_hash, new_hash);
+	return __ftrace_hash_update_ipmodify(ops, old_hash, new_hash, false);
 }
 
 static void print_ip_ins(const char *fmt, const unsigned char *p)
@@ -3059,7 +3070,7 @@ int ftrace_startup(struct ftrace_ops *ops, int command)
 	 */
 	ops->flags |= FTRACE_OPS_FL_ENABLED | FTRACE_OPS_FL_ADDING;
 
-	ret = ftrace_hash_ipmodify_enable(ops);
+	ret = ftrace_hash_ipmodify_enable(ops, false);
 	if (ret < 0) {
 		/* Rollback registration process */
 		__unregister_ftrace_function(ops);
@@ -6131,7 +6142,7 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
 	 * ops->ops_func for the ops. This is needed because the above
 	 * register_ftrace_function_nolock() worked on tmp_ops.
 	 */
-	err = ftrace_hash_ipmodify_enable(ops);
+	err = ftrace_hash_ipmodify_enable(ops, true);
 	if (err)
 		goto out;
 

-- Steve

^ permalink raw reply related

* Re: [PATCH bpf-next 2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
From: Song Liu @ 2025-10-24 15:47 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Song Liu, bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	live-patching@vger.kernel.org, ast@kernel.org,
	daniel@iogearbox.net, andrii@kernel.org, rostedt@goodmis.org,
	andrey.grodzovsky@crowdstrike.com, mhiramat@kernel.org,
	Kernel Team
In-Reply-To: <aPtmThVpiCrlKc0b@krava>



> On Oct 24, 2025, at 4:43 AM, Jiri Olsa <olsajiri@gmail.com> wrote:
> 
> On Fri, Oct 24, 2025 at 12:12:56AM -0700, Song Liu wrote:
>> ftrace_hash_ipmodify_enable() checks IPMODIFY and DIRECT ftrace_ops on
>> the same kernel function. When needed, ftrace_hash_ipmodify_enable()
>> calls ops->ops_func() to prepare the direct ftrace (BPF trampoline) to
>> share the same function as the IPMODIFY ftrace (livepatch).
>> 
>> ftrace_hash_ipmodify_enable() is called in register_ftrace_direct() path,
>> but not called in modify_ftrace_direct() path. As a result, the following
>> operations will break livepatch:
>> 
>> 1. Load livepatch to a kernel function;
>> 2. Attach fentry program to the kernel function;
>> 3. Attach fexit program to the kernel function.
>> 
>> After 3, the kernel function being used will not be the livepatched
>> version, but the original version.
>> 
>> Fix this by adding ftrace_hash_ipmodify_enable() to modify_ftrace_direct()
>> and adjust some logic around the call.
>> 
>> Signed-off-by: Song Liu <song@kernel.org>
>> ---
>> kernel/bpf/trampoline.c | 12 +++++++-----
>> kernel/trace/ftrace.c   | 12 ++++++++++--
>> 2 files changed, 17 insertions(+), 7 deletions(-)
>> 
>> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
>> index 5949095e51c3..8015f5dc3169 100644
>> --- a/kernel/bpf/trampoline.c
>> +++ b/kernel/bpf/trampoline.c
>> @@ -221,6 +221,13 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
>> 
>> if (tr->func.ftrace_managed) {
>> ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
>> + /*
>> + * Clearing fops->trampoline_mutex and fops->NULL is
> 
> s/trampoline_mutex/trampoline/

Good catch!

> 
>> + * needed by the "goto again" case in
>> + * bpf_trampoline_update().
>> + */
>> + tr->fops->trampoline = 0;
>> + tr->fops->func = NULL;
> 
> IIUC you move this because if modify_fentry returns -EAGAIN
> we don't want to reset the trampoline, right?

Right, we don’t want to reset this in the modify_fentry path. 
We can add a check before “goto again” so that we only do the
reset for register_fentry, but I think it is cleaner this way. 
I can be convinced to change it. 

> 
>> ret = register_ftrace_direct(tr->fops, (long)new_addr);
>> } else {
>> ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
>> @@ -479,11 +486,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>> * BPF_TRAMP_F_SHARE_IPMODIFY is set, we can generate the
>> * trampoline again, and retry register.
>> */
>> - /* reset fops->func and fops->trampoline for re-register */
>> - tr->fops->func = NULL;
>> - tr->fops->trampoline = 0;
>> -
>> - /* free im memory and reallocate later */
>> bpf_tramp_image_free(im);
>> goto again;
>> }
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index 7f432775a6b5..370f620734cf 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -2020,8 +2020,6 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
>> if (is_ipmodify)
>> goto rollback;
>> 
>> - FTRACE_WARN_ON(rec->flags & FTRACE_FL_DIRECT);
> 
> why is this needed?

This is needed for the modify_ftrace_direct case, because 
the record already have a direct function (BPF trampoline)
attached. 

Thanks,
Song



^ permalink raw reply

* Re: [PATCH bpf-next 1/3] ftrace: Fix BPF fexit with livepatch
From: Song Liu @ 2025-10-24 15:42 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Song Liu, bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	live-patching@vger.kernel.org, ast@kernel.org,
	daniel@iogearbox.net, andrii@kernel.org, rostedt@goodmis.org,
	andrey.grodzovsky@crowdstrike.com, mhiramat@kernel.org,
	Kernel Team, stable@vger.kernel.org
In-Reply-To: <aPtmOJ9jY3bGPvEq@krava>



> On Oct 24, 2025, at 4:42 AM, Jiri Olsa <olsajiri@gmail.com> wrote:
> 
> On Fri, Oct 24, 2025 at 12:12:55AM -0700, Song Liu wrote:
>> When livepatch is attached to the same function as bpf trampoline with
>> a fexit program, bpf trampoline code calls register_ftrace_direct()
>> twice. The first time will fail with -EAGAIN, and the second time it
>> will succeed. This requires register_ftrace_direct() to unregister
>> the address on the first attempt. Otherwise, the bpf trampoline cannot
>> attach. Here is an easy way to reproduce this issue:
>> 
>>  insmod samples/livepatch/livepatch-sample.ko
>>  bpftrace -e 'fexit:cmdline_proc_show {}'
>>  ERROR: Unable to attach probe: fexit:vmlinux:cmdline_proc_show...
>> 
>> Fix this by cleaning up the hash when register_ftrace_function_nolock hits
>> errors.
>> 
>> Fixes: d05cb470663a ("ftrace: Fix modification of direct_function hash while in use")
>> Cc: stable@vger.kernel.org # v6.6+
>> Reported-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
>> Closes: https://lore.kernel.org/live-patching/c5058315a39d4615b333e485893345be@crowdstrike.com/ 
>> Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
>> Cc: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>> Acked-and-tested-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
>> Signed-off-by: Song Liu <song@kernel.org>
>> ---
>> kernel/trace/ftrace.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> 
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index 42bd2ba68a82..7f432775a6b5 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -6048,6 +6048,8 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>> ops->direct_call = addr;
>> 
>> err = register_ftrace_function_nolock(ops);
>> + if (err)
>> + remove_direct_functions_hash(hash, addr);
> 
> should this be handled by the caller of the register_ftrace_direct?
> fops->hash is updated by ftrace_set_filter_ip in register_fentry

We need to clean up here. This is because register_ftrace_direct added 
the new entries to direct_functions. It need to clean these entries 
for the caller so that the next call of register_ftrace_direct can 
work. 

> seems like it's should be caller responsibility, also you could do that
> just for (err == -EAGAIN) case to address the use case directly

The cleanup is valid for any error cases, as we need to remove unused
entries from direct_functions. 

Thanks,
Song




^ permalink raw reply

* Re: [PATCH bpf-next 2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
From: Jiri Olsa @ 2025-10-24 11:43 UTC (permalink / raw)
  To: Song Liu
  Cc: bpf, linux-trace-kernel, live-patching, ast, daniel, andrii,
	rostedt, andrey.grodzovsky, mhiramat, kernel-team
In-Reply-To: <20251024071257.3956031-3-song@kernel.org>

On Fri, Oct 24, 2025 at 12:12:56AM -0700, Song Liu wrote:
> ftrace_hash_ipmodify_enable() checks IPMODIFY and DIRECT ftrace_ops on
> the same kernel function. When needed, ftrace_hash_ipmodify_enable()
> calls ops->ops_func() to prepare the direct ftrace (BPF trampoline) to
> share the same function as the IPMODIFY ftrace (livepatch).
> 
> ftrace_hash_ipmodify_enable() is called in register_ftrace_direct() path,
> but not called in modify_ftrace_direct() path. As a result, the following
> operations will break livepatch:
> 
> 1. Load livepatch to a kernel function;
> 2. Attach fentry program to the kernel function;
> 3. Attach fexit program to the kernel function.
> 
> After 3, the kernel function being used will not be the livepatched
> version, but the original version.
> 
> Fix this by adding ftrace_hash_ipmodify_enable() to modify_ftrace_direct()
> and adjust some logic around the call.
> 
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  kernel/bpf/trampoline.c | 12 +++++++-----
>  kernel/trace/ftrace.c   | 12 ++++++++++--
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 5949095e51c3..8015f5dc3169 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -221,6 +221,13 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
>  
>  	if (tr->func.ftrace_managed) {
>  		ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
> +		/*
> +		 * Clearing fops->trampoline_mutex and fops->NULL is

s/trampoline_mutex/trampoline/

> +		 * needed by the "goto again" case in
> +		 * bpf_trampoline_update().
> +		 */
> +		tr->fops->trampoline = 0;
> +		tr->fops->func = NULL;

IIUC you move this because if modify_fentry returns -EAGAIN
we don't want to reset the trampoline, right?

>  		ret = register_ftrace_direct(tr->fops, (long)new_addr);
>  	} else {
>  		ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
> @@ -479,11 +486,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>  		 * BPF_TRAMP_F_SHARE_IPMODIFY is set, we can generate the
>  		 * trampoline again, and retry register.
>  		 */
> -		/* reset fops->func and fops->trampoline for re-register */
> -		tr->fops->func = NULL;
> -		tr->fops->trampoline = 0;
> -
> -		/* free im memory and reallocate later */
>  		bpf_tramp_image_free(im);
>  		goto again;
>  	}
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 7f432775a6b5..370f620734cf 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2020,8 +2020,6 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
>  				if (is_ipmodify)
>  					goto rollback;
>  
> -				FTRACE_WARN_ON(rec->flags & FTRACE_FL_DIRECT);

why is this needed?

thanks,
jirka

> -
>  				/*
>  				 * Another ops with IPMODIFY is already
>  				 * attached. We are now attaching a direct
> @@ -6128,6 +6126,15 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>  	if (err)
>  		return err;
>  
> +	/*
> +	 * Call ftrace_hash_ipmodify_enable() here, so that we can call
> +	 * ops->ops_func for the ops. This is needed because the above
> +	 * register_ftrace_function_nolock() worked on tmp_ops.
> +	 */
> +	err = ftrace_hash_ipmodify_enable(ops);
> +	if (err)
> +		goto out;
> +
>  	/*
>  	 * Now the ftrace_ops_list_func() is called to do the direct callers.
>  	 * We can safely change the direct functions attached to each entry.
> @@ -6149,6 +6156,7 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>  
>  	mutex_unlock(&ftrace_lock);
>  
> +out:
>  	/* Removing the tmp_ops will add the updated direct callers to the functions */
>  	unregister_ftrace_function(&tmp_ops);
>  
> -- 
> 2.47.3
> 
> 

^ permalink raw reply

* Re: [PATCH bpf-next 1/3] ftrace: Fix BPF fexit with livepatch
From: Jiri Olsa @ 2025-10-24 11:42 UTC (permalink / raw)
  To: Song Liu
  Cc: bpf, linux-trace-kernel, live-patching, ast, daniel, andrii,
	rostedt, andrey.grodzovsky, mhiramat, kernel-team, stable
In-Reply-To: <20251024071257.3956031-2-song@kernel.org>

On Fri, Oct 24, 2025 at 12:12:55AM -0700, Song Liu wrote:
> When livepatch is attached to the same function as bpf trampoline with
> a fexit program, bpf trampoline code calls register_ftrace_direct()
> twice. The first time will fail with -EAGAIN, and the second time it
> will succeed. This requires register_ftrace_direct() to unregister
> the address on the first attempt. Otherwise, the bpf trampoline cannot
> attach. Here is an easy way to reproduce this issue:
> 
>   insmod samples/livepatch/livepatch-sample.ko
>   bpftrace -e 'fexit:cmdline_proc_show {}'
>   ERROR: Unable to attach probe: fexit:vmlinux:cmdline_proc_show...
> 
> Fix this by cleaning up the hash when register_ftrace_function_nolock hits
> errors.
> 
> Fixes: d05cb470663a ("ftrace: Fix modification of direct_function hash while in use")
> Cc: stable@vger.kernel.org # v6.6+
> Reported-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
> Closes: https://lore.kernel.org/live-patching/c5058315a39d4615b333e485893345be@crowdstrike.com/
> Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
> Cc: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Acked-and-tested-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  kernel/trace/ftrace.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 42bd2ba68a82..7f432775a6b5 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6048,6 +6048,8 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>  	ops->direct_call = addr;
>  
>  	err = register_ftrace_function_nolock(ops);
> +	if (err)
> +		remove_direct_functions_hash(hash, addr);

should this be handled by the caller of the register_ftrace_direct?
fops->hash is updated by ftrace_set_filter_ip in register_fentry

seems like it's should be caller responsibility, also you could do that
just for (err == -EAGAIN) case to address the use case directly

jirka

>  
>   out_unlock:
>  	mutex_unlock(&direct_mutex);
> -- 
> 2.47.3
> 
> 

^ permalink raw reply

* [PATCH bpf-next 3/3] selftests/bpf: Add tests for livepatch + bpf trampoline
From: Song Liu @ 2025-10-24  7:12 UTC (permalink / raw)
  To: bpf, linux-trace-kernel, live-patching
  Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
	kernel-team, Song Liu
In-Reply-To: <20251024071257.3956031-1-song@kernel.org>

Both livepatch and BPF trampoline use ftrace. Special attention is needed
when livepatch and fexit program touch the same function at the same
time, because livepatch updates a kernel function and the BPF trampoline
need to call into the right version of the kernel function.

Use samples/livepatch/livepatch-sample.ko for the test.

The test covers two cases:
  1) When a fentry program is loaded first. This exercises the
     modify_ftrace_direct code path.
  2) When a fentry program is loaded first. This exercises the
     register_ftrace_direct code path.

Signed-off-by: Song Liu <song@kernel.org>
---
 tools/testing/selftests/bpf/config            |   3 +
 .../bpf/prog_tests/livepatch_trampoline.c     | 113 ++++++++++++++++++
 .../bpf/progs/livepatch_trampoline.c          |  30 +++++
 3 files changed, 146 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
 create mode 100644 tools/testing/selftests/bpf/progs/livepatch_trampoline.c

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 70b28c1e653e..f2a2fd236ca8 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -50,6 +50,7 @@ CONFIG_IPV6_SIT=y
 CONFIG_IPV6_TUNNEL=y
 CONFIG_KEYS=y
 CONFIG_LIRC=y
+CONFIG_LIVEPATCH=y
 CONFIG_LWTUNNEL=y
 CONFIG_MODULE_SIG=y
 CONFIG_MODULE_SRCVERSION_ALL=y
@@ -111,6 +112,8 @@ CONFIG_IP6_NF_FILTER=y
 CONFIG_NF_NAT=y
 CONFIG_PACKET=y
 CONFIG_RC_CORE=y
+CONFIG_SAMPLES=y
+CONFIG_SAMPLE_LIVEPATCH=m
 CONFIG_SECURITY=y
 CONFIG_SECURITYFS=y
 CONFIG_SYN_COOKIES=y
diff --git a/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c b/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
new file mode 100644
index 000000000000..6326f957dbcd
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <test_progs.h>
+#include "testing_helpers.h"
+#include "livepatch_trampoline.skel.h"
+
+static int load_livepatch(void)
+{
+	const char *livepatch_paths[] = {
+		"../../../../samples/livepatch/livepatch-sample.ko",
+		/* This is the path used by CI */
+		"/tmp/work/bpf/bpf/kbuild-output/samples/livepatch/livepatch-sample.ko",
+	};
+	int ret, i;
+
+	for (i = 0; i < sizeof(livepatch_paths) / sizeof(char *); i++) {
+		ret = load_module(livepatch_paths[i], env_verbosity > VERBOSE_NONE);
+		if (!ret)
+			break;
+	}
+	return ret;
+}
+
+static void unload_livepatch(void)
+{
+	/* Disable the livepatch before unloading the module */
+	system("echo 0 > /sys/kernel/livepatch/livepatch_sample/enabled");
+
+	unload_module("livepatch_sample", env_verbosity > VERBOSE_NONE);
+}
+
+static void read_proc_cmdline(void)
+{
+	char buf[4096];
+	int fd, ret;
+
+	fd = open("/proc/cmdline", O_RDONLY);
+	if (!ASSERT_OK_FD(fd, "open /proc/cmdline"))
+		return;
+
+	ret = read(fd, buf, sizeof(buf));
+	if (!ASSERT_GT(ret, 0, "read /proc/cmdline"))
+		goto out;
+
+	ASSERT_OK(strncmp(buf, "this has been live patched", 26), "strncmp");
+
+out:
+	close(fd);
+}
+
+static void __test_livepatch_trampoline(bool fexit_first)
+{
+	struct livepatch_trampoline *skel = NULL;
+	int err;
+
+	skel = livepatch_trampoline__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
+		goto out;
+
+	skel->bss->my_pid = getpid();
+
+	if (!fexit_first) {
+		/* fentry program is loaded first by default */
+		err = livepatch_trampoline__attach(skel);
+		if (!ASSERT_OK(err, "skel_attach"))
+			goto out;
+	} else {
+		/* Manually load fexit program first. */
+		skel->links.fexit_cmdline = bpf_program__attach(skel->progs.fexit_cmdline);
+		if (!ASSERT_OK_PTR(skel->links.fexit_cmdline, "attach_fexit"))
+			goto out;
+
+		skel->links.fentry_cmdline = bpf_program__attach(skel->progs.fentry_cmdline);
+		if (!ASSERT_OK_PTR(skel->links.fentry_cmdline, "attach_fentry"))
+			goto out;
+	}
+
+	read_proc_cmdline();
+
+	ASSERT_EQ(skel->bss->fentry_hit, 1, "fentry_hit");
+	ASSERT_EQ(skel->bss->fexit_hit, 1, "fexit_hit");
+out:
+	livepatch_trampoline__destroy(skel);
+}
+
+void test_livepatch_trampoline(void)
+{
+	int retry_cnt = 0;
+
+retry:
+	if (load_livepatch()) {
+		if (retry_cnt) {
+			ASSERT_OK(1, "load_livepatch");
+			goto out;
+		}
+		/*
+		 * Something else (previous run of the same test?) loaded
+		 * the KLP module. Unload the KLP module and retry.
+		 */
+		unload_livepatch();
+		retry_cnt++;
+		goto retry;
+	}
+
+	if (test__start_subtest("fentry_first"))
+		__test_livepatch_trampoline(false);
+
+	if (test__start_subtest("fexit_first"))
+		__test_livepatch_trampoline(true);
+out:
+	unload_livepatch();
+}
diff --git a/tools/testing/selftests/bpf/progs/livepatch_trampoline.c b/tools/testing/selftests/bpf/progs/livepatch_trampoline.c
new file mode 100644
index 000000000000..15579d5bcd91
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/livepatch_trampoline.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+int fentry_hit;
+int fexit_hit;
+int my_pid;
+
+SEC("fentry/cmdline_proc_show")
+int BPF_PROG(fentry_cmdline)
+{
+	if (my_pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
+
+	fentry_hit = 1;
+	return 0;
+}
+
+SEC("fexit/cmdline_proc_show")
+int BPF_PROG(fexit_cmdline)
+{
+	if (my_pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
+
+	fexit_hit = 1;
+	return 0;
+}
-- 
2.47.3


^ permalink raw reply related

* [PATCH bpf-next 2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
From: Song Liu @ 2025-10-24  7:12 UTC (permalink / raw)
  To: bpf, linux-trace-kernel, live-patching
  Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
	kernel-team, Song Liu
In-Reply-To: <20251024071257.3956031-1-song@kernel.org>

ftrace_hash_ipmodify_enable() checks IPMODIFY and DIRECT ftrace_ops on
the same kernel function. When needed, ftrace_hash_ipmodify_enable()
calls ops->ops_func() to prepare the direct ftrace (BPF trampoline) to
share the same function as the IPMODIFY ftrace (livepatch).

ftrace_hash_ipmodify_enable() is called in register_ftrace_direct() path,
but not called in modify_ftrace_direct() path. As a result, the following
operations will break livepatch:

1. Load livepatch to a kernel function;
2. Attach fentry program to the kernel function;
3. Attach fexit program to the kernel function.

After 3, the kernel function being used will not be the livepatched
version, but the original version.

Fix this by adding ftrace_hash_ipmodify_enable() to modify_ftrace_direct()
and adjust some logic around the call.

Signed-off-by: Song Liu <song@kernel.org>
---
 kernel/bpf/trampoline.c | 12 +++++++-----
 kernel/trace/ftrace.c   | 12 ++++++++++--
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 5949095e51c3..8015f5dc3169 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -221,6 +221,13 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
 
 	if (tr->func.ftrace_managed) {
 		ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
+		/*
+		 * Clearing fops->trampoline_mutex and fops->NULL is
+		 * needed by the "goto again" case in
+		 * bpf_trampoline_update().
+		 */
+		tr->fops->trampoline = 0;
+		tr->fops->func = NULL;
 		ret = register_ftrace_direct(tr->fops, (long)new_addr);
 	} else {
 		ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
@@ -479,11 +486,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
 		 * BPF_TRAMP_F_SHARE_IPMODIFY is set, we can generate the
 		 * trampoline again, and retry register.
 		 */
-		/* reset fops->func and fops->trampoline for re-register */
-		tr->fops->func = NULL;
-		tr->fops->trampoline = 0;
-
-		/* free im memory and reallocate later */
 		bpf_tramp_image_free(im);
 		goto again;
 	}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7f432775a6b5..370f620734cf 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2020,8 +2020,6 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
 				if (is_ipmodify)
 					goto rollback;
 
-				FTRACE_WARN_ON(rec->flags & FTRACE_FL_DIRECT);
-
 				/*
 				 * Another ops with IPMODIFY is already
 				 * attached. We are now attaching a direct
@@ -6128,6 +6126,15 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
 	if (err)
 		return err;
 
+	/*
+	 * Call ftrace_hash_ipmodify_enable() here, so that we can call
+	 * ops->ops_func for the ops. This is needed because the above
+	 * register_ftrace_function_nolock() worked on tmp_ops.
+	 */
+	err = ftrace_hash_ipmodify_enable(ops);
+	if (err)
+		goto out;
+
 	/*
 	 * Now the ftrace_ops_list_func() is called to do the direct callers.
 	 * We can safely change the direct functions attached to each entry.
@@ -6149,6 +6156,7 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
 
 	mutex_unlock(&ftrace_lock);
 
+out:
 	/* Removing the tmp_ops will add the updated direct callers to the functions */
 	unregister_ftrace_function(&tmp_ops);
 
-- 
2.47.3


^ permalink raw reply related

* [PATCH bpf-next 1/3] ftrace: Fix BPF fexit with livepatch
From: Song Liu @ 2025-10-24  7:12 UTC (permalink / raw)
  To: bpf, linux-trace-kernel, live-patching
  Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
	kernel-team, Song Liu, stable
In-Reply-To: <20251024071257.3956031-1-song@kernel.org>

When livepatch is attached to the same function as bpf trampoline with
a fexit program, bpf trampoline code calls register_ftrace_direct()
twice. The first time will fail with -EAGAIN, and the second time it
will succeed. This requires register_ftrace_direct() to unregister
the address on the first attempt. Otherwise, the bpf trampoline cannot
attach. Here is an easy way to reproduce this issue:

  insmod samples/livepatch/livepatch-sample.ko
  bpftrace -e 'fexit:cmdline_proc_show {}'
  ERROR: Unable to attach probe: fexit:vmlinux:cmdline_proc_show...

Fix this by cleaning up the hash when register_ftrace_function_nolock hits
errors.

Fixes: d05cb470663a ("ftrace: Fix modification of direct_function hash while in use")
Cc: stable@vger.kernel.org # v6.6+
Reported-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
Closes: https://lore.kernel.org/live-patching/c5058315a39d4615b333e485893345be@crowdstrike.com/
Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
Cc: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-and-tested-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
Signed-off-by: Song Liu <song@kernel.org>
---
 kernel/trace/ftrace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 42bd2ba68a82..7f432775a6b5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6048,6 +6048,8 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
 	ops->direct_call = addr;
 
 	err = register_ftrace_function_nolock(ops);
+	if (err)
+		remove_direct_functions_hash(hash, addr);
 
  out_unlock:
 	mutex_unlock(&direct_mutex);
-- 
2.47.3


^ permalink raw reply related

* [PATCH bpf-next 0/3] Fix ftrace for livepatch + BPF fexit programs
From: Song Liu @ 2025-10-24  7:12 UTC (permalink / raw)
  To: bpf, linux-trace-kernel, live-patching
  Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
	kernel-team, Song Liu

livepatch and BPF trampoline are two special users of ftrace. livepatch
uses ftrace with IPMODIFY flag and BPF trampoline uses ftrace direct
functions. When livepatch and BPF trampoline with fexit programs attach to
the same kernel function, BPF trampoline needs to call into the patched
version of the kernel function.

1/3 and 2/3 of this patchset fix two issues with livepatch + fexit cases,
one in the register_ftrace_direct path, the other in the
modify_ftrace_direct path.

3/3 adds selftests for both cases.

Song Liu (3):
  ftrace: Fix BPF fexit with livepatch
  ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
  selftests/bpf: Add tests for livepatch + bpf trampoline

 kernel/bpf/trampoline.c                       |  12 ++-
 kernel/trace/ftrace.c                         |  14 ++-
 tools/testing/selftests/bpf/config            |   3 +
 .../bpf/prog_tests/livepatch_trampoline.c     | 101 ++++++++++++++++++
 .../bpf/progs/livepatch_trampoline.c          |  30 ++++++
 5 files changed, 153 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
 create mode 100644 tools/testing/selftests/bpf/progs/livepatch_trampoline.c

--
2.47.3

^ permalink raw reply

* Re: [PATCH] module: Fix device table module aliases
From: Venkat @ 2025-10-22 14:32 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: x86, LKML, Petr Mladek, Miroslav Benes, Joe Lawrence,
	live-patching, Song Liu, laokz, Jiri Kosina,
	Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
	Puranjay Mohan, Dylan Hatch, Peter Zijlstra, Alexander Stein,
	Marek Szyprowski, Mark Brown, Cosmin Tanislav
In-Reply-To: <e52ee3edf32874da645a9e037a7d77c69893a22a.1760982784.git.jpoimboe@kernel.org>



> On 20 Oct 2025, at 11:23 PM, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> 
> Commit 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from
> __KBUILD_MODNAME") inadvertently broke module alias generation for
> modules which rely on MODULE_DEVICE_TABLE().
> 
> It removed the "kmod_" prefix from __KBUILD_MODNAME, which caused
> MODULE_DEVICE_TABLE() to generate a symbol name which no longer matched
> the format expected by handle_moddevtable() in scripts/mod/file2alias.c.
> 
> As a result, modpost failed to find the device tables, leading to
> missing module aliases.
> 
> Fix this by explicitly adding the "kmod_" string within the
> MODULE_DEVICE_TABLE() macro itself, restoring the symbol name to the
> format expected by file2alias.c.


IBM CI has also encountered this issue. I have tested this patch and it fixes the same.

Report link: https://lore.kernel.org/all/cdf7c458-b28f-4657-8708-1f820369baa6@linux.ibm.com/

Please add below tags.

Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>

Regards,
Venkat.


> Fixes: 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from __KBUILD_MODNAME")
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Reported-by: Mark Brown <broonie@kernel.org>
> Reported-by: Cosmin Tanislav <demonsingur@gmail.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
> include/linux/module.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index e135cc79aceea..d80c3ea574726 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -251,10 +251,11 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
>  */
> #define __mod_device_table(type, name) \
> __PASTE(__mod_device_table__, \
> + __PASTE(kmod_, \
> __PASTE(__KBUILD_MODNAME, \
> __PASTE(__, \
> __PASTE(type, \
> - __PASTE(__, name)))))
> + __PASTE(__, name))))))
> 
> /* Creates an alias so file2alias.c can find device table. */
> #define MODULE_DEVICE_TABLE(type, name) \
> -- 
> 2.51.0
> 


^ permalink raw reply

* Re: [PATCH] module: Fix device table module aliases
From: Peter Zijlstra @ 2025-10-22 13:18 UTC (permalink / raw)
  To: Mark Brown
  Cc: Josh Poimboeuf, x86, linux-kernel, Petr Mladek, Miroslav Benes,
	Joe Lawrence, live-patching, Song Liu, laokz, Jiri Kosina,
	Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
	Puranjay Mohan, Dylan Hatch, Alexander Stein, Marek Szyprowski,
	Cosmin Tanislav
In-Reply-To: <13384578-a7e7-439a-8f30-387a2cb92680@sirena.org.uk>

On Wed, Oct 22, 2025 at 01:01:40PM +0100, Mark Brown wrote:
> On Mon, Oct 20, 2025 at 10:53:40AM -0700, Josh Poimboeuf wrote:
> 
> > Commit 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from
> > __KBUILD_MODNAME") inadvertently broke module alias generation for
> > modules which rely on MODULE_DEVICE_TABLE().
> 
> It'd be really good to get this fix applied, modules not loading is
> hugely impacting CI coverage for -next - a lot of systems aren't even
> able to get their rootfs due to the drivers for it being built as
> modules.

Oh, this needs to go in tip/objtool/core ? This wasn't immediately
obvious to me.

Let me go queue that then.

^ permalink raw reply

* Re: [PATCH] module: Fix device table module aliases
From: Mark Brown @ 2025-10-22 12:01 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: x86, linux-kernel, Petr Mladek, Miroslav Benes, Joe Lawrence,
	live-patching, Song Liu, laokz, Jiri Kosina,
	Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
	Puranjay Mohan, Dylan Hatch, Peter Zijlstra, Alexander Stein,
	Marek Szyprowski, Cosmin Tanislav
In-Reply-To: <e52ee3edf32874da645a9e037a7d77c69893a22a.1760982784.git.jpoimboe@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

On Mon, Oct 20, 2025 at 10:53:40AM -0700, Josh Poimboeuf wrote:

> Commit 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from
> __KBUILD_MODNAME") inadvertently broke module alias generation for
> modules which rely on MODULE_DEVICE_TABLE().

It'd be really good to get this fix applied, modules not loading is
hugely impacting CI coverage for -next - a lot of systems aren't even
able to get their rootfs due to the drivers for it being built as
modules.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v4 08/63] kbuild: Remove 'kmod_' prefix from __KBUILD_MODNAME
From: Anders Roxell @ 2025-10-22  9:53 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Alexander Stein, x86, linux-kernel, Petr Mladek, Miroslav Benes,
	Joe Lawrence, live-patching, Song Liu, laokz, Jiri Kosina,
	Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
	Puranjay Mohan, Dylan Hatch, Peter Zijlstra, Masahiro Yamada,
	Marek Szyprowski, Mark Brown, Cosmin Tanislav
In-Reply-To: <ycrgjcczkgt6morojzfpkjyg4ehrm5ova2hzzxy2dxv23hhyre@nf5bltmr4lxm>

On 2025-10-20 10:22, Josh Poimboeuf wrote:
> On Mon, Oct 20, 2025 at 02:20:35PM +0200, Alexander Stein wrote:
> > Hi,
> > 
> > Am Mittwoch, 17. September 2025, 18:03:16 CEST schrieb Josh Poimboeuf:
> > > In preparation for the objtool klp diff subcommand, remove the arbitrary
> > > 'kmod_' prefix from __KBUILD_MODNAME and instead add it explicitly in
> > > the __initcall_id() macro.
> > > 
> > > This change supports the standardization of "unique" symbol naming by
> > > ensuring the non-unique portion of the name comes before the unique
> > > part.  That will enable objtool to properly correlate symbols across
> > > builds.
> > > 
> > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > 
> > Starting with this commit 6717e8f91db71 ("kbuild: Remove 'kmod_' prefix
> > from __KBUILD_MODNAME") in next-20251020 I don't get any
> > module aliases anymore.
> > modinfo spi-fsl-dspi.ko returns:
> > > filename:       /work/repo/linux/build_arm64/drivers/spi/spi-fsl-dspi.ko
> > > alias:          platform:fsl-dspi
> > > license:        GPL
> > > description:    Freescale DSPI Controller Driver
> > > depends:        
> > > intree:         Y
> > > name:           spi_fsl_dspi
> > > vermagic:       6.18.0-rc1+ SMP preempt mod_unload modversions aarch64
> > 
> > but it should be like this:
> > > filename:       /work/repo/linux/build_arm64/drivers/spi/spi-fsl-dspi.ko
> > > alias:          platform:fsl-dspi
> > > license:        GPL
> > > description:    Freescale DSPI Controller Driver
> > > alias:          of:N*T*Cnxp,s32g2-dspiC*
> 
> Thanks, this patch broke the MODULE_DEVICE_TABLE() macro, as it no
> longer produces the format expected by scripts/mod/file2alias.c.
> 
> I didn't see this in x86 testing since it doesn't have device tree.
> 
> I will post the following fix shortly:

Tested-by: Anders Roxell <anders.roxell@linaro.org>

When can we expect it?

Cheers,
Anders

> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index e135cc79aceea..d80c3ea574726 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -251,10 +251,11 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
>   */
>  #define __mod_device_table(type, name)	\
>  	__PASTE(__mod_device_table__,	\
> +	__PASTE(kmod_,			\
>  	__PASTE(__KBUILD_MODNAME,	\
>  	__PASTE(__,			\
>  	__PASTE(type,			\
> -	__PASTE(__, name)))))
> +	__PASTE(__, name))))))
>  
>  /* Creates an alias so file2alias.c can find device table. */
>  #define MODULE_DEVICE_TABLE(type, name)					\

^ permalink raw reply

* Re: [PATCH] module: Fix device table module aliases
From: Chen-Yu Tsai @ 2025-10-22  9:52 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: x86, linux-kernel, Petr Mladek, Miroslav Benes, Joe Lawrence,
	live-patching, Song Liu, laokz, Jiri Kosina,
	Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
	Puranjay Mohan, Dylan Hatch, Peter Zijlstra, Alexander Stein,
	Marek Szyprowski, Mark Brown, Cosmin Tanislav
In-Reply-To: <e52ee3edf32874da645a9e037a7d77c69893a22a.1760982784.git.jpoimboe@kernel.org>

On Mon, Oct 20, 2025 at 10:53:40AM -0700, Josh Poimboeuf wrote:
> Commit 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from
> __KBUILD_MODNAME") inadvertently broke module alias generation for
> modules which rely on MODULE_DEVICE_TABLE().
> 
> It removed the "kmod_" prefix from __KBUILD_MODNAME, which caused
> MODULE_DEVICE_TABLE() to generate a symbol name which no longer matched
> the format expected by handle_moddevtable() in scripts/mod/file2alias.c.
> 
> As a result, modpost failed to find the device tables, leading to
> missing module aliases.
> 
> Fix this by explicitly adding the "kmod_" string within the
> MODULE_DEVICE_TABLE() macro itself, restoring the symbol name to the
> format expected by file2alias.c.
> 
> Fixes: 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from __KBUILD_MODNAME")
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Reported-by: Mark Brown <broonie@kernel.org>
> Reported-by: Cosmin Tanislav <demonsingur@gmail.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Tested-by: Chen-Yu Tsai <wenst@chromium.org>

^ permalink raw reply

* Re: [External] Re: Question - Livepatch/Kprobe Coexistence on Ftrace-enabled Functions (Ubuntu kernel based on Linux stable 5.15.30)
From: Song Liu @ 2025-10-21 16:08 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: Song Liu, Petr Mladek, kernel-team@lists.ubuntu.com,
	live-patching@vger.kernel.org, Steven Rostedt
In-Reply-To: <c3ad390e-4320-46bb-bc72-b57ab628bff6@crowdstrike.com>

On Tue, Oct 21, 2025 at 7:15 AM Andrey Grodzovsky
<andrey.grodzovsky@crowdstrike.com> wrote:
[...]
> > commit a8b9cf62ade1bf17261a979fc97e40c2d7842353
> > Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > Date: 1 year, 9 months ago
> > ftrace: Fix DIRECT_CALLS to use SAVE_REGS by default
> >
> > commit bdbddb109c75365d22ec4826f480c5e75869e1cb
> > Author: Petr Pavlu <petr.pavlu@suse.com>
> > Date:   1 year, 8 months ago
> >
> >      tracing: Fix HAVE_DYNAMIC_FTRACE_WITH_REGS ifdef
> >
> > I tried to cherry-pick 60c8971899f3b34ad24857913c0784dab08962f0
> > and a8b9cf62ade1bf17261a979fc97e40c2d7842353, on top of 6.5.13
> > kernel. Then, fentry and fexit both work with livepatch.
>
>
> I see, thanks for testing! Is the reason it breaks so often is because
> this combination of having BPF
> and llivepatch together on a system with intersection on same functions
> as relatively   rate event and
> so regressions go easily unnoticed ? Isn't there any relevant automated
> testing in upstream that checks for
> those types of breaks ?

This case is not being covered because it is the intersection of tracing
and livepatch. I am thinking about adding a BPF selftest to cover this
case.

Thanks,
Song

^ permalink raw reply

* Re: [External] Re: Question - Livepatch/Kprobe Coexistence on Ftrace-enabled Functions (Ubuntu kernel based on Linux stable 5.15.30)
From: Song Liu @ 2025-10-21 16:06 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Song Liu, Andrey Grodzovsky, Petr Mladek,
	kernel-team@lists.ubuntu.com, live-patching@vger.kernel.org
In-Reply-To: <20251021100956.4c64272c@gandalf.local.home>

On Tue, Oct 21, 2025 at 7:09 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon, 20 Oct 2025 23:07:26 -0700
> Song Liu <song@kernel.org> wrote:
>
> > commit a8b9cf62ade1bf17261a979fc97e40c2d7842353
> > Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > Date: 1 year, 9 months ago
> > ftrace: Fix DIRECT_CALLS to use SAVE_REGS by default
>
> Hmm, this is a work around. I wonder if we can make this work with ARGS as
> well? Hmm. I'll have to take a look when I get a chance.

Given both ftrace_caller and arch_prepare_bpf_trampoline are created
per arch, I think it is possible. But I guess it may require quite some work
to update and test each architecture?

Thanks,
Song

^ permalink raw reply

* Re: [External] Re: Question - Livepatch/Kprobe Coexistence on Ftrace-enabled Functions (Ubuntu kernel based on Linux stable 5.15.30)
From: Andrey Grodzovsky @ 2025-10-21 14:15 UTC (permalink / raw)
  To: Song Liu
  Cc: Petr Mladek, kernel-team@lists.ubuntu.com,
	live-patching@vger.kernel.org, Steven Rostedt
In-Reply-To: <CAHzjS_vD1TJkVxN+bf+2srKhH9ajn=BHyvEn7oeu664R481R+g@mail.gmail.com>

On 10/21/25 02:07, Song Liu wrote:
> On Mon, Oct 20, 2025 at 2:31 PM Andrey Grodzovsky
> <andrey.grodzovsky@crowdstrike.com> wrote:
> [...]
>> Song, I identified another issue in pre 6.6 kernel, building
>> ~/linux-6.5/samples/livepatch/livepatch-sample.c as ko,
>> before insmoding it, bpftrace fentry/fexit fires as expected, after
>> insmod, while no errors reported on attachments,
>> the hooks stop firing, both if attaching before insmod and if attaching
>> after insmod. If i rrmod the ko, existing hooks
>> resume working.
>>
>> ubuntu@ip-10-10-115-238:~$ cat /proc/version_signature
>> Ubuntu 6.5.0-1008.8-aws 6.5.3
>> Source obtained to build the test module for the AWS kernel from the
>> related stable branch -
>> https://urldefense.com/v3/__https://cdn.kernel.org/pub/linux/kernel/v6.x/linux-6.5.tar.xz__;!!BmdzS3_lV9HdKG8!xLmCb7PCwCj7vM8JjKx_n7ZUVjW0Oj8Ih9T3YqU4I-JoGy7evTsc7U17emt3nnDmdXdchXxcKHi_6mVBt5QbKzj2$
>>
>> Let me know what you think.
> I tested various stable kernels. I got:
>
> With livepatch, fentry and fexit work on 6.3 kernels.
>
> On 6.4 and 6.5 kernels, the combination stops working since this commit:
>
> commit 60c8971899f3b34ad24857913c0784dab08962f0
> Author: Florent Revest <revest@chromium.org>
> Date:   2 years, 7 months ago
>
>      ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS
>
>
> On 6.5 kernels, it got fixed by the following two commits:
>
> commit a8b9cf62ade1bf17261a979fc97e40c2d7842353
> Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Date: 1 year, 9 months ago
> ftrace: Fix DIRECT_CALLS to use SAVE_REGS by default
>
> commit bdbddb109c75365d22ec4826f480c5e75869e1cb
> Author: Petr Pavlu <petr.pavlu@suse.com>
> Date:   1 year, 8 months ago
>
>      tracing: Fix HAVE_DYNAMIC_FTRACE_WITH_REGS ifdef
>
> I tried to cherry-pick 60c8971899f3b34ad24857913c0784dab08962f0
> and a8b9cf62ade1bf17261a979fc97e40c2d7842353, on top of 6.5.13
> kernel. Then, fentry and fexit both work with livepatch.


I see, thanks for testing! Is the reason it breaks so often is because 
this combination of having BPF
and llivepatch together on a system with intersection on same functions 
as relatively   rate event and
so regressions go easily unnoticed ? Isn't there any relevant automated 
testing in upstream that checks for
those types of breaks ?

Thanks,
Andrey

>
> Thanks,
> Song



^ permalink raw reply

* Re: [External] Re: Question - Livepatch/Kprobe Coexistence on Ftrace-enabled Functions (Ubuntu kernel based on Linux stable 5.15.30)
From: Steven Rostedt @ 2025-10-21 14:09 UTC (permalink / raw)
  To: Song Liu
  Cc: Andrey Grodzovsky, Petr Mladek, kernel-team@lists.ubuntu.com,
	live-patching@vger.kernel.org
In-Reply-To: <CAHzjS_vD1TJkVxN+bf+2srKhH9ajn=BHyvEn7oeu664R481R+g@mail.gmail.com>

On Mon, 20 Oct 2025 23:07:26 -0700
Song Liu <song@kernel.org> wrote:

> commit a8b9cf62ade1bf17261a979fc97e40c2d7842353
> Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Date: 1 year, 9 months ago
> ftrace: Fix DIRECT_CALLS to use SAVE_REGS by default

Hmm, this is a work around. I wonder if we can make this work with ARGS as
well? Hmm. I'll have to take a look when I get a chance.

-- Steve

^ permalink raw reply

* Re: [PATCH] module: Fix device table module aliases
From: Alexander Stein @ 2025-10-21  6:59 UTC (permalink / raw)
  To: x86, Josh Poimboeuf
  Cc: linux-kernel, Petr Mladek, Miroslav Benes, Joe Lawrence,
	live-patching, Song Liu, laokz, Jiri Kosina,
	Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
	Puranjay Mohan, Dylan Hatch, Peter Zijlstra, Marek Szyprowski,
	Mark Brown, Cosmin Tanislav
In-Reply-To: <e52ee3edf32874da645a9e037a7d77c69893a22a.1760982784.git.jpoimboe@kernel.org>

Am Montag, 20. Oktober 2025, 19:53:40 CEST schrieb Josh Poimboeuf:
> Commit 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from
> __KBUILD_MODNAME") inadvertently broke module alias generation for
> modules which rely on MODULE_DEVICE_TABLE().
> 
> It removed the "kmod_" prefix from __KBUILD_MODNAME, which caused
> MODULE_DEVICE_TABLE() to generate a symbol name which no longer matched
> the format expected by handle_moddevtable() in scripts/mod/file2alias.c.
> 
> As a result, modpost failed to find the device tables, leading to
> missing module aliases.
> 
> Fix this by explicitly adding the "kmod_" string within the
> MODULE_DEVICE_TABLE() macro itself, restoring the symbol name to the
> format expected by file2alias.c.
> 
> Fixes: 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from __KBUILD_MODNAME")
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Reported-by: Mark Brown <broonie@kernel.org>
> Reported-by: Cosmin Tanislav <demonsingur@gmail.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Thanks!

> ---
>  include/linux/module.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index e135cc79aceea..d80c3ea574726 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -251,10 +251,11 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
>   */
>  #define __mod_device_table(type, name)	\
>  	__PASTE(__mod_device_table__,	\
> +	__PASTE(kmod_,			\
>  	__PASTE(__KBUILD_MODNAME,	\
>  	__PASTE(__,			\
>  	__PASTE(type,			\
> -	__PASTE(__, name)))))
> +	__PASTE(__, name))))))
>  
>  /* Creates an alias so file2alias.c can find device table. */
>  #define MODULE_DEVICE_TABLE(type, name)					\
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



^ permalink raw reply

* Re: [External] Re: Question - Livepatch/Kprobe Coexistence on Ftrace-enabled Functions (Ubuntu kernel based on Linux stable 5.15.30)
From: Song Liu @ 2025-10-21  6:07 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: Song Liu, Petr Mladek, kernel-team@lists.ubuntu.com,
	live-patching@vger.kernel.org, Steven Rostedt
In-Reply-To: <07ab2111-0f41-40cb-aeb1-d9d3463b1a6a@crowdstrike.com>

On Mon, Oct 20, 2025 at 2:31 PM Andrey Grodzovsky
<andrey.grodzovsky@crowdstrike.com> wrote:
[...]
> Song, I identified another issue in pre 6.6 kernel, building
> ~/linux-6.5/samples/livepatch/livepatch-sample.c as ko,
> before insmoding it, bpftrace fentry/fexit fires as expected, after
> insmod, while no errors reported on attachments,
> the hooks stop firing, both if attaching before insmod and if attaching
> after insmod. If i rrmod the ko, existing hooks
> resume working.
>
> ubuntu@ip-10-10-115-238:~$ cat /proc/version_signature
> Ubuntu 6.5.0-1008.8-aws 6.5.3
> Source obtained to build the test module for the AWS kernel from the
> related stable branch -
> https://cdn.kernel.org/pub/linux/kernel/v6.x/linux-6.5.tar.xz
>
> Let me know what you think.

I tested various stable kernels. I got:

With livepatch, fentry and fexit work on 6.3 kernels.

On 6.4 and 6.5 kernels, the combination stops working since this commit:

commit 60c8971899f3b34ad24857913c0784dab08962f0
Author: Florent Revest <revest@chromium.org>
Date:   2 years, 7 months ago

    ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS


On 6.5 kernels, it got fixed by the following two commits:

commit a8b9cf62ade1bf17261a979fc97e40c2d7842353
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 1 year, 9 months ago
ftrace: Fix DIRECT_CALLS to use SAVE_REGS by default

commit bdbddb109c75365d22ec4826f480c5e75869e1cb
Author: Petr Pavlu <petr.pavlu@suse.com>
Date:   1 year, 8 months ago

    tracing: Fix HAVE_DYNAMIC_FTRACE_WITH_REGS ifdef

I tried to cherry-pick 60c8971899f3b34ad24857913c0784dab08962f0
and a8b9cf62ade1bf17261a979fc97e40c2d7842353, on top of 6.5.13
kernel. Then, fentry and fexit both work with livepatch.

Thanks,
Song

^ permalink raw reply

* Re: [External] Re: Question - Livepatch/Kprobe Coexistence on Ftrace-enabled Functions (Ubuntu kernel based on Linux stable 5.15.30)
From: Andrey Grodzovsky @ 2025-10-20 21:31 UTC (permalink / raw)
  To: Song Liu
  Cc: Petr Mladek, kernel-team@lists.ubuntu.com,
	live-patching@vger.kernel.org, Steven Rostedt
In-Reply-To: <f3f3e753-1014-4fb2-9d6e-328b33c7356f@crowdstrike.com>

On 10/20/25 15:53, Andrey Grodzovsky wrote:
> On 10/20/25 15:10, Andrey Grodzovsky wrote:
>> On 10/20/25 14:53, Song Liu wrote:
>>> On Mon, Oct 20, 2025 at 9:45 AM Andrey Grodzovsky
>>> <andrey.grodzovsky@crowdstrike.com> wrote:
>>>> On 10/20/25 12:03, Song Liu wrote:
>>>>> On Mon, Oct 20, 2025 at 7:56 AM Andrey Grodzovsky
>>>>> <andrey.grodzovsky@crowdstrike.com> wrote:
>>>>> [...]
>>>>>>> If you build the kernel from source code, there are some samples in
>>>>>>> samples/livepatch that you can use for testing. PS: You need to 
>>>>>>> enable
>>>>>>>
>>>>>>>      CONFIG_SAMPLE_LIVEPATCH=m
>>>>>>>
>>>>>>> I hope this helps.
>>>>>> Thanks Song, working on repro, kernel rebuilt, test module is 
>>>>>> loading
>>>>>> but, bpftrace is refusing to attach now to fentries/fexits 
>>>>>> claiming the
>>>>>> costum kernel is not supporting it. It did
>>>>>> attach in the case of stock AWS kernel i copied the .config from. So
>>>>>> just trying to figure out now if some Kcofnig flags are missing or
>>>>>> different . Let me know in case you manage to confirm yourself in 
>>>>>> the
>>>>>> meanwhile the fix works for
>>>>>> you.
>>>>> Yes, it worked in my tests.
>>>>>
>>>>> [root@(none) /]# kpatch load 
>>>>> linux/samples/livepatch/livepatch-sample.ko
>>>>> loading patch module: linux/samples/livepatch/livepatch-sample.ko
>>>>> [root@(none) /]# bpftrace.real -e 'fexit:cmdline_proc_show
>>>>> {printf("fexit\n");}' &
>>>>> [1] 388
>>>>> [root@(none) /]# Attached 1 probe
>>>>> [root@(none) /]# bpftrace.real -e 'fentry:cmdline_proc_show
>>>>> {printf("fentry\n");}' &
>>>>> [2] 397
>>>>> [root@(none) /]# Attached 1 probe
>>>>>
>>>>> [root@(none) /]# cat /proc/cmdline
>>>>> this has been live patched
>>>>> fentry
>>>>> fexit
>>>>>
>>>>> Thanks,
>>>>> Song
>>>>>
>>>> Verified the failures I observe when trying to attach with BPF 
>>>> trace are
>>>> only in presence of patch you provided.
>>>> Please see attached dmesg for failures. Initial warning on boot.
>>>> Subsequebt warnings and errors at the point i try to run
>>>> sudo bpftrace -e "fexit:cmdline_proc_show { printf(\"fexit hit\\n\");
>>>> exit(); }"
>>>>
>>>> sudo: unable to resolve host ip-10-10-115-238: Temporary failure in 
>>>> name
>>>> resolution
>>>> stdin:1:1-25: ERROR: kfunc/kretfunc not available for your kernel 
>>>> version.
>>>>
>>>> ubuntu@ip-10-10-115-238:~/linux-6.8.1$ sudo cat
>>>> /sys/kernel/debug/tracing/available_filter_functions | grep
>>>> cmdline_proc_show
>>>> sudo: unable to resolve host ip-10-10-115-238: Temporary failure in 
>>>> name
>>>> resolution
>>>> cat: /sys/kernel/debug/tracing/available_filter_functions: No such 
>>>> device
>>>>
>>>> After reboot and before trying to attacg with bpftrace,
>>>> /sys/kernel/debug/tracing/available_filter_functions is available and
>>>> shows all function.
>>>>
>>>> Using stable kernel from
>>>> https://urldefense.com/v3/__https://cdn.kernel.org/pub/linux/kernel/v6.x/linux-6.8.1.tar.gz__;!!BmdzS3_lV9HdKG8!1ZJe4jY49_xIzp4h4i4AbqpkLKoAqrXLFX2wDxhoSUDg2kSeTjy3COy9MngNDRlZhJ1oUKgf1yPqmnTY9-Y50TkA$ 
>>>> for build.
>>>> FTRACE related KCONFIGs bellow
>>> I can see the similar issue with the upstream kernel. I was testing on
>>> stable 6.17 before just know because of another issue with upstream
>>> kernel, and somehow 6.17 kernel doesn't seem to have the issue.
>>>
>>> To fix this, I think we should land a fix similar to the earlier diff:
>>>
>>> diff --git i/kernel/trace/ftrace.c w/kernel/trace/ftrace.c
>>> index 42bd2ba68a82..8f320df0ac52 100644
>>> --- i/kernel/trace/ftrace.c
>>> +++ w/kernel/trace/ftrace.c
>>> @@ -6049,6 +6049,9 @@ int register_ftrace_direct(struct ftrace_ops
>>> *ops, unsigned long addr)
>>>
>>>          err = register_ftrace_function_nolock(ops);
>>>
>>> +       if (err)
>>> +               remove_direct_functions_hash(hash, addr);
>>> +
>>>    out_unlock:
>>>          mutex_unlock(&direct_mutex);
>>>
>>>
>>> Steven,
>>>
>>> Does this change look good to you?
>>>
>>>
>>
>> Seems reasonable to me, we are simply cleaning the entry on failure 
>> so we don't encounter it late anymore.
>> So I will apply this patch ONLY and retest - correct ?
>>
>> Another question - it seems you found where it broke ? I saw 'Cc: 
>> stable@vger.kernel.org # v6.6+' in your prev. patch.'
>> If so , can you please point me to the offending patch so I add this 
>> to my records of my discovery work of bpf coexistence
>> livepatching ?
>>
>> Thanks,
>>
>> Andrey
>
>
> Update - with latest fix work find, both after loading the livepatch 
> .ko  no conflicts and hooks work and,
> before loading it, pre exsisting hooks keep working after the patch is 
> loaded
>
> You can add Acked-and-tested-by: Andrey Grodzovsky 
> <andrey.grodzovsky@crowdstrike.com>
>
> Once again, in case you now the exact commit that broke it, please let 
> me know.
>
> Thanks,
> Andrey


Song, I identified another issue in pre 6.6 kernel, building 
~/linux-6.5/samples/livepatch/livepatch-sample.c as ko,
before insmoding it, bpftrace fentry/fexit fires as expected, after 
insmod, while no errors reported on attachments,
the hooks stop firing, both if attaching before insmod and if attaching 
after insmod. If i rrmod the ko, existing hooks
resume working.

ubuntu@ip-10-10-115-238:~$ cat /proc/version_signature
Ubuntu 6.5.0-1008.8-aws 6.5.3
Source obtained to build the test module for the AWS kernel from the 
related stable branch - 
https://cdn.kernel.org/pub/linux/kernel/v6.x/linux-6.5.tar.xz

Let me know what you think.

Thanks,
Andrey



>
>>
>>>
>>> Thanks,
>>> Song
>>
>>
>


^ permalink raw reply

* Re: [PATCH] module: Fix device table module aliases
From: Mark Brown @ 2025-10-20 20:55 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: x86, linux-kernel, Petr Mladek, Miroslav Benes, Joe Lawrence,
	live-patching, Song Liu, laokz, Jiri Kosina,
	Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
	Puranjay Mohan, Dylan Hatch, Peter Zijlstra, Alexander Stein,
	Marek Szyprowski, Cosmin Tanislav
In-Reply-To: <e52ee3edf32874da645a9e037a7d77c69893a22a.1760982784.git.jpoimboe@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

On Mon, Oct 20, 2025 at 10:53:40AM -0700, Josh Poimboeuf wrote:
> Commit 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from
> __KBUILD_MODNAME") inadvertently broke module alias generation for
> modules which rely on MODULE_DEVICE_TABLE().
> 
> It removed the "kmod_" prefix from __KBUILD_MODNAME, which caused
> MODULE_DEVICE_TABLE() to generate a symbol name which no longer matched
> the format expected by handle_moddevtable() in scripts/mod/file2alias.c.

I've not done an exhaustive test of all the impacted platforms but this
looks like it fixes things:

Tested-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH] module: Fix device table module aliases
From: Marek Szyprowski @ 2025-10-20 20:40 UTC (permalink / raw)
  To: Josh Poimboeuf, x86
  Cc: linux-kernel, Petr Mladek, Miroslav Benes, Joe Lawrence,
	live-patching, Song Liu, laokz, Jiri Kosina,
	Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
	Puranjay Mohan, Dylan Hatch, Peter Zijlstra, Alexander Stein,
	Mark Brown, Cosmin Tanislav
In-Reply-To: <e52ee3edf32874da645a9e037a7d77c69893a22a.1760982784.git.jpoimboe@kernel.org>

On 20.10.2025 19:53, Josh Poimboeuf wrote:
> Commit 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from
> __KBUILD_MODNAME") inadvertently broke module alias generation for
> modules which rely on MODULE_DEVICE_TABLE().
>
> It removed the "kmod_" prefix from __KBUILD_MODNAME, which caused
> MODULE_DEVICE_TABLE() to generate a symbol name which no longer matched
> the format expected by handle_moddevtable() in scripts/mod/file2alias.c.
>
> As a result, modpost failed to find the device tables, leading to
> missing module aliases.
>
> Fix this by explicitly adding the "kmod_" string within the
> MODULE_DEVICE_TABLE() macro itself, restoring the symbol name to the
> format expected by file2alias.c.
>
> Fixes: 6717e8f91db7 ("kbuild: Remove 'kmod_' prefix from __KBUILD_MODNAME")
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Reported-by: Mark Brown <broonie@kernel.org>
> Reported-by: Cosmin Tanislav <demonsingur@gmail.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

This fixes the issue observed on my test systems. Thanks!

Closes: 
https://lore.kernel.org/all/d7b49971-8d77-43e2-b0cd-a70c6463ea57@samsung.com/
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>


> ---
>   include/linux/module.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index e135cc79aceea..d80c3ea574726 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -251,10 +251,11 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
>    */
>   #define __mod_device_table(type, name)	\
>   	__PASTE(__mod_device_table__,	\
> +	__PASTE(kmod_,			\
>   	__PASTE(__KBUILD_MODNAME,	\
>   	__PASTE(__,			\
>   	__PASTE(type,			\
> -	__PASTE(__, name)))))
> +	__PASTE(__, name))))))
>   
>   /* Creates an alias so file2alias.c can find device table. */
>   #define MODULE_DEVICE_TABLE(type, name)					\

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply

* Re: [External] Re: Question - Livepatch/Kprobe Coexistence on Ftrace-enabled Functions (Ubuntu kernel based on Linux stable 5.15.30)
From: Steven Rostedt @ 2025-10-20 20:31 UTC (permalink / raw)
  To: Song Liu
  Cc: Andrey Grodzovsky, Petr Mladek, kernel-team@lists.ubuntu.com,
	live-patching@vger.kernel.org
In-Reply-To: <CAHzjS_tuotYQQ0HmncVp=oFOfcyxmYqCds0MDBMOr5FC5KzhSA@mail.gmail.com>

On Mon, 20 Oct 2025 11:53:34 -0700
Song Liu <song@kernel.org> wrote:

> diff --git i/kernel/trace/ftrace.c w/kernel/trace/ftrace.c
> index 42bd2ba68a82..8f320df0ac52 100644
> --- i/kernel/trace/ftrace.c
> +++ w/kernel/trace/ftrace.c
> @@ -6049,6 +6049,9 @@ int register_ftrace_direct(struct ftrace_ops
> *ops, unsigned long addr)
> 
>         err = register_ftrace_function_nolock(ops);
> 
> +       if (err)
> +               remove_direct_functions_hash(hash, addr);
> +
>   out_unlock:
>         mutex_unlock(&direct_mutex);
> 
> 
> Steven,
> 
> Does this change look good to you?

With the small nit that you don't need a space between the err = and the if (err).

But yeah, looks fine to me.

-- Steve

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox