All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Will Deacon <will@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Mahe Tardy <mahe.tardy@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, x86@kernel.org,
	Yonghong Song <yhs@fb.com>, Song Liu <songliubraving@fb.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCHv2 bpf-next 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
Date: Sat, 10 Jan 2026 18:06:11 +0100	[thread overview]
Message-ID: <aWKHA2bldOSZ0lMB@krava> (raw)
In-Reply-To: <aWEG685zlaV0o7M7@willie-the-truck>

On Fri, Jan 09, 2026 at 01:47:23PM +0000, Will Deacon wrote:
> On Fri, Jan 09, 2026 at 10:34:53AM +0100, Jiri Olsa wrote:
> > Mahe reported issue with bpf_override_return helper not working when
> > executed from kprobe.multi bpf program on arm.
> > 
> > The problem is that on arm we use alternate storage for pt_regs object
> > that is passed to bpf_prog_run and if any register is changed (which
> > is the case of bpf_override_return) it's not propagated back to actual
> > pt_regs object.
> > 
> > Fixing this by introducing and calling ftrace_partial_regs_update function
> > to propagate the values of changed registers (ip and stack).
> > 
> > Reported-by: Mahe Tardy <mahe.tardy@gmail.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > v2 changes:
> > - moved ftrace_partial_regs_update to generic code [Will]
> > 
> >  include/linux/ftrace_regs.h | 25 +++++++++++++++++++++++++
> >  kernel/trace/bpf_trace.c    |  1 +
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/include/linux/ftrace_regs.h b/include/linux/ftrace_regs.h
> > index 15627ceea9bc..f9a7c009cdae 100644
> > --- a/include/linux/ftrace_regs.h
> > +++ b/include/linux/ftrace_regs.h
> > @@ -33,6 +33,31 @@ struct ftrace_regs;
> >  #define ftrace_regs_get_frame_pointer(fregs) \
> >  	frame_pointer(&arch_ftrace_regs(fregs)->regs)
> >  
> > +static __always_inline void
> > +ftrace_partial_regs_update(const struct ftrace_regs *fregs, struct pt_regs *regs) { }
> > +
> > +#else
> > +
> > +/*
> > + * ftrace_partial_regs_update - update the original ftrace_regs from regs
> > + * @fregs: The ftrace_regs to update from @regs
> > + * @regs: The partial regs from ftrace_partial_regs() that was updated
> > + *
> > + * Some architectures have the partial regs living in the ftrace_regs
> > + * structure, whereas other architectures need to make a different copy
> > + * of the @regs. If a partial @regs is retrieved by ftrace_partial_regs() and
> > + * if the code using @regs updates a field (like the instruction pointer or
> > + * stack pointer) it may need to propagate that change to the original @fregs
> > + * it retrieved the partial @regs from. Use this function to guarantee that
> > + * update happens.
> > + */
> > +static __always_inline void
> > +ftrace_partial_regs_update(const struct ftrace_regs *fregs, struct pt_regs *regs)
> > +{
> > +	ftrace_regs_set_instruction_pointer(fregs, instruction_pointer(regs));
> > +	ftrace_regs_set_return_value(fregs, regs_return_value(regs));
> > +}
> 
> I think the AI thingy is right about dropping the const qualifier here

yes, will resend, thanks

jirka

> but overall I prefer this to the previous revisions. Thanks for sticking
> with it!
> 
> Acked-by: Will Deacon <will@kernel.org>
> 
> Will

  parent reply	other threads:[~2026-01-10 17:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-09  9:34 [PATCHv2 bpf-next 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run Jiri Olsa
2026-01-09  9:34 ` [PATCHv2 bpf-next 2/2] selftests/bpf: Add test for bpf_override_return helper Jiri Olsa
2026-01-09 10:00 ` [PATCHv2 bpf-next 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run bot+bpf-ci
2026-01-09 13:47 ` Will Deacon
2026-01-09 16:19   ` Steven Rostedt
2026-01-10 17:06   ` Jiri Olsa [this message]
2026-01-09 16:18 ` Steven Rostedt
2026-01-09 22:07 ` kernel test robot
2026-01-10  1:47 ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aWKHA2bldOSZ0lMB@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mahe.tardy@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=songliubraving@fb.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.