How to debug "insecure W+X mapping"?

How to debug "insecure W+X mapping"?

[Timur Tabi]

We have a 4.10-based kernel that occasionally displays an insecure W+X 
mapping (courtesy of CONFIG_DEBUG_WX):

[    7.151680] arm64/mm: Found insecure W+X mapping at address 
0000345a049d2000/0x345a049d2000
...
[    7.435481] Checked W+X mappings: FAILED, 4 W+X pages found, 0 
non-UXN pages found

The number of actual W+X pages varies, e.g. sometimes it says 6 pages.

How do I go about debugging this? How do I identify the source of 
0000345a049d2000?

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

How to debug "insecure W+X mapping"?

[Laura Abbott]

On 12/12/2017 02:57 PM, Timur Tabi wrote:
> We have a 4.10-based kernel that occasionally displays an insecure W+X mapping (courtesy of CONFIG_DEBUG_WX):
> 
> [??? 7.151680] arm64/mm: Found insecure W+X mapping at address 0000345a049d2000/0x345a049d2000
> ...
> [??? 7.435481] Checked W+X mappings: FAILED, 4 W+X pages found, 0 non-UXN pages found
> 
> The number of actual W+X pages varies, e.g. sometimes it says 6 pages.
> 
> How do I go about debugging this? How do I identify the source of 0000345a049d2000?	
> 

That's a funny address. The check was written to scan the init_mm
page table but that's not a kernel address on arm64. It almost looks
like something set up a userspace mapping very early in the boot process?

I'd start by dumping the physical address at that address to
see if that gives a suggestion where the mapping came from.

Thanks,
Laura

[PATCH] arm64: fix CONFIG_DEBUG_WX address reporting (was: Re: How to debug "insecure W+X mapping"?)

[Mark Rutland]

On Tue, Dec 12, 2017 at 03:30:00PM -0800, Laura Abbott wrote:
> On 12/12/2017 02:57 PM, Timur Tabi wrote:
> > We have a 4.10-based kernel that occasionally displays an insecure W+X mapping (courtesy of CONFIG_DEBUG_WX):
> > 
> > [??? 7.151680] arm64/mm: Found insecure W+X mapping at address 0000345a049d2000/0x345a049d2000
> > ...
> > [??? 7.435481] Checked W+X mappings: FAILED, 4 W+X pages found, 0 non-UXN pages found
> > 
> > The number of actual W+X pages varies, e.g. sometimes it says 6 pages.
> > 
> > How do I go about debugging this? How do I identify the source of 0000345a049d2000?	
> 
> That's a funny address. The check was written to scan the init_mm
> page table but that's not a kernel address on arm64. It almost looks
> like something set up a userspace mapping very early in the boot process?

Whoops; I think we forgot to apply the VA_START offset in
ptdump_check_wx(), so we report the address wrong.

Does the below (untested) patch result in a sane-looking report?

Thanks,
Mark.

---->8----
>From b3021b76b9ea1e65388b38dfe622ea956cb18647 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Wed, 13 Dec 2017 11:45:42 +0000
Subject: [PATCH] arm64: fix CONFIG_DEBUG_WX address reporting

In ptdump_check_wx(), we pass walk_pgd() a start address of 0 (rather
than VA_START) for the init_mm. This means that any reported W&X
addresses are offset by VA_START, which is unexepcted and confusing.

Let's fix this by telling the ptdump code that we're walking init_mm
starting at VA_START. We don't need to update the addr_markers, since
these are still valid bounds regardless.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Laura Abbott <labbott@redhat.com>
Reported-by: Timur Tabi <timur@codeaurora.org>
---
 arch/arm64/mm/dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index ca74a2aace42..7b60d62ac593 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -389,7 +389,7 @@ void ptdump_check_wx(void)
 		.check_wx = true,
 	};
 
-	walk_pgd(&st, &init_mm, 0);
+	walk_pgd(&st, &init_mm, VA_START);
 	note_page(&st, 0, 0, 0);
 	if (st.wx_pages || st.uxn_pages)
 		pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found, %lu non-UXN pages found\n",
-- 
2.11.0

[PATCH] arm64: fix CONFIG_DEBUG_WX address reporting

[Laura Abbott]

On 12/13/2017 03:58 AM, Mark Rutland wrote:
> On Tue, Dec 12, 2017 at 03:30:00PM -0800, Laura Abbott wrote:
>> On 12/12/2017 02:57 PM, Timur Tabi wrote:
>>> We have a 4.10-based kernel that occasionally displays an insecure W+X mapping (courtesy of CONFIG_DEBUG_WX):
>>>
>>> [??? 7.151680] arm64/mm: Found insecure W+X mapping at address 0000345a049d2000/0x345a049d2000
>>> ...
>>> [??? 7.435481] Checked W+X mappings: FAILED, 4 W+X pages found, 0 non-UXN pages found
>>>
>>> The number of actual W+X pages varies, e.g. sometimes it says 6 pages.
>>>
>>> How do I go about debugging this? How do I identify the source of 0000345a049d2000?	
>>
>> That's a funny address. The check was written to scan the init_mm
>> page table but that's not a kernel address on arm64. It almost looks
>> like something set up a userspace mapping very early in the boot process?
> 
> Whoops; I think we forgot to apply the VA_START offset in
> ptdump_check_wx(), so we report the address wrong.
> 
> Does the below (untested) patch result in a sane-looking report?
> 
> Thanks,
> Mark.
> 
> ---->8----
>  From b3021b76b9ea1e65388b38dfe622ea956cb18647 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Wed, 13 Dec 2017 11:45:42 +0000
> Subject: [PATCH] arm64: fix CONFIG_DEBUG_WX address reporting
> 
> In ptdump_check_wx(), we pass walk_pgd() a start address of 0 (rather
> than VA_START) for the init_mm. This means that any reported W&X
> addresses are offset by VA_START, which is unexepcted and confusing.
> 
> Let's fix this by telling the ptdump code that we're walking init_mm
> starting at VA_START. We don't need to update the addr_markers, since
> these are still valid bounds regardless.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Laura Abbott <labbott@redhat.com>
> Reported-by: Timur Tabi <timur@codeaurora.org>
> ---
>   arch/arm64/mm/dump.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
> index ca74a2aace42..7b60d62ac593 100644
> --- a/arch/arm64/mm/dump.c
> +++ b/arch/arm64/mm/dump.c
> @@ -389,7 +389,7 @@ void ptdump_check_wx(void)
>   		.check_wx = true,
>   	};
>   
> -	walk_pgd(&st, &init_mm, 0);
> +	walk_pgd(&st, &init_mm, VA_START);
>   	note_page(&st, 0, 0, 0);
>   	if (st.wx_pages || st.uxn_pages)
>   		pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found, %lu non-UXN pages found\n",
> 

This looks better from my tests so you are welcome to add
Tested-by: Laura Abbott <labbott@redhat.com>

While we're fixing this up, do you want to drop the %p from the
"Found insecure W+X" message from above since pointer hashing
renders it kind of pointless or switch it to %px?

Thanks,
Laura

[PATCH] arm64: fix CONFIG_DEBUG_WX address reporting

[Timur Tabi]

On 12/14/2017 01:02 PM, Laura Abbott wrote:
> 
> While we're fixing this up, do you want to drop the %p from the
> "Found insecure W+X" message from above since pointer hashing
> renders it kind of pointless or switch it to %px?

Switching to %px gives me:

arm64/mm: Found insecure W+X mapping at address 
ffff2b1b4e512000/0xffff2b1b4e512000

Looks the same as before.

However, when I try to dump the contents of memory at that address via 
print_hex_dump(), I get an "Unable to handle kernel paging request" oops.

[   11.236091] Unable to handle kernel paging request at virtual address 
ffff2b1b4e512000
[   11.243985] pgd = ffff2b1b55a0d000
[   11.247371] [ffff2b1b4e512000] *pgd=00000007ffffe003, 
*pud=00000007ffffd003, *pmd=000000079972a003, *pte=0000000000000000


-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH] arm64: fix CONFIG_DEBUG_WX address reporting

[Timur Tabi]

On 12/13/2017 05:58 AM, Mark Rutland wrote:
> Whoops; I think we forgot to apply the VA_START offset in
> ptdump_check_wx(), so we report the address wrong.
> 
> Does the below (untested) patch result in a sane-looking report?

[   10.977304] arm64/mm: Found insecure W+X mapping at address 
ffff190621232000/0xffff190621232000

I can reproduce the problem after 2-3 reboots.  When it happens, I get a 
different pair of addresses, but they all look like these.

I'm not sure what to do next, however.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.