* [PATCH 5.10/5.15 0/1] kprobes: Fix to handle forcibly unoptimized kprobes on freeing_list
@ 2024-01-22 11:30 Fedor Pchelkin
2024-01-22 11:30 ` [PATCH 5.10/5.15 1/1] " Fedor Pchelkin
0 siblings, 1 reply; 3+ messages in thread
From: Fedor Pchelkin @ 2024-01-22 11:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, stable
Cc: Fedor Pchelkin, Masami Hiramatsu, Steven Rostedt (Google),
linux-kernel, lvc-project
The `WARNING in kprobe_optimizer` is reproduced on 5.10/5.15 stable
branches.
The problem ([1], link included in original commit description) has been
fixed by the following patch which can be cleanly applied to 5.10/5.15.
The fix is already present in all stable branches starting from 6.1.
Link to the "failed to apply to 5.15" report [2].
Link to the "failed to apply to 5.10" report [3].
[1]: https://lore.kernel.org/all/Y8URdIfVr3pq2X8w@xpf.sh.intel.com/
[2]: https://lore.kernel.org/stable/16781827629218@kroah.com/
[3]: https://lore.kernel.org/stable/16781827635613@kroah.com/
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 5.10/5.15 1/1] kprobes: Fix to handle forcibly unoptimized kprobes on freeing_list
2024-01-22 11:30 [PATCH 5.10/5.15 0/1] kprobes: Fix to handle forcibly unoptimized kprobes on freeing_list Fedor Pchelkin
@ 2024-01-22 11:30 ` Fedor Pchelkin
2024-01-22 17:47 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Fedor Pchelkin @ 2024-01-22 11:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, stable
Cc: Fedor Pchelkin, Masami Hiramatsu, Steven Rostedt (Google),
linux-kernel, lvc-project, Pengfei Xu
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
commit 4fbd2f83fda0ca44a2ec6421ca3508b355b31858 upstream.
Since forcibly unoptimized kprobes will be put on the freeing_list directly
in the unoptimize_kprobe(), do_unoptimize_kprobes() must continue to check
the freeing_list even if unoptimizing_list is empty.
This bug can happen if a kprobe is put in an instruction which is in the
middle of the jump-replaced instruction sequence of an optprobe, *and* the
optprobe is recently unregistered and queued on unoptimizing_list.
In this case, the optprobe will be unoptimized forcibly (means immediately)
and put it into the freeing_list, expecting the optprobe will be handled in
do_unoptimize_kprobe().
But if there is no other optprobes on the unoptimizing_list, current code
returns from the do_unoptimize_kprobe() soon and does not handle the
optprobe which is on the freeing_list. Then the optprobe will hit the
WARN_ON_ONCE() in the do_free_cleaned_kprobes(), because it is not handled
in the latter loop of the do_unoptimize_kprobe().
To solve this issue, do not return from do_unoptimize_kprobes() immediately
even if unoptimizing_list is empty.
Moreover, this change affects another case. kill_optimized_kprobes() expects
kprobe_optimizer() will just free the optprobe on freeing_list.
So I changed it to just do list_move() to freeing_list if optprobes are on
unoptimizing list. And the do_unoptimize_kprobe() will skip
arch_disarm_kprobe() if the probe on freeing_list has gone flag.
Link: https://lore.kernel.org/all/Y8URdIfVr3pq2X8w@xpf.sh.intel.com/
Link: https://lore.kernel.org/all/167448024501.3253718.13037333683110512967.stgit@devnote3/
Fixes: e4add247789e ("kprobes: Fix optimize_kprobe()/unoptimize_kprobe() cancellation logic")
Reported-by: Pengfei Xu <pengfei.xu@intel.com>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
[fp: adjust comment conflict regarding commit 223a76b268c9 ("kprobes: Fix
coding style issues")]
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
kernel/kprobes.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 07d36cee2a80..5d713a7d7e87 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -549,17 +549,15 @@ static void do_unoptimize_kprobes(void)
/* See comment in do_optimize_kprobes() */
lockdep_assert_cpus_held();
- /* Unoptimization must be done anytime */
- if (list_empty(&unoptimizing_list))
- return;
+ if (!list_empty(&unoptimizing_list))
+ arch_unoptimize_kprobes(&unoptimizing_list, &freeing_list);
- arch_unoptimize_kprobes(&unoptimizing_list, &freeing_list);
- /* Loop free_list for disarming */
+ /* Loop on 'freeing_list' for disarming and removing from kprobe hash list */
list_for_each_entry_safe(op, tmp, &freeing_list, list) {
/* Switching from detour code to origin */
op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
- /* Disarm probes if marked disabled */
- if (kprobe_disabled(&op->kp))
+ /* Disarm probes if marked disabled and not gone */
+ if (kprobe_disabled(&op->kp) && !kprobe_gone(&op->kp))
arch_disarm_kprobe(&op->kp);
if (kprobe_unused(&op->kp)) {
/*
@@ -788,14 +786,13 @@ static void kill_optimized_kprobe(struct kprobe *p)
op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
if (kprobe_unused(p)) {
- /* Enqueue if it is unused */
- list_add(&op->list, &freeing_list);
/*
- * Remove unused probes from the hash list. After waiting
- * for synchronization, this probe is reclaimed.
- * (reclaiming is done by do_free_cleaned_kprobes().)
+ * Unused kprobe is on unoptimizing or freeing list. We move it
+ * to freeing_list and let the kprobe_optimizer() remove it from
+ * the kprobe hash list and free it.
*/
- hlist_del_rcu(&op->kp.hlist);
+ if (optprobe_queued_unopt(op))
+ list_move(&op->list, &freeing_list);
}
/* Don't touch the code, because it is already freed. */
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 5.10/5.15 1/1] kprobes: Fix to handle forcibly unoptimized kprobes on freeing_list
2024-01-22 11:30 ` [PATCH 5.10/5.15 1/1] " Fedor Pchelkin
@ 2024-01-22 17:47 ` Greg KH
0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2024-01-22 17:47 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: stable, Masami Hiramatsu, Steven Rostedt (Google),
linux-kernel, lvc-project, Pengfei Xu
On Mon, Jan 22, 2024 at 02:30:31PM +0300, Fedor Pchelkin wrote:
> From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
>
> commit 4fbd2f83fda0ca44a2ec6421ca3508b355b31858 upstream.
>
> Since forcibly unoptimized kprobes will be put on the freeing_list directly
> in the unoptimize_kprobe(), do_unoptimize_kprobes() must continue to check
> the freeing_list even if unoptimizing_list is empty.
>
> This bug can happen if a kprobe is put in an instruction which is in the
> middle of the jump-replaced instruction sequence of an optprobe, *and* the
> optprobe is recently unregistered and queued on unoptimizing_list.
> In this case, the optprobe will be unoptimized forcibly (means immediately)
> and put it into the freeing_list, expecting the optprobe will be handled in
> do_unoptimize_kprobe().
> But if there is no other optprobes on the unoptimizing_list, current code
> returns from the do_unoptimize_kprobe() soon and does not handle the
> optprobe which is on the freeing_list. Then the optprobe will hit the
> WARN_ON_ONCE() in the do_free_cleaned_kprobes(), because it is not handled
> in the latter loop of the do_unoptimize_kprobe().
>
> To solve this issue, do not return from do_unoptimize_kprobes() immediately
> even if unoptimizing_list is empty.
>
> Moreover, this change affects another case. kill_optimized_kprobes() expects
> kprobe_optimizer() will just free the optprobe on freeing_list.
> So I changed it to just do list_move() to freeing_list if optprobes are on
> unoptimizing list. And the do_unoptimize_kprobe() will skip
> arch_disarm_kprobe() if the probe on freeing_list has gone flag.
>
> Link: https://lore.kernel.org/all/Y8URdIfVr3pq2X8w@xpf.sh.intel.com/
> Link: https://lore.kernel.org/all/167448024501.3253718.13037333683110512967.stgit@devnote3/
>
> Fixes: e4add247789e ("kprobes: Fix optimize_kprobe()/unoptimize_kprobe() cancellation logic")
> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Cc: stable@vger.kernel.org
> Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> [fp: adjust comment conflict regarding commit 223a76b268c9 ("kprobes: Fix
> coding style issues")]
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> ---
> kernel/kprobes.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
Now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-01-22 17:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-22 11:30 [PATCH 5.10/5.15 0/1] kprobes: Fix to handle forcibly unoptimized kprobes on freeing_list Fedor Pchelkin
2024-01-22 11:30 ` [PATCH 5.10/5.15 1/1] " Fedor Pchelkin
2024-01-22 17:47 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.