linux-kernel - Re: [PATCH] ARM: mm: flip priority of CONFIG_DEBUG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <565CF5E3.7000109@redhat.com>
Date:	Mon, 30 Nov 2015 17:20:35 -0800
From:	Laura Abbott <labbott@...hat.com>
To:	Kees Cook <keescook@...omium.org>
Cc:	Russell King <linux@....linux.org.uk>,
	Ard Biesheuvel <ard.biesheuvel@...aro.org>,
	Will Deacon <will.deacon@....com>,
	Nicolas Pitre <nico@...aro.org>, Arnd Bergmann <arnd@...db.de>,
	Catalin Marinas <catalin.marinas@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"kernel-hardening@...ts.openwall.com" 
	<kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH] ARM: mm: flip priority of CONFIG_DEBUG_RODATA

On 11/30/2015 05:08 PM, Kees Cook wrote:
> On Mon, Nov 30, 2015 at 5:03 PM, Laura Abbott <labbott@...hat.com> wrote:
>> On 11/30/2015 03:38 PM, Kees Cook wrote:
>>>
>>> Given the choice between making things NX or making things RO, we want
>>> RO first. As such, redefine CONFIG_DEBUG_RODATA to actually do the bulk
>>
>>
>> Can you give a citation for why? The thread that inspired it might be
>> a good link.
>
> This was inspired by my examining the existing architecture's
> implementations of CONFIG_DEBUG_RODATA after Ingo suggested it be made
> a common feature not a build-time config (or at least renamed):
> http://www.openwall.com/lists/kernel-hardening/2015/11/30/13
>

Thanks. I read the thread and I think it would be good to put a link
in the commit message to make it clearer why this is going in.

>>> index 41218867a9a6..b617084e9520 100644
>>> --- a/arch/arm/mm/Kconfig
>>> +++ b/arch/arm/mm/Kconfig
>>> @@ -1039,24 +1039,26 @@ config ARCH_SUPPORTS_BIG_ENDIAN
>>>            This option specifies the architecture can support big endian
>>>            operation.
>>>
>>> -config ARM_KERNMEM_PERMS
>>> -       bool "Restrict kernel memory permissions"
>>> +config DEBUG_RODATA
>>> +       bool "Make kernel text and rodata read-only"
>>>          depends on MMU
>>> +       default y if CPU_V7
>>>          help
>>> -         If this is set, kernel memory other than kernel text (and
>>> rodata)
>>> -         will be made non-executable. The tradeoff is that each region is
>>> -         padded to section-size (1MiB) boundaries (because their
>>> permissions
>>> -         are different and splitting the 1M pages into 4K ones causes TLB
>>> -         performance problems), wasting memory.
>>> +         If this is set, kernel memory (text, rodata, etc) will be made
>>> +         read-only, and non-text kernel memory will be made
>>> non-executable.
>>> +         The tradeoff is that each region is padded to section-size
>>> (1MiB)
>>> +         boundaries (because their permissions are different and
>>> splitting
>>> +         the 1M pages into 4K ones causes TLB performance problems),
>>> which
>>> +         can waste memory.
>>>
>>> -config DEBUG_RODATA
>>> -       bool "Make kernel text and rodata read-only"
>>> -       depends on ARM_KERNMEM_PERMS
>>> +config DEBUG_ALIGN_RODATA
>>> +       bool "Make rodata strictly non-executable"
>>> +       depends on DEBUG_RODATA
>>>          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. This creates
>>> -         another section-size padded region, so it can waste more memory
>>> -         space while gaining the read-only protections.
>>> +         If this is set, rodata will be made explicitly non-executable.
>>> This
>>> +         provides protection on the rare chance that attackers might find
>>> and
>>> +         use ROP gadgets that exist in the rodata section. This adds an
>>> +         additional section-aligned split of rodata from kernel text so
>>> it
>>> +         can be made explicitly non-executable. This padding may waste
>>> memory
>>> +         space to gain this additional protection.
>>
>>
>> I get that you want to make this match arm64 but it's really not intuitive that
>> something with ALIGN_RODATA in the name is actually for setting NX. The purpose
>> of ALIGN_RODATA was also slightly different on arm64 since the RO/NX will still
>> be there, the difference is if the sections are present versus broken down into
>> pages.
>
> Well, it seems to have the same effect: without the alignment, a
> portion of rodata may remain executable on arm64. Unless I
> misunderstand?
>

No, on arm64 everything should always be NX, the difference is part of the NX
sections may be mapped as pages instead of sections so you take the TLB hit.
It's a trade off of memory vs TLB pressure instead of just security vs TLB.

Thanks,
Laura
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/