Re: [PATCH v2 3/3] elf: Remove has_interp property from arch_adjust_elf_prot()

[Dave Martin <Dave.Martin@arm.com>]

On Wed, Jun 09, 2021 at 09:55:36AM -0700, Yu, Yu-cheng wrote:
> On 6/9/2021 8:17 AM, Dave Martin wrote:
> >On Fri, Jun 04, 2021 at 12:24:50PM +0100, Mark Brown wrote:
> >>Since we have added an is_interp flag to arch_parse_elf_property() we can
> >>drop the has_interp flag from arch_elf_adjust_prot(), the only user was
> >>the arm64 code which no longer needs it and any future users will be able
> >>to use arch_parse_elf_properties() to determine if an interpreter is in
> >>use.
> >
> >So far so good, but can we also drop the has_interp argument from
> >arch_parse_elf_properties()?
> >
> >Cross-check with Yu-Cheng Yu's series, but I don't see this being used
> >any more (except for passthrough in binfmt_elf.c).
> >
> >Since we are treating the interpreter and main executable orthogonally
> >to each other now, I don't think we should need a has_interp argument to
> >pass knowledge between the interpreter and executable handling phases
> >here.
> >
> 
> For CET, arch_parse_elf_property() needs to know has_interp and is_interp.
> Like the following, on top of your patches:
> 
> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
> index 607b782afe2c..9e6f142b5cef 100644
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -837,8 +837,15 @@ unsigned long KSTK_ESP(struct task_struct *task)
>  }
> 
>  int arch_parse_elf_property(u32 type, const void *data, size_t datasz,
> -			    bool compat, struct arch_elf_state *state)
> +			    bool compat, bool has_interp, bool is_interp,
> +			    struct arch_elf_state *state)
>  {
> +	/*
> +	 * Parse static-linked executable or the loader.
> +	 */
> +	if (has_interp != is_interp)
> +		return 0;
> +

[...]

Ah, sorry, I did attempt to check this with your series, but I didn't
attempt to build it.  I must have missed this somehow.

But: does x86 actually need to do this?

For arm64, we've discovered that it is better to treat the ELF
interpreter and main executable independently when applying the ELF
properties.

So, can x86 actually port away from this?  arch_parse_elf_properties()
and arch_adjust_elf_prot() would still know whether the interpreter is
being considered or not, via the is_interp argument to both functions.
This allows interpreter and main executable info to be stashed
independently in the arch_elf_state.

If x86 really needs to carry on following the existing model then that's
fine, but we should try to keep x86 and arm64 aligned if at all possible.

Cheers
---Dave