Messages in this thread | | | Date | Sat, 4 Jun 2022 00:03:12 +0900 | From | Masami Hiramatsu (Google) <> | Subject | Re: [PATCH] kprobes: Rollback kprobe flags on failed arm_kprobe |
| |
Hi Chuang,
On Thu, 2 Jun 2022 15:32:59 +0800 Chuang <nashuiliang@gmail.com> wrote:
> From: Chuang Wang <nashuiliang@gmail.com> > > In aggrprobe scenes, if arm_kprobe() returns an error(e.g. livepatch and > kprobe are using the same function X), kprobe flags, while has been > modified to ~KPROBE_FLAG_DISABLED, is not rollled back. > > Then, __disable_kprobe() will be failed in __unregister_kprobe_top(), > the kprobe list will be not removed from aggrprobe, memory leaks or > illegal pointers will be caused. > > WARN disarm_kprobe: > Failed to disarm kprobe-ftrace at 00000000c729fdbc (-2) > RIP: 0010:disarm_kprobe+0xcc/0x110 > Call Trace: > __disable_kprobe+0x78/0x90 > __unregister_kprobe_top+0x13/0x1b0 > ? _cond_resched+0x15/0x30 > unregister_kprobes+0x32/0x80 > unregister_kprobe+0x1a/0x20 > > Illegal Pointers: > BUG: unable to handle kernel paging request at 0000000000656369 > RIP: 0010:__get_valid_kprobe+0x69/0x90 > Call Trace: > register_kprobe+0x30/0x60 > __register_trace_kprobe.part.7+0x8b/0xc0 > create_local_trace_kprobe+0xd2/0x130 > perf_kprobe_init+0x83/0xd0
Oops, thanks for reporting!
> Signed-off-by: Chuang Wang <nashuiliang@gmail.com> > Signed-off-by: Jingren Zhou <zhoujingren@didiglobal.com>
This should go to stable, so add below tag. (No need to CC to stable)
Fixes: 12310e343755 ("kprobes: Propagate error from arm_kprobe_ftrace()") Cc: stable@vger.kernel.org
And could you also update this patch as below?
> --- > kernel/kprobes.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index f214f8c088ed..96c75e23113c 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -2422,8 +2422,10 @@ int enable_kprobe(struct kprobe *kp) > if (!kprobes_all_disarmed && kprobe_disabled(p)) { > p->flags &= ~KPROBE_FLAG_DISABLED; > ret = arm_kprobe(p); > - if (ret) > + if (ret) { > p->flags |= KPROBE_FLAG_DISABLED;
Here, can you add a check?
if (p != kp)
> + kp->flags |= KPROBE_FLAG_DISABLED;
Thus is is clear that this is corresponding to --- if (p != kp) kp->flags &= ~KPROBE_FLAG_DISABLED; ---
Thank you,
> + } > } > out: > mutex_unlock(&kprobe_mutex); > -- > 2.34.1 >
-- Masami Hiramatsu (Google) <mhiramat@kernel.org>
| |