lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aL8wL1rJRpWp_qHs@krava>
Date: Mon, 8 Sep 2025 21:36:15 +0200
From: Jiri Olsa <olsajiri@...il.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Oleg Nesterov <oleg@...hat.com>, Masami Hiramatsu <mhiramat@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	linux-trace-kernel <linux-trace-kernel@...r.kernel.org>,
	X86 ML <x86@...nel.org>, Song Liu <songliubraving@...com>,
	Yonghong Song <yhs@...com>,
	John Fastabend <john.fastabend@...il.com>,
	Hao Luo <haoluo@...gle.com>, Steven Rostedt <rostedt@...dmis.org>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCHv2 perf/core 1/4] bpf: Allow uprobe program to change
 context registers

On Mon, Sep 08, 2025 at 10:20:55AM -0700, Alexei Starovoitov wrote:
> On Mon, Sep 8, 2025 at 5:13 AM Jiri Olsa <jolsa@...nel.org> wrote:
> >
> > Currently uprobe (BPF_PROG_TYPE_KPROBE) program can't write to the
> > context registers data. While this makes sense for kprobe attachments,
> > for uprobe attachment it might make sense to be able to change user
> > space registers to alter application execution.
> >
> > Since uprobe and kprobe programs share the same type (BPF_PROG_TYPE_KPROBE),
> > we can't deny write access to context during the program load. We need
> > to check on it during program attachment to see if it's going to be
> > kprobe or uprobe.
> >
> > Storing the program's write attempt to context and checking on it
> > during the attachment.
> >
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> >  include/linux/bpf.h      | 1 +
> >  kernel/events/core.c     | 4 ++++
> >  kernel/trace/bpf_trace.c | 3 +--
> >  3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index cc700925b802..404a30cde84e 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1619,6 +1619,7 @@ struct bpf_prog_aux {
> >         bool priv_stack_requested;
> >         bool changes_pkt_data;
> >         bool might_sleep;
> > +       bool kprobe_write_ctx;
> >         u64 prog_array_member_cnt; /* counts how many times as member of prog_array */
> >         struct mutex ext_mutex; /* mutex for is_extended and prog_array_member_cnt */
> >         struct bpf_arena *arena;
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 28de3baff792..c3f37b266fc4 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -11238,6 +11238,10 @@ static int __perf_event_set_bpf_prog(struct perf_event *event,
> >         if (prog->kprobe_override && !is_kprobe)
> >                 return -EINVAL;
> >
> > +       /* Writing to context allowed only for uprobes. */
> > +       if (prog->aux->kprobe_write_ctx && !is_uprobe)
> > +               return -EINVAL;
> > +
> >         if (is_tracepoint || is_syscall_tp) {
> >                 int off = trace_event_get_offsets(event->tp_event);
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 3ae52978cae6..467fd5ab4b79 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -1521,8 +1521,6 @@ static bool kprobe_prog_is_valid_access(int off, int size, enum bpf_access_type
> >  {
> >         if (off < 0 || off >= sizeof(struct pt_regs))
> >                 return false;
> > -       if (type != BPF_READ)
> > -               return false;
> >         if (off % size != 0)
> >                 return false;
> >         /*
> > @@ -1532,6 +1530,7 @@ static bool kprobe_prog_is_valid_access(int off, int size, enum bpf_access_type
> >         if (off + size > sizeof(struct pt_regs))
> >                 return false;
> >
> > +       prog->aux->kprobe_write_ctx |= type == BPF_WRITE;
> 
> iirc the same function is used to validate [ku]probe.multi ctx access,
> but attaching is not done via __perf_event_set_bpf_prog().
> The check at attach time is missing?

argh, yes, missed that.. good catch, thanks

jirka

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ