diff mbox

ARM: vdso: Mark vDSO code as read-only

Message ID 20160216213659.GA47194@davidb.org (mailing list archive)
State New, archived
Headers show

Commit Message

David Brown Feb. 16, 2016, 9:36 p.m. UTC
Although the arm vDSO is cleanly separated by code/data with the code
being read-only in userspace mappings, the code page is still writable
from the kernel.  There have been exploits (such as
http://itszn.com/blog/?p=21) that take advantage of this on x86 to go
from a bad kernel write to full root.

Prevent this specific exploit on arm by putting the vDSO code page in
post-init read-only memory as well.

Before:
vdso: 1 text pages at base 80927000
root@Vexpress:/ cat /sys/kernel/debug/kernel_page_tables
---[ Modules ]---
---[ Kernel Mapping ]---
0x80000000-0x80100000           1M     RW NX SHD
0x80100000-0x80600000           5M     ro x  SHD
0x80600000-0x80800000           2M     ro NX SHD
0x80800000-0xbe000000         984M     RW NX SHD

After:
vdso: 1 text pages at base 8072b000
root@Vexpress:/ cat /sys/kernel/debug/kernel_page_tables
---[ Modules ]---
---[ Kernel Mapping ]---
0x80000000-0x80100000           1M     RW NX SHD
0x80100000-0x80600000           5M     ro x  SHD
0x80600000-0x80800000           2M     ro NX SHD
0x80800000-0xbe000000         984M     RW NX SHD

Inspired by https://lkml.org/lkml/2016/1/19/494 based on work by the
PaX Team, Brad Spengler, and Kees Cook.

Signed-off-by: David Brown <david.brown@linaro.org>
---
This patch depends on Kees Cook's series
https://lkml.org/lkml/2016/1/19/497 which adds the ro_after_init
section.

 arch/arm/vdso/vdso.S | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Kees Cook Feb. 16, 2016, 9:52 p.m. UTC | #1
On Tue, Feb 16, 2016 at 1:36 PM, David Brown <david.brown@linaro.org> wrote:
> Although the arm vDSO is cleanly separated by code/data with the code
> being read-only in userspace mappings, the code page is still writable
> from the kernel.  There have been exploits (such as
> http://itszn.com/blog/?p=21) that take advantage of this on x86 to go
> from a bad kernel write to full root.
>
> Prevent this specific exploit on arm by putting the vDSO code page in
> post-init read-only memory as well.

Is the vdso dynamically built at init time like on x86, or can this
just use .rodata directly?

-Kees

>
> Before:
> vdso: 1 text pages at base 80927000
> root@Vexpress:/ cat /sys/kernel/debug/kernel_page_tables
> ---[ Modules ]---
> ---[ Kernel Mapping ]---
> 0x80000000-0x80100000           1M     RW NX SHD
> 0x80100000-0x80600000           5M     ro x  SHD
> 0x80600000-0x80800000           2M     ro NX SHD
> 0x80800000-0xbe000000         984M     RW NX SHD
>
> After:
> vdso: 1 text pages at base 8072b000
> root@Vexpress:/ cat /sys/kernel/debug/kernel_page_tables
> ---[ Modules ]---
> ---[ Kernel Mapping ]---
> 0x80000000-0x80100000           1M     RW NX SHD
> 0x80100000-0x80600000           5M     ro x  SHD
> 0x80600000-0x80800000           2M     ro NX SHD
> 0x80800000-0xbe000000         984M     RW NX SHD
>
> Inspired by https://lkml.org/lkml/2016/1/19/494 based on work by the
> PaX Team, Brad Spengler, and Kees Cook.
>
> Signed-off-by: David Brown <david.brown@linaro.org>
> ---
> This patch depends on Kees Cook's series
> https://lkml.org/lkml/2016/1/19/497 which adds the ro_after_init
> section.
>
> arch/arm/vdso/vdso.S | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/vdso/vdso.S b/arch/arm/vdso/vdso.S
> index b2b97e3..a62a7b6 100644
> --- a/arch/arm/vdso/vdso.S
> +++ b/arch/arm/vdso/vdso.S
> @@ -23,9 +23,8 @@
> #include <linux/const.h>
> #include <asm/page.h>
>
> -       __PAGE_ALIGNED_DATA
> -
>         .globl vdso_start, vdso_end
> +       .section .data..ro_after_init
>         .balign PAGE_SIZE
> vdso_start:
>         .incbin "arch/arm/vdso/vdso.so"
> --
> 2.7.1
>
David Brown Feb. 17, 2016, 5:20 a.m. UTC | #2
On Tue, Feb 16, 2016 at 01:52:33PM -0800, Kees Cook wrote:
>On Tue, Feb 16, 2016 at 1:36 PM, David Brown <david.brown@linaro.org> wrote:
>> Although the arm vDSO is cleanly separated by code/data with the code
>> being read-only in userspace mappings, the code page is still writable
>> from the kernel.  There have been exploits (such as
>> http://itszn.com/blog/?p=21) that take advantage of this on x86 to go
>> from a bad kernel write to full root.
>>
>> Prevent this specific exploit on arm by putting the vDSO code page in
>> post-init read-only memory as well.
>
>Is the vdso dynamically built at init time like on x86, or can this
>just use .rodata directly?

On ARM, it is patched during init.  Arm64's is just plain read-only.

David
Kees Cook Feb. 17, 2016, 11 p.m. UTC | #3
On Tue, Feb 16, 2016 at 9:20 PM, David Brown <david.brown@linaro.org> wrote:
> On Tue, Feb 16, 2016 at 01:52:33PM -0800, Kees Cook wrote:
>>
>> On Tue, Feb 16, 2016 at 1:36 PM, David Brown <david.brown@linaro.org>
>> wrote:
>>>
>>> Although the arm vDSO is cleanly separated by code/data with the code
>>> being read-only in userspace mappings, the code page is still writable
>>> from the kernel.  There have been exploits (such as
>>> http://itszn.com/blog/?p=21) that take advantage of this on x86 to go
>>> from a bad kernel write to full root.
>>>
>>> Prevent this specific exploit on arm by putting the vDSO code page in
>>> post-init read-only memory as well.
>>
>>
>> Is the vdso dynamically built at init time like on x86, or can this
>> just use .rodata directly?
>
>
> On ARM, it is patched during init.  Arm64's is just plain read-only.

Okay, great. I've added this to my postinit-readonly series (which I
just refreshed and sent out again...)

-Kees
David Brown Feb. 17, 2016, 11:43 p.m. UTC | #4
On Wed, Feb 17, 2016 at 03:00:52PM -0800, Kees Cook wrote:
>On Tue, Feb 16, 2016 at 9:20 PM, David Brown <david.brown@linaro.org> wrote:
>> On Tue, Feb 16, 2016 at 01:52:33PM -0800, Kees Cook wrote:
>>>
>>> On Tue, Feb 16, 2016 at 1:36 PM, David Brown <david.brown@linaro.org>
>>> wrote:
>>>>
>>>> Although the arm vDSO is cleanly separated by code/data with the code
>>>> being read-only in userspace mappings, the code page is still writable
>>>> from the kernel.  There have been exploits (such as
>>>> http://itszn.com/blog/?p=21) that take advantage of this on x86 to go
>>>> from a bad kernel write to full root.
>>>>
>>>> Prevent this specific exploit on arm by putting the vDSO code page in
>>>> post-init read-only memory as well.
>>>
>>>
>>> Is the vdso dynamically built at init time like on x86, or can this
>>> just use .rodata directly?
>>
>>
>> On ARM, it is patched during init.  Arm64's is just plain read-only.
>
>Okay, great. I've added this to my postinit-readonly series (which I
>just refreshed and sent out again...)

However, this distinction between .rodata and .data..ro_after_init is
kind of fuzzy, anyway, since they both get made actually read-only at
the same time (post init).  The patch actually does work fine with the
vDSO page in .rodata, since the patching happens during init.

Is there a possible future consideration to perhaps make .rodata read
only much earlier?

David
Kees Cook Feb. 17, 2016, 11:48 p.m. UTC | #5
On Wed, Feb 17, 2016 at 3:43 PM, David Brown <david.brown@linaro.org> wrote:
> On Wed, Feb 17, 2016 at 03:00:52PM -0800, Kees Cook wrote:
>>
>> On Tue, Feb 16, 2016 at 9:20 PM, David Brown <david.brown@linaro.org>
>> wrote:
>>>
>>> On Tue, Feb 16, 2016 at 01:52:33PM -0800, Kees Cook wrote:
>>>>
>>>>
>>>> On Tue, Feb 16, 2016 at 1:36 PM, David Brown <david.brown@linaro.org>
>>>> wrote:
>>>>>
>>>>>
>>>>> Although the arm vDSO is cleanly separated by code/data with the code
>>>>> being read-only in userspace mappings, the code page is still writable
>>>>> from the kernel.  There have been exploits (such as
>>>>> http://itszn.com/blog/?p=21) that take advantage of this on x86 to go
>>>>> from a bad kernel write to full root.
>>>>>
>>>>> Prevent this specific exploit on arm by putting the vDSO code page in
>>>>> post-init read-only memory as well.
>>>>
>>>>
>>>>
>>>> Is the vdso dynamically built at init time like on x86, or can this
>>>> just use .rodata directly?
>>>
>>>
>>>
>>> On ARM, it is patched during init.  Arm64's is just plain read-only.
>>
>>
>> Okay, great. I've added this to my postinit-readonly series (which I
>> just refreshed and sent out again...)
>
>
> However, this distinction between .rodata and .data..ro_after_init is
> kind of fuzzy, anyway, since they both get made actually read-only at
> the same time (post init).  The patch actually does work fine with the
> vDSO page in .rodata, since the patching happens during init.

Yeah, in the ARM case, that's true. I think we should probably keep it
marked "correctly" though.

> Is there a possible future consideration to perhaps make .rodata read
> only much earlier?

Yeah, this will likely be a future improvement. Some architectures
already mark .rodata before the mark_rodata_ro() call. Once we start
to have more use of postinit-readonly, I suspect we'll see more
clarification of when those things happen.

-Kees
pageexec@freemail.hu Feb. 18, 2016, 10:46 a.m. UTC | #6
On 17 Feb 2016 at 15:48, Kees Cook wrote:

> On Wed, Feb 17, 2016 at 3:43 PM, David Brown <david.brown@linaro.org> wrote:

> > Is there a possible future consideration to perhaps make .rodata read
> > only much earlier?
> 
> Yeah, this will likely be a future improvement. Some architectures
> already mark .rodata before the mark_rodata_ro() call. Once we start
> to have more use of postinit-readonly, I suspect we'll see more
> clarification of when those things happen.

FYI, PaX had enforced early rodata on i386 during the 2.4 series (i.e.,
decade+ ago) but i abandoned it for 2.6 due to the maintenance burden
coupled with its low benefit...
diff mbox

Patch

diff --git a/arch/arm/vdso/vdso.S b/arch/arm/vdso/vdso.S
index b2b97e3..a62a7b6 100644
--- a/arch/arm/vdso/vdso.S
+++ b/arch/arm/vdso/vdso.S
@@ -23,9 +23,8 @@ 
 #include <linux/const.h>
 #include <asm/page.h>
 
-	__PAGE_ALIGNED_DATA
-
 	.globl vdso_start, vdso_end
+	.section .data..ro_after_init
 	.balign PAGE_SIZE
 vdso_start:
 	.incbin "arch/arm/vdso/vdso.so"