lkml.org 
[lkml]   [2022]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH] kprobes: Rollback post_handler on failed arm_kprobe()
On Wed, Jun 15, 2022 at 8:34 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi Chuang,
>
> On Tue, 14 Jun 2022 17:06:33 +0800
> Chuang W <nashuiliang@gmail.com> wrote:
>
> > In a scenario where livepatch and aggrprobe coexist, if arm_kprobe()
> > returns an error, ap.post_handler, while has been modified to
> > p.post_handler, is not rolled back.
>
> Would you mean 'coexist' on the same function?

Yes, It's the same function.

>
> >
> > When ap.post_handler is not NULL (not rolled back), the caller (e.g.
> > register_kprobe/enable_kprobe) of arm_kprobe_ftrace() will always fail.
>
> It seems this explanation and the actual code does not
> match. Can you tell me what actually you observed?
>
> Thank you,
>

I briefly describe the steps involved, a patch (kprobes: Rollback
kprobe flags on failed arm_kprobe,
https://lore.kernel.org/all/20220612213156.1323776351ee1be3cabc7fcc@kernel.org/T/)
must be added, otherwise it will panic:

1) add a livepatch

$ insmod livepatch-XXX.ko

2) add a kprobe using tracefs API

$ echo 'p:mykprobe XXX' > /sys/kernel/debug/tracing/kprobe_events

At this time, XXX is a simple kprobe, kprobe->post_handler = NULL.

3) add a second kprobe using raw kprobe API (i.e. register_kprobe),
the new kprobe->post_handler != NULL

$ insmod kprobe_XXX.ko
$ insmod: ERROR: could not insert module kprobe_XXX.ko: Device or resource busy

This will fail (as expected). However, XXX is modified to an
aggrprobe. agKprobe->post_handler = aggr_post_handler, it's not rolled
back on failed arm_kprobe().

4) add a third kprobe using bpftrace/bcc tool

$ bpftrace -e 'kprobe:XXX {printf("%s", kstack());}'
Attaching 1 probe...
perf_event_open(/sys/kernel/debug/tracing/events/kprobes/p_XXX_0_1_bcc_440/id):
Device or resource busy
Error attaching probe: 'kprobe:blkcg_destroy_blkgs'
$ bpftrace -e 'kprobe:XXX {printf("%s", kstack());}'
Attaching 1 probe...
perf_event_open(/sys/kernel/debug/tracing/events/kprobes/p_XXX_0_1_bcc_440/id):
Device or resource busy
Error attaching probe: 'kprobe:blkcg_destroy_blkgs'

This will always fail (not as expected).

> >
> > Fixes: 12310e343755 ("kprobes: Propagate error from arm_kprobe_ftrace()")
> > Signed-off-by: Chuang W <nashuiliang@gmail.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > kernel/kprobes.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index f214f8c088ed..0610b02a3a05 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -1300,6 +1300,7 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
> > {
> > int ret = 0;
> > struct kprobe *ap = orig_p;
> > + kprobe_post_handler_t old_post_handler = NULL;
> >
> > cpus_read_lock();
> >
> > @@ -1351,6 +1352,9 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
> >
> > /* Copy the insn slot of 'p' to 'ap'. */
> > copy_kprobe(ap, p);
> > +
> > + /* save the old post_handler */
> > + old_post_handler = ap->post_handler;
> > ret = add_new_kprobe(ap, p);
> >
> > out:
> > @@ -1365,6 +1369,7 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
> > ret = arm_kprobe(ap);
> > if (ret) {
> > ap->flags |= KPROBE_FLAG_DISABLED;
> > + ap->post_handler = old_post_handler;
> > list_del_rcu(&p->list);
> > synchronize_rcu();
> > }
> > --
> > 2.34.1
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

\
 
 \ /
  Last update: 2022-06-15 05:12    [W:0.168 / U:1.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site