* [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