linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default
@ 2016-03-03 14:10 Ard Biesheuvel
  2016-03-03 16:00 ` Mark Rutland
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2016-03-03 14:10 UTC (permalink / raw)
  To: linux-arm-kernel

In spite of its name, CONFIG_DEBUG_RODATA is an important hardening feature
for production kernels, and distros all enable it by default in their
kernel configs. However, since enabling it used to result in more granular,
and thus less efficient kernel mappings, it is not enabled by default for
performance reasons.

However, since commit 2f39b5f91eb4 ("arm64: mm: Mark .rodata as RO"), the
various kernel segments (.text, .rodata, .init and .data) are already
mapped individually, and the only effect of setting CONFIG_DEBUG_RODATA is
that the existing .text and .rodata mappings are updated late in the boot
sequence to have their read-only attributes set, which means that any
performance concerns related to enabling CONFIG_DEBUG_RODATA are no longer
valid.

So from now on, make CONFIG_DEBUG_RODATA default to 'y'

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/Kconfig.debug | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index e13c4bf84d9e..7e76845a0434 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -50,13 +50,13 @@ config DEBUG_SET_MODULE_RONX
 
 config DEBUG_RODATA
 	bool "Make kernel text and rodata read-only"
+	default y
 	help
 	  If this is set, kernel text and rodata will be made read-only. This
 	  is to help catch accidental or malicious attempts to change the
-	  kernel's executable code. Additionally splits rodata from kernel
-	  text so it can be made explicitly non-executable.
+	  kernel's executable code.
 
-          If in doubt, say Y
+	  If in doubt, say Y
 
 config DEBUG_ALIGN_RODATA
 	depends on DEBUG_RODATA && ARM64_4K_PAGES
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default
  2016-03-03 14:10 [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default Ard Biesheuvel
@ 2016-03-03 16:00 ` Mark Rutland
  2016-03-03 16:50 ` Kees Cook
  2016-03-03 18:15 ` Catalin Marinas
  2 siblings, 0 replies; 6+ messages in thread
From: Mark Rutland @ 2016-03-03 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 03, 2016 at 03:10:59PM +0100, Ard Biesheuvel wrote:
> In spite of its name, CONFIG_DEBUG_RODATA is an important hardening feature
> for production kernels, and distros all enable it by default in their
> kernel configs. However, since enabling it used to result in more granular,
> and thus less efficient kernel mappings, it is not enabled by default for
> performance reasons.
> 
> However, since commit 2f39b5f91eb4 ("arm64: mm: Mark .rodata as RO"), the
> various kernel segments (.text, .rodata, .init and .data) are already
> mapped individually, and the only effect of setting CONFIG_DEBUG_RODATA is
> that the existing .text and .rodata mappings are updated late in the boot
> sequence to have their read-only attributes set, which means that any
> performance concerns related to enabling CONFIG_DEBUG_RODATA are no longer
> valid.
> 
> So from now on, make CONFIG_DEBUG_RODATA default to 'y'
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Finally! :)

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  arch/arm64/Kconfig.debug | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index e13c4bf84d9e..7e76845a0434 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -50,13 +50,13 @@ config DEBUG_SET_MODULE_RONX
>  
>  config DEBUG_RODATA
>  	bool "Make kernel text and rodata read-only"
> +	default y
>  	help
>  	  If this is set, kernel text and rodata will be made read-only. This
>  	  is to help catch accidental or malicious attempts to change the
> -	  kernel's executable code. Additionally splits rodata from kernel
> -	  text so it can be made explicitly non-executable.
> +	  kernel's executable code.
>  
> -          If in doubt, say Y
> +	  If in doubt, say Y
>  
>  config DEBUG_ALIGN_RODATA
>  	depends on DEBUG_RODATA && ARM64_4K_PAGES
> -- 
> 2.5.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default
  2016-03-03 14:10 [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default Ard Biesheuvel
  2016-03-03 16:00 ` Mark Rutland
@ 2016-03-03 16:50 ` Kees Cook
  2016-03-03 16:56   ` Ard Biesheuvel
  2016-03-03 18:15 ` Catalin Marinas
  2 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2016-03-03 16:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 3, 2016 at 6:10 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> In spite of its name, CONFIG_DEBUG_RODATA is an important hardening feature
> for production kernels, and distros all enable it by default in their
> kernel configs. However, since enabling it used to result in more granular,
> and thus less efficient kernel mappings, it is not enabled by default for
> performance reasons.
>
> However, since commit 2f39b5f91eb4 ("arm64: mm: Mark .rodata as RO"), the
> various kernel segments (.text, .rodata, .init and .data) are already
> mapped individually, and the only effect of setting CONFIG_DEBUG_RODATA is
> that the existing .text and .rodata mappings are updated late in the boot
> sequence to have their read-only attributes set, which means that any
> performance concerns related to enabling CONFIG_DEBUG_RODATA are no longer
> valid.
>
> So from now on, make CONFIG_DEBUG_RODATA default to 'y'
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Acked-by: Kees Cook <keescook@chromium.org>

If this doesn't cause any problems, perhaps we can make it always 'y' soon?

-Kees

> ---
>  arch/arm64/Kconfig.debug | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index e13c4bf84d9e..7e76845a0434 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -50,13 +50,13 @@ config DEBUG_SET_MODULE_RONX
>
>  config DEBUG_RODATA
>         bool "Make kernel text and rodata read-only"
> +       default y
>         help
>           If this is set, kernel text and rodata will be made read-only. This
>           is to help catch accidental or malicious attempts to change the
> -         kernel's executable code. Additionally splits rodata from kernel
> -         text so it can be made explicitly non-executable.
> +         kernel's executable code.
>
> -          If in doubt, say Y
> +         If in doubt, say Y
>
>  config DEBUG_ALIGN_RODATA
>         depends on DEBUG_RODATA && ARM64_4K_PAGES
> --
> 2.5.0
>



-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default
  2016-03-03 16:50 ` Kees Cook
@ 2016-03-03 16:56   ` Ard Biesheuvel
  2016-03-03 17:41     ` Kees Cook
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2016-03-03 16:56 UTC (permalink / raw)
  To: linux-arm-kernel

On 3 March 2016 at 17:50, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Mar 3, 2016 at 6:10 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> In spite of its name, CONFIG_DEBUG_RODATA is an important hardening feature
>> for production kernels, and distros all enable it by default in their
>> kernel configs. However, since enabling it used to result in more granular,
>> and thus less efficient kernel mappings, it is not enabled by default for
>> performance reasons.
>>
>> However, since commit 2f39b5f91eb4 ("arm64: mm: Mark .rodata as RO"), the
>> various kernel segments (.text, .rodata, .init and .data) are already
>> mapped individually, and the only effect of setting CONFIG_DEBUG_RODATA is
>> that the existing .text and .rodata mappings are updated late in the boot
>> sequence to have their read-only attributes set, which means that any
>> performance concerns related to enabling CONFIG_DEBUG_RODATA are no longer
>> valid.
>>
>> So from now on, make CONFIG_DEBUG_RODATA default to 'y'
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
> If this doesn't cause any problems, perhaps we can make it always 'y' soon?
>

You mean remove the option altogether? I would not mind, although
arguably, being able to map .text and .rodata writable could be
considered a useful debug option (and then it would almost, but not
quite, live up to its name)

-- 
Ard.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default
  2016-03-03 16:56   ` Ard Biesheuvel
@ 2016-03-03 17:41     ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2016-03-03 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 3, 2016 at 8:56 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 3 March 2016 at 17:50, Kees Cook <keescook@chromium.org> wrote:
>> On Thu, Mar 3, 2016 at 6:10 AM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>> In spite of its name, CONFIG_DEBUG_RODATA is an important hardening feature
>>> for production kernels, and distros all enable it by default in their
>>> kernel configs. However, since enabling it used to result in more granular,
>>> and thus less efficient kernel mappings, it is not enabled by default for
>>> performance reasons.
>>>
>>> However, since commit 2f39b5f91eb4 ("arm64: mm: Mark .rodata as RO"), the
>>> various kernel segments (.text, .rodata, .init and .data) are already
>>> mapped individually, and the only effect of setting CONFIG_DEBUG_RODATA is
>>> that the existing .text and .rodata mappings are updated late in the boot
>>> sequence to have their read-only attributes set, which means that any
>>> performance concerns related to enabling CONFIG_DEBUG_RODATA are no longer
>>> valid.
>>>
>>> So from now on, make CONFIG_DEBUG_RODATA default to 'y'
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> Acked-by: Kees Cook <keescook@chromium.org>
>>
>> If this doesn't cause any problems, perhaps we can make it always 'y' soon?
>>
>
> You mean remove the option altogether? I would not mind, although
> arguably, being able to map .text and .rodata writable could be
> considered a useful debug option (and then it would almost, but not
> quite, live up to its name)

For x86 (and everything that implements mark_rodata_ro()) this will
soon be controlled by rodata=off on the kernel command line (i.e. it
skips mark_rodata_ro() during startup).

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default
  2016-03-03 14:10 [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default Ard Biesheuvel
  2016-03-03 16:00 ` Mark Rutland
  2016-03-03 16:50 ` Kees Cook
@ 2016-03-03 18:15 ` Catalin Marinas
  2 siblings, 0 replies; 6+ messages in thread
From: Catalin Marinas @ 2016-03-03 18:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 03, 2016 at 03:10:59PM +0100, Ard Biesheuvel wrote:
> In spite of its name, CONFIG_DEBUG_RODATA is an important hardening feature
> for production kernels, and distros all enable it by default in their
> kernel configs. However, since enabling it used to result in more granular,
> and thus less efficient kernel mappings, it is not enabled by default for
> performance reasons.
> 
> However, since commit 2f39b5f91eb4 ("arm64: mm: Mark .rodata as RO"), the
> various kernel segments (.text, .rodata, .init and .data) are already
> mapped individually, and the only effect of setting CONFIG_DEBUG_RODATA is
> that the existing .text and .rodata mappings are updated late in the boot
> sequence to have their read-only attributes set, which means that any
> performance concerns related to enabling CONFIG_DEBUG_RODATA are no longer
> valid.
> 
> So from now on, make CONFIG_DEBUG_RODATA default to 'y'

Patch applied. If we later get a command line option as Kees mentioned,
I'm happy to drop this option entirely.

-- 
Catalin

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-03-03 18:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-03 14:10 [PATCH] arm64: enable CONFIG_DEBUG_RODATA by default Ard Biesheuvel
2016-03-03 16:00 ` Mark Rutland
2016-03-03 16:50 ` Kees Cook
2016-03-03 16:56   ` Ard Biesheuvel
2016-03-03 17:41     ` Kees Cook
2016-03-03 18:15 ` Catalin Marinas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).