All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Martin <Dave.Martin@arm.com>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	Jeremy Linton <jeremy.linton@arm.com>,
	"H . J . Lu" <hjl.tools@gmail.com>,
	Yu-cheng Yu <yu-cheng.yu@intel.com>,
	linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	libc-alpha@sourceware.org
Subject: Re: [PATCH v2 3/3] elf: Remove has_interp property from arch_adjust_elf_prot()
Date: Wed, 9 Jun 2021 16:17:24 +0100	[thread overview]
Message-ID: <20210609151724.GM4187@arm.com> (raw)
In-Reply-To: <20210604112450.13344-4-broonie@kernel.org>

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.

> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kernel/process.c | 2 +-
>  fs/binfmt_elf.c             | 2 +-
>  include/linux/elf.h         | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index f7fff4a4c99f..e51c4aa7e048 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -742,7 +742,7 @@ asmlinkage void __sched arm64_preempt_schedule_irq(void)
>  
>  #ifdef CONFIG_BINFMT_ELF
>  int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
> -			 bool has_interp, bool is_interp)
> +			 bool is_interp)
>  {
>  	if (prot & PROT_EXEC) {
>  		if (state->flags & ARM64_ELF_INTERP_BTI && is_interp)
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 253ca9969345..1aba4e50e651 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -580,7 +580,7 @@ static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state,
>  	if (p_flags & PF_X)
>  		prot |= PROT_EXEC;
>  
> -	return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp);
> +	return arch_elf_adjust_prot(prot, arch_state, is_interp);
>  }
>  
>  /* This is much more generalized than the library routine read function,
> diff --git a/include/linux/elf.h b/include/linux/elf.h
> index 1c45ecf29147..d8392531899d 100644
> --- a/include/linux/elf.h
> +++ b/include/linux/elf.h
> @@ -101,11 +101,11 @@ extern int arch_parse_elf_property(u32 type, const void *data, size_t datasz,
>  
>  #ifdef CONFIG_ARCH_HAVE_ELF_PROT
>  int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
> -			 bool has_interp, bool is_interp);
> +			 bool is_interp);
>  #else
>  static inline int arch_elf_adjust_prot(int prot,
>  				       const struct arch_elf_state *state,
> -				       bool has_interp, bool is_interp)
> +				       bool is_interp)

[...]

Otherwise, looks reasonable.

Cheers
---Dave

WARNING: multiple messages have this Message-ID (diff)
From: Dave Martin <Dave.Martin@arm.com>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	Jeremy Linton <jeremy.linton@arm.com>,
	"H . J . Lu" <hjl.tools@gmail.com>,
	Yu-cheng Yu <yu-cheng.yu@intel.com>,
	linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	libc-alpha@sourceware.org
Subject: Re: [PATCH v2 3/3] elf: Remove has_interp property from arch_adjust_elf_prot()
Date: Wed, 9 Jun 2021 16:17:24 +0100	[thread overview]
Message-ID: <20210609151724.GM4187@arm.com> (raw)
In-Reply-To: <20210604112450.13344-4-broonie@kernel.org>

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.

> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kernel/process.c | 2 +-
>  fs/binfmt_elf.c             | 2 +-
>  include/linux/elf.h         | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index f7fff4a4c99f..e51c4aa7e048 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -742,7 +742,7 @@ asmlinkage void __sched arm64_preempt_schedule_irq(void)
>  
>  #ifdef CONFIG_BINFMT_ELF
>  int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
> -			 bool has_interp, bool is_interp)
> +			 bool is_interp)
>  {
>  	if (prot & PROT_EXEC) {
>  		if (state->flags & ARM64_ELF_INTERP_BTI && is_interp)
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 253ca9969345..1aba4e50e651 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -580,7 +580,7 @@ static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state,
>  	if (p_flags & PF_X)
>  		prot |= PROT_EXEC;
>  
> -	return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp);
> +	return arch_elf_adjust_prot(prot, arch_state, is_interp);
>  }
>  
>  /* This is much more generalized than the library routine read function,
> diff --git a/include/linux/elf.h b/include/linux/elf.h
> index 1c45ecf29147..d8392531899d 100644
> --- a/include/linux/elf.h
> +++ b/include/linux/elf.h
> @@ -101,11 +101,11 @@ extern int arch_parse_elf_property(u32 type, const void *data, size_t datasz,
>  
>  #ifdef CONFIG_ARCH_HAVE_ELF_PROT
>  int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
> -			 bool has_interp, bool is_interp);
> +			 bool is_interp);
>  #else
>  static inline int arch_elf_adjust_prot(int prot,
>  				       const struct arch_elf_state *state,
> -				       bool has_interp, bool is_interp)
> +				       bool is_interp)

[...]

Otherwise, looks reasonable.

Cheers
---Dave

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

  reply	other threads:[~2021-06-09 15:18 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 11:24 [PATCH v2 0/3] arm64: Enable BTI for the executable as well as the interpreter Mark Brown
2021-06-04 11:24 ` Mark Brown
2021-06-04 11:24 ` [PATCH v2 1/3] elf: Allow architectures to parse properties on the main executable Mark Brown
2021-06-04 11:24   ` Mark Brown
2021-06-09 15:16   ` Dave Martin
2021-06-09 15:16     ` Dave Martin
2021-06-10 13:41     ` Mark Brown
2021-06-10 13:41       ` Mark Brown
2021-06-04 11:24 ` [PATCH v2 2/3] arm64: Enable BTI for main executable as well as the interpreter Mark Brown
2021-06-04 11:24   ` Mark Brown
2021-06-09 15:17   ` Dave Martin
2021-06-09 15:17     ` Dave Martin
2021-06-10 13:19     ` Mark Brown
2021-06-10 13:19       ` Mark Brown
2021-06-10 15:34       ` Dave Martin
2021-06-10 15:34         ` Dave Martin
2021-06-04 11:24 ` [PATCH v2 3/3] elf: Remove has_interp property from arch_adjust_elf_prot() Mark Brown
2021-06-04 11:24   ` Mark Brown
2021-06-09 15:17   ` Dave Martin [this message]
2021-06-09 15:17     ` Dave Martin
2021-06-09 16:55     ` Yu, Yu-cheng
2021-06-09 16:55       ` Yu, Yu-cheng
2021-06-10  9:58       ` Dave Martin
2021-06-10  9:58         ` Dave Martin
2021-06-10 18:17         ` Yu, Yu-cheng
2021-06-10 18:17           ` Yu, Yu-cheng
2021-06-10 13:34     ` Mark Brown
2021-06-10 13:34       ` Mark Brown
2021-06-10 15:40       ` Dave Martin
2021-06-10 15:40         ` Dave Martin
2021-06-10 16:28 ` [PATCH v2 0/3] arm64: Enable BTI for the executable as well as the interpreter Jeremy Linton
2021-06-10 16:28   ` Jeremy Linton
2021-06-14 16:00   ` Mark Brown
2021-06-14 16:00     ` Mark Brown
2021-06-15 15:22   ` Dave Martin
2021-06-15 15:22     ` Dave Martin
2021-06-15 15:33     ` Mark Brown
2021-06-15 15:33       ` Mark Brown
2021-06-15 15:41       ` Dave Martin
2021-06-15 15:41         ` Dave Martin
2021-06-16  5:12         ` Jeremy Linton
2021-06-16  5:12           ` Jeremy Linton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210609151724.GM4187@arm.com \
    --to=dave.martin@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=hjl.tools@gmail.com \
    --cc=jeremy.linton@arm.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=szabolcs.nagy@arm.com \
    --cc=will@kernel.org \
    --cc=yu-cheng.yu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.