kernel_text_address returns ftrace_trampoline, kprobe_insn_slot
and bpf_text_address as kprobe legal address.
These text are removable and changeable without any notifier to
kprobes. Probing on them can trigger some unexpected behavior[1].
Considering that jump_label and static_call text are already be
forbiden to probe, kernel_text_address should be replaced with
core_kernel_text and is_module_text_address to check other text
which is unsafe to kprobe.
[1] https://lkml.org/lkml/2022/7/26/1148
Fixes: 5b485629ba0d ("kprobes, extable: Identify kprobes trampolines as kernel text area")
Fixes: 74451e66d516 ("bpf: make jited programs visible in traces")
Signed-off-by: Chen Zhongjin <[email protected]>
---
v2 -> v3:
Remove '-next' carelessly added in title.
v1 -> v2:
Check core_kernel_text and is_module_text_address rather than
only kprobe_insn.
Also fix title and commit message for this. See old patch at [1].
---
kernel/kprobes.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f214f8c088ed..80697e5e03e4 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1560,7 +1560,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
preempt_disable();
/* Ensure it is not in reserved area nor out of text */
- if (!kernel_text_address((unsigned long) p->addr) ||
+ if (!(core_kernel_text((unsigned long) p->addr) ||
+ is_module_text_address((unsigned long) p->addr)) ||
within_kprobe_blacklist((unsigned long) p->addr) ||
jump_label_text_reserved(p->addr, p->addr) ||
static_call_text_reserved(p->addr, p->addr) ||
--
2.17.1
On Mon, Aug 01, 2022 at 11:37:19AM +0800, Chen Zhongjin wrote:
> kernel_text_address returns ftrace_trampoline, kprobe_insn_slot
> and bpf_text_address as kprobe legal address.
>
> These text are removable and changeable without any notifier to
> kprobes. Probing on them can trigger some unexpected behavior[1].
>
> Considering that jump_label and static_call text are already be
> forbiden to probe, kernel_text_address should be replaced with
> core_kernel_text and is_module_text_address to check other text
> which is unsafe to kprobe.
>
> [1] https://lkml.org/lkml/2022/7/26/1148
>
> Fixes: 5b485629ba0d ("kprobes, extable: Identify kprobes trampolines as kernel text area")
> Fixes: 74451e66d516 ("bpf: make jited programs visible in traces")
> Signed-off-by: Chen Zhongjin <[email protected]>
> ---
> v2 -> v3:
> Remove '-next' carelessly added in title.
LGTM cc-ing Steven because it affects ftrace as well
jirka
>
> v1 -> v2:
> Check core_kernel_text and is_module_text_address rather than
> only kprobe_insn.
> Also fix title and commit message for this. See old patch at [1].
> ---
> kernel/kprobes.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index f214f8c088ed..80697e5e03e4 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1560,7 +1560,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
> preempt_disable();
>
> /* Ensure it is not in reserved area nor out of text */
> - if (!kernel_text_address((unsigned long) p->addr) ||
> + if (!(core_kernel_text((unsigned long) p->addr) ||
> + is_module_text_address((unsigned long) p->addr)) ||
> within_kprobe_blacklist((unsigned long) p->addr) ||
> jump_label_text_reserved(p->addr, p->addr) ||
> static_call_text_reserved(p->addr, p->addr) ||
> --
> 2.17.1
>
On Mon, 1 Aug 2022 22:41:19 +0200
Jiri Olsa <[email protected]> wrote:
> LGTM cc-ing Steven because it affects ftrace as well
Thanks for the Cc, but I don't quite see how it affects ftrace.
Unless you are just saying how it can affect kprobe_events?
-- Steve
>
> jirka
>
> >
> > v1 -> v2:
> > Check core_kernel_text and is_module_text_address rather than
> > only kprobe_insn.
> > Also fix title and commit message for this. See old patch at [1].
> > ---
> > kernel/kprobes.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index f214f8c088ed..80697e5e03e4 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -1560,7 +1560,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
> > preempt_disable();
> >
> > /* Ensure it is not in reserved area nor out of text */
> > - if (!kernel_text_address((unsigned long) p->addr) ||
> > + if (!(core_kernel_text((unsigned long) p->addr) ||
> > + is_module_text_address((unsigned long) p->addr)) ||
> > within_kprobe_blacklist((unsigned long) p->addr) ||
> > jump_label_text_reserved(p->addr, p->addr) ||
> > static_call_text_reserved(p->addr, p->addr) ||
> > --
> > 2.17.1
> >
On Mon, 1 Aug 2022 11:37:19 +0800
Chen Zhongjin <[email protected]> wrote:
> kernel_text_address returns ftrace_trampoline, kprobe_insn_slot
> and bpf_text_address as kprobe legal address.
>
> These text are removable and changeable without any notifier to
> kprobes. Probing on them can trigger some unexpected behavior[1].
>
> Considering that jump_label and static_call text are already be
> forbiden to probe, kernel_text_address should be replaced with
> core_kernel_text and is_module_text_address to check other text
> which is unsafe to kprobe.
>
> [1] https://lkml.org/lkml/2022/7/26/1148
>
> Fixes: 5b485629ba0d ("kprobes, extable: Identify kprobes trampolines as kernel text area")
> Fixes: 74451e66d516 ("bpf: make jited programs visible in traces")
> Signed-off-by: Chen Zhongjin <[email protected]>
Thanks! this looks good to me.
Acked-by: Masami Hiramatsu (Google) <[email protected]>
> ---
> v2 -> v3:
> Remove '-next' carelessly added in title.
>
> v1 -> v2:
> Check core_kernel_text and is_module_text_address rather than
> only kprobe_insn.
> Also fix title and commit message for this. See old patch at [1].
> ---
> kernel/kprobes.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index f214f8c088ed..80697e5e03e4 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1560,7 +1560,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
> preempt_disable();
>
> /* Ensure it is not in reserved area nor out of text */
> - if (!kernel_text_address((unsigned long) p->addr) ||
> + if (!(core_kernel_text((unsigned long) p->addr) ||
> + is_module_text_address((unsigned long) p->addr)) ||
> within_kprobe_blacklist((unsigned long) p->addr) ||
> jump_label_text_reserved(p->addr, p->addr) ||
> static_call_text_reserved(p->addr, p->addr) ||
> --
> 2.17.1
>
--
Masami Hiramatsu (Google) <[email protected]>
On Mon, 1 Aug 2022 16:51:46 -0400
Steven Rostedt <[email protected]> wrote:
> On Mon, 1 Aug 2022 22:41:19 +0200
> Jiri Olsa <[email protected]> wrote:
>
> > LGTM cc-ing Steven because it affects ftrace as well
>
> Thanks for the Cc, but I don't quite see how it affects ftrace.
>
> Unless you are just saying how it can affect kprobe_events?
Maybe kprobe_events can probe the ftrace trampoline buffer if
CONFIG_KPROBE_EVENTS_ON_NOTRACE=y.
>
> -- Steve
>
>
> >
> > jirka
> >
> > >
> > > v1 -> v2:
> > > Check core_kernel_text and is_module_text_address rather than
> > > only kprobe_insn.
> > > Also fix title and commit message for this. See old patch at [1].
> > > ---
> > > kernel/kprobes.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > > index f214f8c088ed..80697e5e03e4 100644
> > > --- a/kernel/kprobes.c
> > > +++ b/kernel/kprobes.c
> > > @@ -1560,7 +1560,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
> > > preempt_disable();
> > >
> > > /* Ensure it is not in reserved area nor out of text */
> > > - if (!kernel_text_address((unsigned long) p->addr) ||
> > > + if (!(core_kernel_text((unsigned long) p->addr) ||
> > > + is_module_text_address((unsigned long) p->addr)) ||
> > > within_kprobe_blacklist((unsigned long) p->addr) ||
> > > jump_label_text_reserved(p->addr, p->addr) ||
> > > static_call_text_reserved(p->addr, p->addr) ||
> > > --
> > > 2.17.1
> > >
>
--
Masami Hiramatsu (Google) <[email protected]>
On Mon, Aug 01, 2022 at 04:51:46PM -0400, Steven Rostedt wrote:
> On Mon, 1 Aug 2022 22:41:19 +0200
> Jiri Olsa <[email protected]> wrote:
>
> > LGTM cc-ing Steven because it affects ftrace as well
>
> Thanks for the Cc, but I don't quite see how it affects ftrace.
>
> Unless you are just saying how it can affect kprobe_events?
nope, I just saw the 'ftrace' in changelog ;-)
anyway the patch makes check_kprobe_address_safe to fail
on ftrace trampoline address.. but not sure you could make
kprobe on ftrace trampoline before, probably not
jirka
>
> -- Steve
>
>
> >
> > jirka
> >
> > >
> > > v1 -> v2:
> > > Check core_kernel_text and is_module_text_address rather than
> > > only kprobe_insn.
> > > Also fix title and commit message for this. See old patch at [1].
> > > ---
> > > kernel/kprobes.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > > index f214f8c088ed..80697e5e03e4 100644
> > > --- a/kernel/kprobes.c
> > > +++ b/kernel/kprobes.c
> > > @@ -1560,7 +1560,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
> > > preempt_disable();
> > >
> > > /* Ensure it is not in reserved area nor out of text */
> > > - if (!kernel_text_address((unsigned long) p->addr) ||
> > > + if (!(core_kernel_text((unsigned long) p->addr) ||
> > > + is_module_text_address((unsigned long) p->addr)) ||
> > > within_kprobe_blacklist((unsigned long) p->addr) ||
> > > jump_label_text_reserved(p->addr, p->addr) ||
> > > static_call_text_reserved(p->addr, p->addr) ||
> > > --
> > > 2.17.1
> > >
>
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 28f6c37a2910f565b4f5960df52b2eccae28c891
Gitweb: https://git.kernel.org/tip/28f6c37a2910f565b4f5960df52b2eccae28c891
Author: Chen Zhongjin <[email protected]>
AuthorDate: Mon, 01 Aug 2022 11:37:19 +08:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Tue, 02 Aug 2022 11:47:29 +02:00
kprobes: Forbid probing on trampoline and BPF code areas
kernel_text_address() treats ftrace_trampoline, kprobe_insn_slot
and bpf_text_address as valid kprobe addresses - which is not ideal.
These text areas are removable and changeable without any notification
to kprobes, and probing on them can trigger unexpected behavior:
https://lkml.org/lkml/2022/7/26/1148
Considering that jump_label and static_call text are already
forbiden to probe, kernel_text_address() should be replaced with
core_kernel_text() and is_module_text_address() to check other text
areas which are unsafe to kprobe.
[ mingo: Rewrote the changelog. ]
Fixes: 5b485629ba0d ("kprobes, extable: Identify kprobes trampolines as kernel text area")
Fixes: 74451e66d516 ("bpf: make jited programs visible in traces")
Signed-off-by: Chen Zhongjin <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Acked-by: Masami Hiramatsu (Google) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/kprobes.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f214f8c..80697e5 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1560,7 +1560,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
preempt_disable();
/* Ensure it is not in reserved area nor out of text */
- if (!kernel_text_address((unsigned long) p->addr) ||
+ if (!(core_kernel_text((unsigned long) p->addr) ||
+ is_module_text_address((unsigned long) p->addr)) ||
within_kprobe_blacklist((unsigned long) p->addr) ||
jump_label_text_reserved(p->addr, p->addr) ||
static_call_text_reserved(p->addr, p->addr) ||