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

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

[Mark Brown]

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
+	 * responsible for setting PROT_BTI on everything except
+	 * itself.
+	 */
 	if (is_interp != has_interp)
 		return prot;
 
-- 
2.20.1


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

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

[Will Deacon]

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, 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.

Will

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

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

[Catalin Marinas]

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