All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: AliOS system security <alios_sys_security@linux.alibaba.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>,
	catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: fix build error when use rodata_enabled
Date: Wed, 5 Jan 2022 10:47:35 +0000	[thread overview]
Message-ID: <YdV3RzQcGb2xFPhS@FVFF77S0Q05N> (raw)
In-Reply-To: <0432f592-789b-7c92-8d7a-99191d7bc669@arm.com>

On Wed, Jan 05, 2022 at 02:51:05PM +0530, Anshuman Khandual wrote:
> Hello,
> 
> On 1/5/22 8:37 AM, AliOS system security wrote:
> > rodata_enabled should be used when CONFIG_STRICT_KERNEL_RWX
> > or CONFIG_STRICT_MODULE_RWX is selected

Further to Anshuman's comments here, for a build issue, please include the
specific build error in the commit log, and describe the configuration where
this manifests.

For the reasons Anshuman gives below I do not see how this can be a problem in
mainline.

Thanks,
Mark.

> Both these configs get selected invariably with CONFIG_ARM64 in the
> platform config file (arch/arm64/Kconfig). I guess there can not be
> any such situation, where both configs will be missing/not selected
> given ARCH_OPTIONAL_KERNEL_RWX[or _DEFAULT] is not enabled on arm64.
> 
> config ARM64
>         def_bool y
>         select ACPI_CCA_REQUIRED if ACPI
> 	.....
>         select ARCH_HAS_STRICT_KERNEL_RWX
>         select ARCH_HAS_STRICT_MODULE_RWX
> 	.....
> 
> Hence for all practical purpose, rodata_enabled could be considered
> always available. I am sure there other similar situations as well,
> where code elements are not wrapped around if the config option is
> always present.
> 
> > 
> > Signed-off-by: AliOS system security <alios_sys_security@linux.alibaba.com>
> 
> Also please refer Documentation/process/submitting-patches.rst for
> the rules regarding names, that can be used for a commit sign off.
> 
> ------------------------------------------------------------------------
> then you just add a line saying::
> 
>         Signed-off-by: Random J Developer <random@developer.example.org>
> 
> using your real name (sorry, no pseudonyms or anonymous contributions.)
> ------------------------------------------------------------------------
> 
> - Anshuman
> 
> > ---
> >  arch/arm64/mm/mmu.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index acfae9b..47f8754 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -596,6 +596,7 @@ static void __init map_kernel_segment(pgd_t *pgdp, void *va_start, void *va_end,
> >  	vm_area_add_early(vma);
> >  }
> >  
> > +#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
> >  static int __init parse_rodata(char *arg)
> >  {
> >  	int ret = strtobool(arg, &rodata_enabled);
> > @@ -613,11 +614,16 @@ static int __init parse_rodata(char *arg)
> >  	return 0;
> >  }
> >  early_param("rodata", parse_rodata);
> > +#endif
> >  
> >  #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> >  static int __init map_entry_trampoline(void)
> >  {
> > -	pgprot_t prot = rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> > +	pgprot_t prot = PAGE_KERNEL_EXEC;
> > +#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
> > +	if (rodata_enabled)
> > +		prot = PAGE_KERNEL_ROX;
> > +#endif
> >  	phys_addr_t pa_start = __pa_symbol(__entry_tramp_text_start);
> >  
> >  	/* The trampoline is always mapped and can therefore be global */
> > @@ -672,7 +678,11 @@ static void __init map_kernel(pgd_t *pgdp)
> >  	 * mapping to install SW breakpoints. Allow this (only) when
> >  	 * explicitly requested with rodata=off.
> >  	 */
> > -	pgprot_t text_prot = rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> > +	pgprot_t text_prot = PAGE_KERNEL_EXEC;
> > +#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
> > +	if (rodata_enabled)
> > +		text_prot = PAGE_KERNEL_ROX;
> > +#endif
> >  
> >  	/*
> >  	 * If we have a CPU that supports BTI and a kernel built for
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: AliOS system security <alios_sys_security@linux.alibaba.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>,
	catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: fix build error when use rodata_enabled
Date: Wed, 5 Jan 2022 10:47:35 +0000	[thread overview]
Message-ID: <YdV3RzQcGb2xFPhS@FVFF77S0Q05N> (raw)
In-Reply-To: <0432f592-789b-7c92-8d7a-99191d7bc669@arm.com>

On Wed, Jan 05, 2022 at 02:51:05PM +0530, Anshuman Khandual wrote:
> Hello,
> 
> On 1/5/22 8:37 AM, AliOS system security wrote:
> > rodata_enabled should be used when CONFIG_STRICT_KERNEL_RWX
> > or CONFIG_STRICT_MODULE_RWX is selected

Further to Anshuman's comments here, for a build issue, please include the
specific build error in the commit log, and describe the configuration where
this manifests.

For the reasons Anshuman gives below I do not see how this can be a problem in
mainline.

Thanks,
Mark.

> Both these configs get selected invariably with CONFIG_ARM64 in the
> platform config file (arch/arm64/Kconfig). I guess there can not be
> any such situation, where both configs will be missing/not selected
> given ARCH_OPTIONAL_KERNEL_RWX[or _DEFAULT] is not enabled on arm64.
> 
> config ARM64
>         def_bool y
>         select ACPI_CCA_REQUIRED if ACPI
> 	.....
>         select ARCH_HAS_STRICT_KERNEL_RWX
>         select ARCH_HAS_STRICT_MODULE_RWX
> 	.....
> 
> Hence for all practical purpose, rodata_enabled could be considered
> always available. I am sure there other similar situations as well,
> where code elements are not wrapped around if the config option is
> always present.
> 
> > 
> > Signed-off-by: AliOS system security <alios_sys_security@linux.alibaba.com>
> 
> Also please refer Documentation/process/submitting-patches.rst for
> the rules regarding names, that can be used for a commit sign off.
> 
> ------------------------------------------------------------------------
> then you just add a line saying::
> 
>         Signed-off-by: Random J Developer <random@developer.example.org>
> 
> using your real name (sorry, no pseudonyms or anonymous contributions.)
> ------------------------------------------------------------------------
> 
> - Anshuman
> 
> > ---
> >  arch/arm64/mm/mmu.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index acfae9b..47f8754 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -596,6 +596,7 @@ static void __init map_kernel_segment(pgd_t *pgdp, void *va_start, void *va_end,
> >  	vm_area_add_early(vma);
> >  }
> >  
> > +#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
> >  static int __init parse_rodata(char *arg)
> >  {
> >  	int ret = strtobool(arg, &rodata_enabled);
> > @@ -613,11 +614,16 @@ static int __init parse_rodata(char *arg)
> >  	return 0;
> >  }
> >  early_param("rodata", parse_rodata);
> > +#endif
> >  
> >  #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> >  static int __init map_entry_trampoline(void)
> >  {
> > -	pgprot_t prot = rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> > +	pgprot_t prot = PAGE_KERNEL_EXEC;
> > +#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
> > +	if (rodata_enabled)
> > +		prot = PAGE_KERNEL_ROX;
> > +#endif
> >  	phys_addr_t pa_start = __pa_symbol(__entry_tramp_text_start);
> >  
> >  	/* The trampoline is always mapped and can therefore be global */
> > @@ -672,7 +678,11 @@ static void __init map_kernel(pgd_t *pgdp)
> >  	 * mapping to install SW breakpoints. Allow this (only) when
> >  	 * explicitly requested with rodata=off.
> >  	 */
> > -	pgprot_t text_prot = rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> > +	pgprot_t text_prot = PAGE_KERNEL_EXEC;
> > +#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
> > +	if (rodata_enabled)
> > +		text_prot = PAGE_KERNEL_ROX;
> > +#endif
> >  
> >  	/*
> >  	 * If we have a CPU that supports BTI and a kernel built for
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

  reply	other threads:[~2022-01-05 10:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05  3:07 [PATCH] arm64: fix build error when use rodata_enabled AliOS system security
2022-01-05  3:07 ` AliOS system security
2022-01-05  9:21 ` Anshuman Khandual
2022-01-05  9:21   ` Anshuman Khandual
2022-01-05 10:47   ` Mark Rutland [this message]
2022-01-05 10:47     ` Mark Rutland
     [not found]   ` <6f37012b-b082-457f-9aee-2315a461c031.alios_sys_security@linux.alibaba.com>
2022-01-05 12:48     ` 回复:[PATCH] " Anshuman Khandual
2022-01-05 12:48       ` Anshuman Khandual
2022-01-05 10:22 ` [PATCH] " kernel test robot
2022-01-05 10:22   ` kernel test robot
2022-01-05 10:22   ` kernel test robot

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=YdV3RzQcGb2xFPhS@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=alios_sys_security@linux.alibaba.com \
    --cc=anshuman.khandual@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=will@kernel.org \
    /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.