Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1510466 > unrolled thread

[PATCHv4 0/4] WX checking for arm64

Back to article view | Back to linux.kernel

Contents

  [PATCHv4 0/4] WX checking for arm64 Laura Abbott <labbott@redhat.com> - 2016-10-27 18:30 +0200
    Re: [PATCHv4 0/4] WX checking for arm64 Catalin Marinas <catalin.marinas@arm.com> - 2016-10-30 22:30 +0100
      Re: [PATCHv4 0/4] WX checking for arm64 Mark Rutland <mark.rutland@arm.com> - 2016-11-07 16:50 +0100
        Re: [PATCHv4 0/4] WX checking for arm64 Laura Abbott <labbott@redhat.com> - 2016-11-07 17:30 +0100
          Re: [PATCHv4 0/4] WX checking for arm64 Catalin Marinas <catalin.marinas@arm.com> - 2016-11-07 17:40 +0100
        Re: [kernel-hardening] Re: [PATCHv4 0/4] WX checking for arm64 Mark Rutland <mark.rutland@arm.com> - 2016-11-07 21:00 +0100
          Re: [kernel-hardening] Re: [PATCHv4 0/4] WX checking for arm64 Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-11-07 21:00 +0100

#1510466 — [PATCHv4 0/4] WX checking for arm64

Hi,

This is v4 of the implementation to check for writable and executable pages on
arm64. This version contains a review from Ard and makes the UXN page count
a separate variable. Overall, minor changes.

Thanks,
Laura

Laura Abbott (4):
  arm64: dump: Make ptdump debugfs a separate option
  arm64: dump: Make the page table dumping seq_file optional
  arm64: dump: Remove max_addr
  arm64: dump: Add checking for writable and exectuable pages

 arch/arm64/Kconfig.debug           |  35 ++++++++++++-
 arch/arm64/include/asm/ptdump.h    |  22 +++++---
 arch/arm64/mm/Makefile             |   3 +-
 arch/arm64/mm/dump.c               | 105 +++++++++++++++++++++++++++----------
 arch/arm64/mm/mmu.c                |   2 +
 arch/arm64/mm/ptdump_debugfs.c     |  31 +++++++++++
 drivers/firmware/efi/arm-runtime.c |   4 +-
 7 files changed, 164 insertions(+), 38 deletions(-)
 create mode 100644 arch/arm64/mm/ptdump_debugfs.c

-- 
2.7.4

[toc] | [next] | [standalone]

#1512210

On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
> Laura Abbott (4):
>   arm64: dump: Make ptdump debugfs a separate option
>   arm64: dump: Make the page table dumping seq_file optional
>   arm64: dump: Remove max_addr
>   arm64: dump: Add checking for writable and exectuable pages

Queued for 4.10. Thanks.

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1516269

On Sun, Oct 30, 2016 at 03:03:07PM +0000, Catalin Marinas wrote:
> On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
> > Laura Abbott (4):
> >   arm64: dump: Make ptdump debugfs a separate option
> >   arm64: dump: Make the page table dumping seq_file optional
> >   arm64: dump: Remove max_addr
> >   arm64: dump: Add checking for writable and exectuable pages
> 
> Queued for 4.10. Thanks.

Catalin mentioned to me that he saw some KASAN splats when testing; it
looks like need a fixup something like the below.

Apologies for not having spotted this when testing!

Thanks,
Mark.

---->8----
From 06fef1ad1138d0808eec770e64458a350941bd2d Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Mon, 7 Nov 2016 15:24:40 +0000
Subject: [PATCH] Fix KASAN splats with DEBUG_WX

Booting a kernel built with both CONFIG_KASAN and CONFIG_DEBUG_WX
results in a stream of KASAN splats for stack-out-of-bounds accesses,
e.g.

==================================================================
BUG: KASAN: stack-out-of-bounds in note_page+0xd8/0x650 at addr ffff8009364ebdd0
Read of size 8 by task swapper/0/1
page:ffff7e0024d93ac0 count:0 mapcount:0 mapping:          (null) index:0x0
flags: 0x4000000000000000()
page dumped because: kasan: bad access detected
CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc3-00004-g25f7267 #77
Hardware name: ARM Juno development board (r1) (DT)
Call trace:
[<ffff20000808b2d8>] dump_backtrace+0x0/0x278
[<ffff20000808b564>] show_stack+0x14/0x20
[<ffff2000084e4e4c>] dump_stack+0xa4/0xc8
[<ffff200008256ee0>] kasan_report_error+0x4a8/0x4d0
[<ffff200008257330>] kasan_report+0x40/0x48
[<ffff200008255894>] __asan_load8+0x84/0x98
[<ffff2000080a0928>] note_page+0xd8/0x650
[<ffff2000080a0fb4>] walk_pgd.isra.1+0x114/0x220
[<ffff2000080a1248>] ptdump_check_wx+0xa8/0x118
[<ffff20000809ed40>] mark_rodata_ro+0x90/0xd0
[<ffff200008cafb88>] kernel_init+0x28/0x110
[<ffff200008083680>] ret_from_fork+0x10/0x50
Memory state around the buggy address:
 ffff8009364ebc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffff8009364ebd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff8009364ebd80: 00 00 00 00 f1 f1 f1 f1 00 00 f4 f4 f2 f2 f2 f2
                                                 ^
 ffff8009364ebe00: 00 00 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00
 ffff8009364ebe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================

... this happens because note_page assumes that the marker array has at
least two elements (the latter of which may be the terminator), but the
marker array for ptdump_check_wx only contains one element. Thus we
dereference some garbage on the stack when looking at
marker[1].start_address.

Given we don't need the markers for the WX checks, we could modify
note_page to allow for a NULL marker array, but for now it's simpler to
add an entry to the ptdump_check_wx marker array, so let's do that. As
it's somewhat confusing to have two identical entries, we add an initial
entry with a start address of zero.

Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/mm/dump.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index ef8aca8..ca74a2a 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -383,6 +383,7 @@ void ptdump_check_wx(void)
 	struct pg_state st = {
 		.seq = NULL,
 		.marker = (struct addr_marker[]) {
+			{ 0, NULL},
 			{ -1, NULL},
 		},
 		.check_wx = true,
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1516291

On 11/07/2016 07:38 AM, Mark Rutland wrote:
> On Sun, Oct 30, 2016 at 03:03:07PM +0000, Catalin Marinas wrote:
>> On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
>>> Laura Abbott (4):
>>>   arm64: dump: Make ptdump debugfs a separate option
>>>   arm64: dump: Make the page table dumping seq_file optional
>>>   arm64: dump: Remove max_addr
>>>   arm64: dump: Add checking for writable and exectuable pages
>>
>> Queued for 4.10. Thanks.
>
> Catalin mentioned to me that he saw some KASAN splats when testing; it
> looks like need a fixup something like the below.
>
> Apologies for not having spotted this when testing!
>
> Thanks,
> Mark.
>
> ---->8----
> From 06fef1ad1138d0808eec770e64458a350941bd2d Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Mon, 7 Nov 2016 15:24:40 +0000
> Subject: [PATCH] Fix KASAN splats with DEBUG_WX
>
> Booting a kernel built with both CONFIG_KASAN and CONFIG_DEBUG_WX
> results in a stream of KASAN splats for stack-out-of-bounds accesses,
> e.g.
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in note_page+0xd8/0x650 at addr ffff8009364ebdd0
> Read of size 8 by task swapper/0/1
> page:ffff7e0024d93ac0 count:0 mapcount:0 mapping:          (null) index:0x0
> flags: 0x4000000000000000()
> page dumped because: kasan: bad access detected
> CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc3-00004-g25f7267 #77
> Hardware name: ARM Juno development board (r1) (DT)
> Call trace:
> [<ffff20000808b2d8>] dump_backtrace+0x0/0x278
> [<ffff20000808b564>] show_stack+0x14/0x20
> [<ffff2000084e4e4c>] dump_stack+0xa4/0xc8
> [<ffff200008256ee0>] kasan_report_error+0x4a8/0x4d0
> [<ffff200008257330>] kasan_report+0x40/0x48
> [<ffff200008255894>] __asan_load8+0x84/0x98
> [<ffff2000080a0928>] note_page+0xd8/0x650
> [<ffff2000080a0fb4>] walk_pgd.isra.1+0x114/0x220
> [<ffff2000080a1248>] ptdump_check_wx+0xa8/0x118
> [<ffff20000809ed40>] mark_rodata_ro+0x90/0xd0
> [<ffff200008cafb88>] kernel_init+0x28/0x110
> [<ffff200008083680>] ret_from_fork+0x10/0x50
> Memory state around the buggy address:
>  ffff8009364ebc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffff8009364ebd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ffff8009364ebd80: 00 00 00 00 f1 f1 f1 f1 00 00 f4 f4 f2 f2 f2 f2
>                                                  ^
>  ffff8009364ebe00: 00 00 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00
>  ffff8009364ebe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
> ... this happens because note_page assumes that the marker array has at
> least two elements (the latter of which may be the terminator), but the
> marker array for ptdump_check_wx only contains one element. Thus we
> dereference some garbage on the stack when looking at
> marker[1].start_address.
>
> Given we don't need the markers for the WX checks, we could modify
> note_page to allow for a NULL marker array, but for now it's simpler to
> add an entry to the ptdump_check_wx marker array, so let's do that. As
> it's somewhat confusing to have two identical entries, we add an initial
> entry with a start address of zero.
>
> Reported-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/mm/dump.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
> index ef8aca8..ca74a2a 100644
> --- a/arch/arm64/mm/dump.c
> +++ b/arch/arm64/mm/dump.c
> @@ -383,6 +383,7 @@ void ptdump_check_wx(void)
>  	struct pg_state st = {
>  		.seq = NULL,
>  		.marker = (struct addr_marker[]) {
> +			{ 0, NULL},
>  			{ -1, NULL},
>  		},
>  		.check_wx = true,
>

Acked-by: Laura Abbott <labbott@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1516296

On Mon, Nov 07, 2016 at 08:26:34AM -0800, Laura Abbott wrote:
> On 11/07/2016 07:38 AM, Mark Rutland wrote:
> >From 06fef1ad1138d0808eec770e64458a350941bd2d Mon Sep 17 00:00:00 2001
> >From: Mark Rutland <mark.rutland@arm.com>
> >Date: Mon, 7 Nov 2016 15:24:40 +0000
> >Subject: [PATCH] Fix KASAN splats with DEBUG_WX
[...]
> Acked-by: Laura Abbott <labbott@redhat.com>

Thanks. I'll queue the patch on top of the others.

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1516543 — Re: [kernel-hardening] Re: [PATCHv4 0/4] WX checking for arm64

On Mon, Nov 07, 2016 at 03:38:02PM +0000, Mark Rutland wrote:
> On Sun, Oct 30, 2016 at 03:03:07PM +0000, Catalin Marinas wrote:
> > On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
> > > Laura Abbott (4):
> > >   arm64: dump: Make ptdump debugfs a separate option
> > >   arm64: dump: Make the page table dumping seq_file optional
> > >   arm64: dump: Remove max_addr
> > >   arm64: dump: Add checking for writable and exectuable pages
> > 
> > Queued for 4.10. Thanks.
> 
> Catalin mentioned to me that he saw some KASAN splats when testing; it
> looks like need a fixup something like the below.

As an aside, it looks like any ptdump usage when KASAN is enabled takes
several minutes, which at boot time looks like a hang.

AFAICT, this is because KASAN allocates *huge* VA ranges (4TB+) worth of
zeroed shadow memory at pte granularity (reusing the same pmd, pud,
tables), and the ptdump code dutifully walks this with, with the added
KASAN instrumentation overhead.

I'll try to dig into that tomorrow; I suspect/hope it's not necessary to
keep all of that mapped.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1516546 — Re: [kernel-hardening] Re: [PATCHv4 0/4] WX checking for arm64

On 7 November 2016 at 19:49, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Nov 07, 2016 at 03:38:02PM +0000, Mark Rutland wrote:
>> On Sun, Oct 30, 2016 at 03:03:07PM +0000, Catalin Marinas wrote:
>> > On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
>> > > Laura Abbott (4):
>> > >   arm64: dump: Make ptdump debugfs a separate option
>> > >   arm64: dump: Make the page table dumping seq_file optional
>> > >   arm64: dump: Remove max_addr
>> > >   arm64: dump: Add checking for writable and exectuable pages
>> >
>> > Queued for 4.10. Thanks.
>>
>> Catalin mentioned to me that he saw some KASAN splats when testing; it
>> looks like need a fixup something like the below.
>
> As an aside, it looks like any ptdump usage when KASAN is enabled takes
> several minutes, which at boot time looks like a hang.
>
> AFAICT, this is because KASAN allocates *huge* VA ranges (4TB+) worth of
> zeroed shadow memory at pte granularity (reusing the same pmd, pud,
> tables), and the ptdump code dutifully walks this with, with the added
> KASAN instrumentation overhead.
>
> I'll try to dig into that tomorrow; I suspect/hope it's not necessary to
> keep all of that mapped.
>

I have noticed that in the past, but I see how this delay at boot time
is an issue. However, I don't think there is a huge cost involved in
terms of memory footprint: AFAIK, the same PMD/PTE/kasan zero page are
mapped over and over across the range.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web