[PATCH v10 2/2] arm64: Enable BTI for main executable as well as the interpreter

[Catalin Marinas]

On Mon, Feb 28, 2022 at 01:06:06PM +0000, Mark Brown wrote:
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 5369e649fa79..82aaf361fa17 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -701,23 +701,49 @@ core_initcall(tagged_addr_init);
>  #endif	/* CONFIG_ARM64_TAGGED_ADDR_ABI */
>  
>  #ifdef CONFIG_BINFMT_ELF
> +static unsigned int bti_main;
> +
>  int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
>  			 bool has_interp, bool is_interp)
>  {
> -	/*
> -	 * For dynamically linked executables the interpreter is
> -	 * responsible for setting PROT_BTI on everything except
> -	 * itself.
> -	 */
> -	if (is_interp != has_interp)
> -		return prot;
> -
> -	if (!(state->flags & ARM64_ELF_BTI))
> -		return prot;
> -
> -	if (prot & PROT_EXEC)
> +	if ((prot & PROT_EXEC) &&
> +	    (is_interp || !has_interp || bti_main) &&
> +	    (state->flags & arm64_elf_bti_flag(is_interp)))
>  		prot |= PROT_BTI;
>  
>  	return prot;
>  }

TBH, I liked the other series more as we were getting rid of
'has_interp' in patches 3 and 4. Now we keep it around only for the
bti_main case on dynamic executables (i.e. we need to distinguish them
from static). We could still get rid of has_interp if bti_main was
default on and it affected static binaries as well (for consistency, it
wouldn't be a bad idea).

I think the risk of ABI breaking is negligible in a glibc distro since
currently the dynamic loader sets PROT_BTI on the main exe anyway, just
as the kernel does after this series.

Anyway, from a correctness perspective, this patch looks fine to me,
just a preference for the other series:

Reviewed-by: Catalin Marinas <catalin.marinas at arm.com>