Re: [PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds

From: Ard Biesheuvel <ardb@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	 Steven Rostedt <rostedt@goodmis.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	 Cristian Marussi <cristian.marussi@arm.com>,
	Nathan Chancellor <nathan@kernel.org>,
	 Nick Desaulniers <ndesaulniers@google.com>,
	Arnd Bergmann <arnd@arndb.de>,
	 Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds
Date: Wed, 2 Feb 2022 09:00:22 +0100	[thread overview]
Message-ID: <CAMj1kXF5wkeycAqUPuU49MQfSFSH2c8z_s_smWF1FBfymR9Mdw@mail.gmail.com> (raw)
In-Reply-To: <20220202151033.ebe29e1256d19781ffc87ca3@kernel.org>

On Wed, 2 Feb 2022 at 07:10, Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Tue, 1 Feb 2022 15:05:25 +0100
> Ard Biesheuvel <ardb@kernel.org> wrote:
>
> > On Tue, 1 Feb 2022 at 14:18, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > > On Mon, 31 Jan 2022 18:03:44 +0100
> > > Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > > Thumb2 code uses R7 as the frame pointer rather than R11, because the
> > > > opcodes to access it are generally shorter.
> > > >
> > > > This means that there are cases where we cannot simply add it to the
> > > > clobber list of an asm() block, but need to preserve/restore it
> > > > explicitly, or the compiler may complain in some cases (e.g., Clang
> > > > builds with ftrace enabled).
> > > >
> > > > Since R11 is not special in that case, clobber it instead, and use it to
> > > > preserve/restore the value of R7.
> > >
> > > Thanks Ard for fixing thumb2 issue!
> >
> > No problem. Although I'm still not 100% sure we can simply
> > preserve/restore the frame pointer like that. What is the expected
> > result when an exception occurs in the kprobes emulation code?
>
> What kind of exception would you mean? If it is a synchronous
> exception and can return to where the exception happens, the
> all registers must be restored. So as long as it is restored
> it is OK to me.
>

So what should happen if a backtrace is triggered while the kprobes
emulation code is calling into a function?

> > The main problem is that with unwind info unwinding, the stack pointer
> > and the frame pointer need to be mutually in sync, and with the
> > kprobes stack frames in the middle, this is no longer correct - the
> > emulated code is executed with the frame pointer value that was
> > captured when the probe was hit, by the stack has a couple of frames
> > added.
> >
> > > BTW, have you build the kernel with CONFIG_KPROBES_SANITY_TEST=y?
> > > It should check the backtrace from kprobe and kretprobe at boot time.
> > >
> >
> > Just checked, and those work fine.
>
> Thank you for testing!
>
> Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
>

Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel