If uprobe handler changes instruction pointer we still execute single
step) or emulate the original instruction and increment the (new) ip
with its length.
This makes the new instruction pointer bogus and application will
likely crash on illegal instruction execution.
If user decided to take execution elsewhere, it makes little sense
to execute the original instruction, so let's skip it.
Acked-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/events/uprobes.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 7ca1940607bd..2b32c32bcb77 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -2741,6 +2741,13 @@ static void handle_swbp(struct pt_regs *regs)
handler_chain(uprobe, regs);
+ /*
+ * If user decided to take execution elsewhere, it makes little sense
+ * to execute the original instruction, so let's skip it.
+ */
+ if (instruction_pointer(regs) != bp_vaddr)
+ goto out;
+
if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
goto out;
--
2.51.0
On Tue, Sep 16, 2025 at 2:53 PM Jiri Olsa <jolsa@kernel.org> wrote: > > If uprobe handler changes instruction pointer we still execute single > step) or emulate the original instruction and increment the (new) ip > with its length. > > This makes the new instruction pointer bogus and application will > likely crash on illegal instruction execution. > > If user decided to take execution elsewhere, it makes little sense > to execute the original instruction, so let's skip it. > > Acked-by: Oleg Nesterov <oleg@redhat.com> > Acked-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > kernel/events/uprobes.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 7ca1940607bd..2b32c32bcb77 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -2741,6 +2741,13 @@ static void handle_swbp(struct pt_regs *regs) > > handler_chain(uprobe, regs); > > + /* > + * If user decided to take execution elsewhere, it makes little sense > + * to execute the original instruction, so let's skip it. > + */ > + if (instruction_pointer(regs) != bp_vaddr) > + goto out; > + Peter, Ingo, Are you guys ok with us routing this through the bpf-next tree? We'll have a tiny conflict because in perf/core branch there is arch_uprobe_optimize() call added after handler_chain(), so git merge will be a bit confused, probably. But it should be trivially resolvable. > if (arch_uprobe_skip_sstep(&uprobe->arch, regs)) > goto out; > > -- > 2.51.0 >
On Tue, Sep 16, 2025 at 03:28:52PM -0700, Andrii Nakryiko wrote: > On Tue, Sep 16, 2025 at 2:53 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > If uprobe handler changes instruction pointer we still execute single > > step) or emulate the original instruction and increment the (new) ip > > with its length. > > > > This makes the new instruction pointer bogus and application will > > likely crash on illegal instruction execution. > > > > If user decided to take execution elsewhere, it makes little sense > > to execute the original instruction, so let's skip it. > > > > Acked-by: Oleg Nesterov <oleg@redhat.com> > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > kernel/events/uprobes.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > index 7ca1940607bd..2b32c32bcb77 100644 > > --- a/kernel/events/uprobes.c > > +++ b/kernel/events/uprobes.c > > @@ -2741,6 +2741,13 @@ static void handle_swbp(struct pt_regs *regs) > > > > handler_chain(uprobe, regs); > > > > + /* > > + * If user decided to take execution elsewhere, it makes little sense > > + * to execute the original instruction, so let's skip it. > > + */ > > + if (instruction_pointer(regs) != bp_vaddr) > > + goto out; > > + > > Peter, Ingo, > > Are you guys ok with us routing this through the bpf-next tree? We'll > have a tiny conflict because in perf/core branch there is > arch_uprobe_optimize() call added after handler_chain(), so git merge > will be a bit confused, probably. But it should be trivially > resolvable. Nah, I suppose that'll be fine. Thanks!
On Wed, Sep 24, 2025 at 11:15 AM Peter Zijlstra <peterz@infradead.org> wrote: > > On Tue, Sep 16, 2025 at 03:28:52PM -0700, Andrii Nakryiko wrote: > > On Tue, Sep 16, 2025 at 2:53 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > If uprobe handler changes instruction pointer we still execute single > > > step) or emulate the original instruction and increment the (new) ip > > > with its length. > > > > > > This makes the new instruction pointer bogus and application will > > > likely crash on illegal instruction execution. > > > > > > If user decided to take execution elsewhere, it makes little sense > > > to execute the original instruction, so let's skip it. > > > > > > Acked-by: Oleg Nesterov <oleg@redhat.com> > > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > kernel/events/uprobes.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > > index 7ca1940607bd..2b32c32bcb77 100644 > > > --- a/kernel/events/uprobes.c > > > +++ b/kernel/events/uprobes.c > > > @@ -2741,6 +2741,13 @@ static void handle_swbp(struct pt_regs *regs) > > > > > > handler_chain(uprobe, regs); > > > > > > + /* > > > + * If user decided to take execution elsewhere, it makes little sense > > > + * to execute the original instruction, so let's skip it. > > > + */ > > > + if (instruction_pointer(regs) != bp_vaddr) > > > + goto out; > > > + > > > > Peter, Ingo, > > > > Are you guys ok with us routing this through the bpf-next tree? We'll > > have a tiny conflict because in perf/core branch there is > > arch_uprobe_optimize() call added after handler_chain(), so git merge > > will be a bit confused, probably. But it should be trivially > > resolvable. > > Nah, I suppose that'll be fine. Thanks! Thanks! Applied. Jiri, in the future, please keep the whole history in the cover letter. v1->v2, v2->v3. Just v4 changes are nice, but pls copy paste previous cover letters and expand them. Also please always include links to previous versions in the cover. Search on lore sucks. Links in the cover are a much better way to preserve the history.
On Wed, Sep 24, 2025 at 11:47:42AM +0200, Alexei Starovoitov wrote: > On Wed, Sep 24, 2025 at 11:15 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > On Tue, Sep 16, 2025 at 03:28:52PM -0700, Andrii Nakryiko wrote: > > > On Tue, Sep 16, 2025 at 2:53 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > If uprobe handler changes instruction pointer we still execute single > > > > step) or emulate the original instruction and increment the (new) ip > > > > with its length. > > > > > > > > This makes the new instruction pointer bogus and application will > > > > likely crash on illegal instruction execution. > > > > > > > > If user decided to take execution elsewhere, it makes little sense > > > > to execute the original instruction, so let's skip it. > > > > > > > > Acked-by: Oleg Nesterov <oleg@redhat.com> > > > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > --- > > > > kernel/events/uprobes.c | 7 +++++++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > > > index 7ca1940607bd..2b32c32bcb77 100644 > > > > --- a/kernel/events/uprobes.c > > > > +++ b/kernel/events/uprobes.c > > > > @@ -2741,6 +2741,13 @@ static void handle_swbp(struct pt_regs *regs) > > > > > > > > handler_chain(uprobe, regs); > > > > > > > > + /* > > > > + * If user decided to take execution elsewhere, it makes little sense > > > > + * to execute the original instruction, so let's skip it. > > > > + */ > > > > + if (instruction_pointer(regs) != bp_vaddr) > > > > + goto out; > > > > + > > > > > > Peter, Ingo, > > > > > > Are you guys ok with us routing this through the bpf-next tree? We'll > > > have a tiny conflict because in perf/core branch there is > > > arch_uprobe_optimize() call added after handler_chain(), so git merge > > > will be a bit confused, probably. But it should be trivially > > > resolvable. > > > > Nah, I suppose that'll be fine. Thanks! > > Thanks! Applied. > > Jiri, > in the future, please keep the whole history in the cover letter. > v1->v2, v2->v3. Just v4 changes are nice, but pls copy paste > previous cover letters and expand them. ok > Also please always include links to previous versions in the cover. > Search on lore sucks. Links in the cover are a much better > way to preserve the history. will add them in future, thanks jirka
On Tue, Sep 16, 2025 at 3:28 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Sep 16, 2025 at 2:53 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > If uprobe handler changes instruction pointer we still execute single > > step) or emulate the original instruction and increment the (new) ip > > with its length. > > > > This makes the new instruction pointer bogus and application will > > likely crash on illegal instruction execution. > > > > If user decided to take execution elsewhere, it makes little sense > > to execute the original instruction, so let's skip it. > > > > Acked-by: Oleg Nesterov <oleg@redhat.com> > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > kernel/events/uprobes.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > index 7ca1940607bd..2b32c32bcb77 100644 > > --- a/kernel/events/uprobes.c > > +++ b/kernel/events/uprobes.c > > @@ -2741,6 +2741,13 @@ static void handle_swbp(struct pt_regs *regs) > > > > handler_chain(uprobe, regs); > > > > + /* > > + * If user decided to take execution elsewhere, it makes little sense > > + * to execute the original instruction, so let's skip it. > > + */ > > + if (instruction_pointer(regs) != bp_vaddr) > > + goto out; > > + > > Peter, Ingo, > > Are you guys ok with us routing this through the bpf-next tree? We'll > have a tiny conflict because in perf/core branch there is > arch_uprobe_optimize() call added after handler_chain(), so git merge > will be a bit confused, probably. But it should be trivially > resolvable. Ping. Any objections for landing this patch in bpf-next? > > > if (arch_uprobe_skip_sstep(&uprobe->arch, regs)) > > goto out; > > > > -- > > 2.51.0 > >
© 2016 - 2025 Red Hat, Inc.