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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzbxjRwxhJTLUgJNwR-vEbDybBpawNsRb+y+PiDsxzT=eA@mail.gmail.com>
Date: Thu, 4 Sep 2025 11:06:31 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Oleg Nesterov <oleg@...hat.com>, Jiri Olsa <olsajiri@...il.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: [PATCH perf/core 02/11] uprobes: Skip emulate/sstep on unique
 uprobe when ip is changed

On Thu, Sep 4, 2025 at 8:02 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Thu, Sep 4, 2025 at 4:26 AM Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > On 09/04, Jiri Olsa wrote:
> > >
> > > On Thu, Sep 04, 2025 at 10:49:50AM +0200, Oleg Nesterov wrote:
> > > > On 09/03, Jiri Olsa wrote:
> > > > >
> > > > > On Wed, Sep 03, 2025 at 01:26:48PM +0200, Oleg Nesterov wrote:
> > > > > > On 09/02, Jiri Olsa wrote:
> > > > > > >
> > > > > > > If user decided to take execution elsewhere, it makes little sense
> > > > > > > to execute the original instruction, so let's skip it.
> > > > > >
> > > > > > Exactly.
> > > > > >
> > > > > > So why do we need all these "is_unique" complications? Only a single
> > > > > > is_unique/exclusive consumer can change regs->ip, so I guess handle_swbp()
> > > > > > can just do
> > > > > >
> > > > > >         handler_chain(uprobe, regs);
> > > > > >         if (instruction_pointer(regs) != bp_vaddr)
> > > > > >                 goto out;
> > > > >
> > > > > hum, that's what I did in rfc [1] but I thought you did not like that [2]
> > > > >
> > > > > [1] https://lore.kernel.org/bpf/20250801210238.2207429-2-jolsa@kernel.org/
> > > > > [2] https://lore.kernel.org/bpf/20250802103426.GC31711@redhat.com/
> > > > >
> > > > > I guess I misunderstood your reply [2], I'd be happy to drop the
> > > > > unique/exclusive flag
> > > >
> > > > Well, but that rfc didn't introduce the exclusive consumers, and I think
> > > > we agree that even with these changes the non-exclusive consumers must
> > > > never change regs->ip?
> > >
> > > ok, got excited too soon.. so you meant getting rid of is_unique
> > > check only for this patch and have just change below..  but keep
> > > the unique/exclusive flag from patch#1
> >
> > Yes, this is what I meant,
> >
> > > IIUC Andrii would remove the unique flag completely?
> >
> > Lets wait for Andrii...
>
> Not Andrii, but I see only negatives in this extra flag.
> It doesn't add any safety or guardrails.
> No need to pollute uapi with pointless flags.

+1. I think it's fine to just have something like

if (unlikely(instruction_pointer(regs) != bp_vaddr))
      goto out;

after all uprobe callbacks were processed. Even if every single one of
them modify IP, the last one that did that wins. Others (if they care)
can detect this.

Generally speaking, this is a very specialized use case (which is why
the opposition to complicating UAPI for all of that), and I'd expect
to have at most 1 such uprobe callbacks at any attach point, while all
others (if there are any "others") are read-only and won't care about
return IP.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ