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@xxxxxxx>
