Re: [PATCH] arm64: bti: Document behaviour for dynamically linked binaries

From: Catalin Marinas <catalin.marinas@arm.com>
To: Will Deacon <will@kernel.org>
Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Mark Brown <broonie@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: bti: Document behaviour for dynamically linked binaries
Date: Wed, 25 Mar 2020 09:50:16 +0000	[thread overview]
Message-ID: <20200325095016.GI3901@mbp> (raw)
In-Reply-To: <20200325080700.GA30293@willie-the-truck>

On Wed, Mar 25, 2020 at 08:07:01AM +0000, Will Deacon wrote:
> On Mon, Mar 23, 2020 at 05:01:19PM +0000, Mark Brown wrote:
> > For dynamically linked binaries the interpreter is responsible for setting
> > PROT_BTI on everything except itself. The dynamic linker needs to be aware
> > of PROT_BTI, for example in order to avoid dropping that when marking
> > executable pages read only after doing relocations, and doing everything
> > in userspace ensures that we don't get any issues due to divergences in
> > behaviour between the kernel and dynamic linker within a single executable.
> > Add a comment indicating that this is intentional to the code to help
> > people trying to understand what's going on.
> > 
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > ---
> >  arch/arm64/kernel/process.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > index 24af13d7bde6..9b668565dd10 100644
> > --- a/arch/arm64/kernel/process.c
> > +++ b/arch/arm64/kernel/process.c
> > @@ -674,6 +674,11 @@ asmlinkage void __sched arm64_preempt_schedule_irq(void)
> >  int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
> >  			 bool has_interp, bool is_interp)
> >  {
> > +	/*
> > +	 * For dynamicly linked executables the interpreter is
> 
> dynamicly => dynamically
> 
> > +	 * responsible for setting PROT_BTI on everything except
> > +	 * itself.
> > +	 */
> >  	if (is_interp != has_interp)
> >  		return prot;
> 
> Catalin: With the typo fixed, it's probably worth sticking this on
> for-next/bti so we don't lose it,

I will queue it but I'd first like Szabolcs to confirm. IIUC, he's still
running some tests with this approach, may get the results in a couple
of days.

> but I don't think this is really 5.7 material until the tools folks
> are happy with the ABI. It's pretty frustrating, but committing to
> something broken would be far worse and it's better to get these
> issues resolved *now* before anything hits mainline.

I agree. I was hoping to get this in 5.7 but, given the recent
discussions, I'd like to see an official ack from the toolchain folk
that the ABI works for them (this includes both glibc and bionic camps).

I guess we could carry the branch forward for 5.8, there are some
trivial conflicts with other cpufeature changes. Anyway, it's up to you
for the next merging window ;).

-- 
Catalin

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