All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
@ 2017-11-21  3:44 ` Chen Feng
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Feng @ 2017-11-21  3:44 UTC (permalink / raw)
  To: puck.chen, catalin.marinas, will.deacon, linux-arm-kernel,
	linux-kernel, ard.biesheuvel
  Cc: dan.zhao, suzhuangluan, xuyiping, chenxiang9, chenya99, houyu3,
	xuyoujun4

With kaslr and kasan enable both, I got the follow issue.

[   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
[   16.142517s]___alloc_bootmem_nopanic:257
[   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
[   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
[   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
[   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
[   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
[   16.189971s]pgd = ffffffad9c507000
[   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003

*reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*

memstart_addr 0
ARM64_MEMSTART_ALIGN 0x40000000
memstart_offset_seed 0xffc7
PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000

reg->base = 0x100000000  -> 0xffffffff40000000
phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.

And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
if the start >= end, it will not map the hole block address. But the memory in this
block is valid. And it can be allocated as well.

So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
range = range / ARM64_MEMSTART_ALIGN;

Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
---
 arch/arm64/mm/init.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 716d122..60112c0 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
 		 * margin, the size of the region that the available physical
 		 * memory spans, randomize the linear region as well.
 		 */
-		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
-			range = range / ARM64_MEMSTART_ALIGN + 1;
-			memstart_addr -= ARM64_MEMSTART_ALIGN *
-					 ((range * memstart_offset_seed) >> 16);
-		}
+		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
+			memstart_addr -= (range * memstart_offset_seed) >> 16;
 	}
 
 	/*
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
@ 2017-11-21  3:44 ` Chen Feng
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Feng @ 2017-11-21  3:44 UTC (permalink / raw)
  To: linux-arm-kernel

With kaslr and kasan enable both, I got the follow issue.

[   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
[   16.142517s]___alloc_bootmem_nopanic:257
[   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
[   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
[   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
[   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
[   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
[   16.189971s]pgd = ffffffad9c507000
[   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003

*reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*

memstart_addr 0
ARM64_MEMSTART_ALIGN 0x40000000
memstart_offset_seed 0xffc7
PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000

reg->base = 0x100000000  -> 0xffffffff40000000
phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.

And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
if the start >= end, it will not map the hole block address. But the memory in this
block is valid. And it can be allocated as well.

So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
range = range / ARM64_MEMSTART_ALIGN;

Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
---
 arch/arm64/mm/init.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 716d122..60112c0 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
 		 * margin, the size of the region that the available physical
 		 * memory spans, randomize the linear region as well.
 		 */
-		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
-			range = range / ARM64_MEMSTART_ALIGN + 1;
-			memstart_addr -= ARM64_MEMSTART_ALIGN *
-					 ((range * memstart_offset_seed) >> 16);
-		}
+		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
+			memstart_addr -= (range * memstart_offset_seed) >> 16;
 	}
 
 	/*
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
  2017-11-21  3:44 ` Chen Feng
@ 2017-11-21  8:37   ` Chen Feng
  -1 siblings, 0 replies; 12+ messages in thread
From: Chen Feng @ 2017-11-21  8:37 UTC (permalink / raw)
  To: catalin.marinas, will.deacon, linux-arm-kernel, linux-kernel,
	ard.biesheuvel
  Cc: dan.zhao, suzhuangluan, xuyiping, chenxiang9, chenya99, houyu3,
	xuyoujun4



On 2017/11/21 11:44, Chen Feng wrote:
> With kaslr and kasan enable both, I got the follow issue.
> 
> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
> [   16.142517s]___alloc_bootmem_nopanic:257
> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
> [   16.189971s]pgd = ffffffad9c507000
> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
> 
> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
> 
> memstart_addr 0
> ARM64_MEMSTART_ALIGN 0x40000000
> memstart_offset_seed 0xffc7
> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
> 
> reg->base = 0x100000000  -> 0xffffffff40000000
> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
> 
> And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
> if the start >= end, it will not map the hole block address. But the memory in this
> block is valid. And it can be allocated as well.
> 
> So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
> range = range / ARM64_MEMSTART_ALIGN;
> 
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
> ---
>  arch/arm64/mm/init.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 716d122..60112c0 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
>  		 * margin, the size of the region that the available physical
>  		 * memory spans, randomize the linear region as well.
>  		 */
> -		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
> -			range = range / ARM64_MEMSTART_ALIGN + 1;
> -			memstart_addr -= ARM64_MEMSTART_ALIGN *
> -					 ((range * memstart_offset_seed) >> 16);
> -		}
> +		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
> +			memstart_addr -= (range * memstart_offset_seed) >> 16;
>  	}
Sorry, here is not align to ARM64_MEMSTART_ALIGN

Should be:
	if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
		range = range / ARM64_MEMSTART_ALIGN;
		memstart_addr -= ARM64_MEMSTART_ALIGN *
				 ((range * memstart_offset_seed) >> 16);
	}

>  
>  	/*
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
@ 2017-11-21  8:37   ` Chen Feng
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Feng @ 2017-11-21  8:37 UTC (permalink / raw)
  To: linux-arm-kernel



On 2017/11/21 11:44, Chen Feng wrote:
> With kaslr and kasan enable both, I got the follow issue.
> 
> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
> [   16.142517s]___alloc_bootmem_nopanic:257
> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
> [   16.189971s]pgd = ffffffad9c507000
> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
> 
> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
> 
> memstart_addr 0
> ARM64_MEMSTART_ALIGN 0x40000000
> memstart_offset_seed 0xffc7
> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
> 
> reg->base = 0x100000000  -> 0xffffffff40000000
> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
> 
> And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
> if the start >= end, it will not map the hole block address. But the memory in this
> block is valid. And it can be allocated as well.
> 
> So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
> range = range / ARM64_MEMSTART_ALIGN;
> 
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
> ---
>  arch/arm64/mm/init.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 716d122..60112c0 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
>  		 * margin, the size of the region that the available physical
>  		 * memory spans, randomize the linear region as well.
>  		 */
> -		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
> -			range = range / ARM64_MEMSTART_ALIGN + 1;
> -			memstart_addr -= ARM64_MEMSTART_ALIGN *
> -					 ((range * memstart_offset_seed) >> 16);
> -		}
> +		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
> +			memstart_addr -= (range * memstart_offset_seed) >> 16;
>  	}
Sorry, here is not align to ARM64_MEMSTART_ALIGN

Should be:
	if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
		range = range / ARM64_MEMSTART_ALIGN;
		memstart_addr -= ARM64_MEMSTART_ALIGN *
				 ((range * memstart_offset_seed) >> 16);
	}

>  
>  	/*
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
  2017-11-21  3:44 ` Chen Feng
@ 2017-11-28 18:35   ` Will Deacon
  -1 siblings, 0 replies; 12+ messages in thread
From: Will Deacon @ 2017-11-28 18:35 UTC (permalink / raw)
  To: Chen Feng, ard.biesheuvel
  Cc: catalin.marinas, linux-arm-kernel, linux-kernel, dan.zhao,
	suzhuangluan, xuyiping, chenxiang9, chenya99, houyu3, xuyoujun4

On Tue, Nov 21, 2017 at 11:44:13AM +0800, Chen Feng wrote:
> With kaslr and kasan enable both, I got the follow issue.
> 
> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
> [   16.142517s]___alloc_bootmem_nopanic:257
> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
> [   16.189971s]pgd = ffffffad9c507000
> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
> 
> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
> 
> memstart_addr 0
> ARM64_MEMSTART_ALIGN 0x40000000
> memstart_offset_seed 0xffc7
> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
> 
> reg->base = 0x100000000  -> 0xffffffff40000000
> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
> 
> And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
> if the start >= end, it will not map the hole block address. But the memory in this
> block is valid. And it can be allocated as well.
> 
> So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
> range = range / ARM64_MEMSTART_ALIGN;
> 
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
> ---
>  arch/arm64/mm/init.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Ard -- can you take a look at this please? I've not found time to page this
in, but if there's a fix in here somewhere then I'm happy to apply it.

Will

> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 716d122..60112c0 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
>  		 * margin, the size of the region that the available physical
>  		 * memory spans, randomize the linear region as well.
>  		 */
> -		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
> -			range = range / ARM64_MEMSTART_ALIGN + 1;
> -			memstart_addr -= ARM64_MEMSTART_ALIGN *
> -					 ((range * memstart_offset_seed) >> 16);
> -		}
> +		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
> +			memstart_addr -= (range * memstart_offset_seed) >> 16;
>  	}
>  
>  	/*
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
@ 2017-11-28 18:35   ` Will Deacon
  0 siblings, 0 replies; 12+ messages in thread
From: Will Deacon @ 2017-11-28 18:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 21, 2017 at 11:44:13AM +0800, Chen Feng wrote:
> With kaslr and kasan enable both, I got the follow issue.
> 
> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
> [   16.142517s]___alloc_bootmem_nopanic:257
> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
> [   16.189971s]pgd = ffffffad9c507000
> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
> 
> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
> 
> memstart_addr 0
> ARM64_MEMSTART_ALIGN 0x40000000
> memstart_offset_seed 0xffc7
> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
> 
> reg->base = 0x100000000  -> 0xffffffff40000000
> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
> 
> And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
> if the start >= end, it will not map the hole block address. But the memory in this
> block is valid. And it can be allocated as well.
> 
> So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
> range = range / ARM64_MEMSTART_ALIGN;
> 
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
> ---
>  arch/arm64/mm/init.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Ard -- can you take a look at this please? I've not found time to page this
in, but if there's a fix in here somewhere then I'm happy to apply it.

Will

> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 716d122..60112c0 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
>  		 * margin, the size of the region that the available physical
>  		 * memory spans, randomize the linear region as well.
>  		 */
> -		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
> -			range = range / ARM64_MEMSTART_ALIGN + 1;
> -			memstart_addr -= ARM64_MEMSTART_ALIGN *
> -					 ((range * memstart_offset_seed) >> 16);
> -		}
> +		if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
> +			memstart_addr -= (range * memstart_offset_seed) >> 16;
>  	}
>  
>  	/*
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
  2017-11-21  3:44 ` Chen Feng
@ 2017-11-28 20:41   ` Ard Biesheuvel
  -1 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-11-28 20:41 UTC (permalink / raw)
  To: Chen Feng
  Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel,
	Dan Zhao, Zhuangluan Su, Yiping Xu, chenxiang9, chenya99, houyu3,
	xuyoujun4

On 21 November 2017 at 03:44, Chen Feng <puck.chen@hisilicon.com> wrote:
> With kaslr and kasan enable both, I got the follow issue.
>
> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
> [   16.142517s]___alloc_bootmem_nopanic:257
> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
> [   16.189971s]pgd = ffffffad9c507000
> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
>
> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
>
> memstart_addr 0
> ARM64_MEMSTART_ALIGN 0x40000000
> memstart_offset_seed 0xffc7
> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
>
> reg->base = 0x100000000  -> 0xffffffff40000000
> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
>

This looks a bit weird because we add the PAGE_OFFSET, but it simply
wraps at the top of the address space.

So this code in kasan_init()

void *start = (void *)__phys_to_virt(reg->base);
void *end = (void *)__phys_to_virt(reg->base + reg->size);

if (start >= end)
     break;

is essentially incorrect, because it translates an address that is
strictly outside of the current memblock region. If the KASLR code
happens to map DRAM all the way at the top of the linear region (which
is what occurs in your case), end - 1 is the last valid address.

So I think the minimal correct fix would be

diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
index acba49fb5aac..3214aa9d90be 100644
--- a/arch/arm64/mm/kasan_init.c
+++ b/arch/arm64/mm/kasan_init.c
@@ -216,7 +216,7 @@ void __init kasan_init(void)

        for_each_memblock(memory, reg) {
                void *start = (void *)__phys_to_virt(reg->base);
-               void *end = (void *)__phys_to_virt(reg->base + reg->size);
+               void *end = start + reg->size;

                if (start >= end)
                        break;

given that mappings in the linear region are congruent with the
underlying physical regions (unless I am missing something wrt special
start/end values in memblock, but in that case, they should not be p2v
translated before the evaluation)

However, since having DRAM at the very top appears to break other things as well

vmemmap : 0xffffffbf00000000 - 0xffffffc000000000   (     4 GB maximum)
          0xffffffbfff000000 - 0xffffffbf00000000   (17592186040336 MB actual)
memory  : 0xffffffffc0000000 - 0x               0   (  1024 MB)

I will leave it to Will and/or Catalin to decide whether they prefer
to follow your approach instead, and prevent KASLR from mapping DRAM
all the way at the top of the address space. Otherwise, we'll need to
track down all problematic uses of __phys_to_virt() et al, because
there will surely be more.

Thanks,
Ard.



> And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
> if the start >= end, it will not map the hole block address. But the memory in this
> block is valid. And it can be allocated as well.
>
> So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
> range = range / ARM64_MEMSTART_ALIGN;
>
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
> ---
>  arch/arm64/mm/init.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 716d122..60112c0 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
>                  * margin, the size of the region that the available physical
>                  * memory spans, randomize the linear region as well.
>                  */
> -               if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
> -                       range = range / ARM64_MEMSTART_ALIGN + 1;
> -                       memstart_addr -= ARM64_MEMSTART_ALIGN *
> -                                        ((range * memstart_offset_seed) >> 16);
> -               }
> +               if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
> +                       memstart_addr -= (range * memstart_offset_seed) >> 16;
>         }
>
>         /*
> --
> 1.9.1
>

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
@ 2017-11-28 20:41   ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-11-28 20:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 November 2017 at 03:44, Chen Feng <puck.chen@hisilicon.com> wrote:
> With kaslr and kasan enable both, I got the follow issue.
>
> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
> [   16.142517s]___alloc_bootmem_nopanic:257
> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
> [   16.189971s]pgd = ffffffad9c507000
> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
>
> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
>
> memstart_addr 0
> ARM64_MEMSTART_ALIGN 0x40000000
> memstart_offset_seed 0xffc7
> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
>
> reg->base = 0x100000000  -> 0xffffffff40000000
> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
>

This looks a bit weird because we add the PAGE_OFFSET, but it simply
wraps at the top of the address space.

So this code in kasan_init()

void *start = (void *)__phys_to_virt(reg->base);
void *end = (void *)__phys_to_virt(reg->base + reg->size);

if (start >= end)
     break;

is essentially incorrect, because it translates an address that is
strictly outside of the current memblock region. If the KASLR code
happens to map DRAM all the way at the top of the linear region (which
is what occurs in your case), end - 1 is the last valid address.

So I think the minimal correct fix would be

diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
index acba49fb5aac..3214aa9d90be 100644
--- a/arch/arm64/mm/kasan_init.c
+++ b/arch/arm64/mm/kasan_init.c
@@ -216,7 +216,7 @@ void __init kasan_init(void)

        for_each_memblock(memory, reg) {
                void *start = (void *)__phys_to_virt(reg->base);
-               void *end = (void *)__phys_to_virt(reg->base + reg->size);
+               void *end = start + reg->size;

                if (start >= end)
                        break;

given that mappings in the linear region are congruent with the
underlying physical regions (unless I am missing something wrt special
start/end values in memblock, but in that case, they should not be p2v
translated before the evaluation)

However, since having DRAM at the very top appears to break other things as well

vmemmap : 0xffffffbf00000000 - 0xffffffc000000000   (     4 GB maximum)
          0xffffffbfff000000 - 0xffffffbf00000000   (17592186040336 MB actual)
memory  : 0xffffffffc0000000 - 0x               0   (  1024 MB)

I will leave it to Will and/or Catalin to decide whether they prefer
to follow your approach instead, and prevent KASLR from mapping DRAM
all the way at the top of the address space. Otherwise, we'll need to
track down all problematic uses of __phys_to_virt() et al, because
there will surely be more.

Thanks,
Ard.



> And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
> if the start >= end, it will not map the hole block address. But the memory in this
> block is valid. And it can be allocated as well.
>
> So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
> range = range / ARM64_MEMSTART_ALIGN;
>
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
> ---
>  arch/arm64/mm/init.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 716d122..60112c0 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
>                  * margin, the size of the region that the available physical
>                  * memory spans, randomize the linear region as well.
>                  */
> -               if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
> -                       range = range / ARM64_MEMSTART_ALIGN + 1;
> -                       memstart_addr -= ARM64_MEMSTART_ALIGN *
> -                                        ((range * memstart_offset_seed) >> 16);
> -               }
> +               if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
> +                       memstart_addr -= (range * memstart_offset_seed) >> 16;
>         }
>
>         /*
> --
> 1.9.1
>

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
  2017-11-28 20:41   ` Ard Biesheuvel
@ 2017-11-29  8:57     ` Chen Feng
  -1 siblings, 0 replies; 12+ messages in thread
From: Chen Feng @ 2017-11-29  8:57 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel,
	Dan Zhao, Zhuangluan Su, Yiping Xu, chenxiang9, chenya99, houyu3,
	xuyoujun4



On 2017/11/29 4:41, Ard Biesheuvel wrote:
> On 21 November 2017 at 03:44, Chen Feng <puck.chen@hisilicon.com> wrote:
>> With kaslr and kasan enable both, I got the follow issue.
>>
>> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
>> [   16.142517s]___alloc_bootmem_nopanic:257
>> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
>> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
>> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
>> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
>> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
>> [   16.189971s]pgd = ffffffad9c507000
>> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
>>
>> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
>>
>> memstart_addr 0
>> ARM64_MEMSTART_ALIGN 0x40000000
>> memstart_offset_seed 0xffc7
>> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
>>
>> reg->base = 0x100000000  -> 0xffffffff40000000
>> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
>>
> 
> This looks a bit weird because we add the PAGE_OFFSET, but it simply
> wraps at the top of the address space.
> 
> So this code in kasan_init()
> 
> void *start = (void *)__phys_to_virt(reg->base);
> void *end = (void *)__phys_to_virt(reg->base + reg->size);
> 
> if (start >= end)
>      break;
> 
> is essentially incorrect, because it translates an address that is
> strictly outside of the current memblock region. If the KASLR code
> happens to map DRAM all the way at the top of the linear region (which
> is what occurs in your case), end - 1 is the last valid address.
> 
> So I think the minimal correct fix would be
> 
> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
> index acba49fb5aac..3214aa9d90be 100644
> --- a/arch/arm64/mm/kasan_init.c
> +++ b/arch/arm64/mm/kasan_init.c
> @@ -216,7 +216,7 @@ void __init kasan_init(void)
> 
>         for_each_memblock(memory, reg) {
>                 void *start = (void *)__phys_to_virt(reg->base);
> -               void *end = (void *)__phys_to_virt(reg->base + reg->size);
> +               void *end = start + reg->size;
> 
>                 if (start >= end)
>                         break;
> 
> given that mappings in the linear region are congruent with the
> underlying physical regions (unless I am missing something wrt special
> start/end values in memblock, but in that case, they should not be p2v
> translated before the evaluation)
> 
> However, since having DRAM at the very top appears to break other things as well
> 
> vmemmap : 0xffffffbf00000000 - 0xffffffc000000000   (     4 GB maximum)
>           0xffffffbfff000000 - 0xffffffbf00000000   (17592186040336 MB actual)
> memory  : 0xffffffffc0000000 - 0x               0   (  1024 MB)
> 

Yes. I also think kasan code is actually incorrect.
But the memory will never reach the top before. So not only kasan, the p2v all has this
limit. Like the message you post above.
Thinking from this, I modify the logic of kaslr to keep it as it is.


> I will leave it to Will and/or Catalin to decide whether they prefer
> to follow your approach instead, and prevent KASLR from mapping DRAM
> all the way at the top of the address space. Otherwise, we'll need to
> track down all problematic uses of __phys_to_virt() et al, because
> there will surely be more.
> 
> Thanks,
> Ard.
> 
> 
> 
>> And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
>> if the start >= end, it will not map the hole block address. But the memory in this
>> block is valid. And it can be allocated as well.
>>
>> So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
>> range = range / ARM64_MEMSTART_ALIGN;
>>
>> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
>> Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
>> ---
>>  arch/arm64/mm/init.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 716d122..60112c0 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
>>                  * margin, the size of the region that the available physical
>>                  * memory spans, randomize the linear region as well.
>>                  */
>> -               if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
>> -                       range = range / ARM64_MEMSTART_ALIGN + 1;
>> -                       memstart_addr -= ARM64_MEMSTART_ALIGN *
>> -                                        ((range * memstart_offset_seed) >> 16);
>> -               }
>> +               if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
>> +                       memstart_addr -= (range * memstart_offset_seed) >> 16;
>>         }
>>
>>         /*
>> --
>> 1.9.1
>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
@ 2017-11-29  8:57     ` Chen Feng
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Feng @ 2017-11-29  8:57 UTC (permalink / raw)
  To: linux-arm-kernel



On 2017/11/29 4:41, Ard Biesheuvel wrote:
> On 21 November 2017 at 03:44, Chen Feng <puck.chen@hisilicon.com> wrote:
>> With kaslr and kasan enable both, I got the follow issue.
>>
>> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
>> [   16.142517s]___alloc_bootmem_nopanic:257
>> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
>> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
>> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
>> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
>> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
>> [   16.189971s]pgd = ffffffad9c507000
>> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
>>
>> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
>>
>> memstart_addr 0
>> ARM64_MEMSTART_ALIGN 0x40000000
>> memstart_offset_seed 0xffc7
>> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
>>
>> reg->base = 0x100000000  -> 0xffffffff40000000
>> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
>>
> 
> This looks a bit weird because we add the PAGE_OFFSET, but it simply
> wraps at the top of the address space.
> 
> So this code in kasan_init()
> 
> void *start = (void *)__phys_to_virt(reg->base);
> void *end = (void *)__phys_to_virt(reg->base + reg->size);
> 
> if (start >= end)
>      break;
> 
> is essentially incorrect, because it translates an address that is
> strictly outside of the current memblock region. If the KASLR code
> happens to map DRAM all the way at the top of the linear region (which
> is what occurs in your case), end - 1 is the last valid address.
> 
> So I think the minimal correct fix would be
> 
> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
> index acba49fb5aac..3214aa9d90be 100644
> --- a/arch/arm64/mm/kasan_init.c
> +++ b/arch/arm64/mm/kasan_init.c
> @@ -216,7 +216,7 @@ void __init kasan_init(void)
> 
>         for_each_memblock(memory, reg) {
>                 void *start = (void *)__phys_to_virt(reg->base);
> -               void *end = (void *)__phys_to_virt(reg->base + reg->size);
> +               void *end = start + reg->size;
> 
>                 if (start >= end)
>                         break;
> 
> given that mappings in the linear region are congruent with the
> underlying physical regions (unless I am missing something wrt special
> start/end values in memblock, but in that case, they should not be p2v
> translated before the evaluation)
> 
> However, since having DRAM at the very top appears to break other things as well
> 
> vmemmap : 0xffffffbf00000000 - 0xffffffc000000000   (     4 GB maximum)
>           0xffffffbfff000000 - 0xffffffbf00000000   (17592186040336 MB actual)
> memory  : 0xffffffffc0000000 - 0x               0   (  1024 MB)
> 

Yes. I also think kasan code is actually incorrect.
But the memory will never reach the top before. So not only kasan, the p2v all has this
limit. Like the message you post above.
Thinking from this, I modify the logic of kaslr to keep it as it is.


> I will leave it to Will and/or Catalin to decide whether they prefer
> to follow your approach instead, and prevent KASLR from mapping DRAM
> all the way at the top of the address space. Otherwise, we'll need to
> track down all problematic uses of __phys_to_virt() et al, because
> there will surely be more.
> 
> Thanks,
> Ard.
> 
> 
> 
>> And In memblock it use "start_addr + size" as the end addr. So in function kasan_init,
>> if the start >= end, it will not map the hole block address. But the memory in this
>> block is valid. And it can be allocated as well.
>>
>> So donot use the last memory region. Changing "range = range / ARM64_MEMSTART_ALIGN + 1" to
>> range = range / ARM64_MEMSTART_ALIGN;
>>
>> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
>> Signed-off-by: Chen Xiang <chenxiang9@huawei.com>
>> ---
>>  arch/arm64/mm/init.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 716d122..60112c0 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -267,11 +267,8 @@ void __init arm64_memblock_init(void)
>>                  * margin, the size of the region that the available physical
>>                  * memory spans, randomize the linear region as well.
>>                  */
>> -               if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN) {
>> -                       range = range / ARM64_MEMSTART_ALIGN + 1;
>> -                       memstart_addr -= ARM64_MEMSTART_ALIGN *
>> -                                        ((range * memstart_offset_seed) >> 16);
>> -               }
>> +               if (memstart_offset_seed > 0 && range >= ARM64_MEMSTART_ALIGN)
>> +                       memstart_addr -= (range * memstart_offset_seed) >> 16;
>>         }
>>
>>         /*
>> --
>> 1.9.1
>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
  2017-11-28 20:41   ` Ard Biesheuvel
@ 2017-11-29 10:33     ` Ard Biesheuvel
  -1 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-11-29 10:33 UTC (permalink / raw)
  To: Chen Feng
  Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel,
	Dan Zhao, Zhuangluan Su, Yiping Xu, chenxiang9, chenya99, houyu3,
	xuyoujun4

On 28 November 2017 at 20:41, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 21 November 2017 at 03:44, Chen Feng <puck.chen@hisilicon.com> wrote:
>> With kaslr and kasan enable both, I got the follow issue.
>>
>> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
>> [   16.142517s]___alloc_bootmem_nopanic:257
>> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
>> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
>> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
>> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
>> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
>> [   16.189971s]pgd = ffffffad9c507000
>> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
>>
>> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
>>
>> memstart_addr 0
>> ARM64_MEMSTART_ALIGN 0x40000000
>> memstart_offset_seed 0xffc7
>> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
>>
>> reg->base = 0x100000000  -> 0xffffffff40000000
>> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
>>
>
> This looks a bit weird because we add the PAGE_OFFSET, but it simply
> wraps at the top of the address space.
>
> So this code in kasan_init()
>
> void *start = (void *)__phys_to_virt(reg->base);
> void *end = (void *)__phys_to_virt(reg->base + reg->size);
>
> if (start >= end)
>      break;
>
> is essentially incorrect, because it translates an address that is
> strictly outside of the current memblock region. If the KASLR code
> happens to map DRAM all the way at the top of the linear region (which
> is what occurs in your case), end - 1 is the last valid address.
>
> So I think the minimal correct fix would be
>
> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
> index acba49fb5aac..3214aa9d90be 100644
> --- a/arch/arm64/mm/kasan_init.c
> +++ b/arch/arm64/mm/kasan_init.c
> @@ -216,7 +216,7 @@ void __init kasan_init(void)
>
>         for_each_memblock(memory, reg) {
>                 void *start = (void *)__phys_to_virt(reg->base);
> -               void *end = (void *)__phys_to_virt(reg->base + reg->size);
> +               void *end = start + reg->size;
>
>                 if (start >= end)
>                         break;
>

This does not actually fix the problem, end will be 0x0 which is still
< start. We'll need a '-1' in there somewhere to really fix it.

> given that mappings in the linear region are congruent with the
> underlying physical regions (unless I am missing something wrt special
> start/end values in memblock, but in that case, they should not be p2v
> translated before the evaluation)
>
> However, since having DRAM at the very top appears to break other things as well
>
> vmemmap : 0xffffffbf00000000 - 0xffffffc000000000   (     4 GB maximum)
>           0xffffffbfff000000 - 0xffffffbf00000000   (17592186040336 MB actual)
> memory  : 0xffffffffc0000000 - 0x               0   (  1024 MB)
>
> I will leave it to Will and/or Catalin to decide whether they prefer
> to follow your approach instead, and prevent KASLR from mapping DRAM
> all the way at the top of the address space. Otherwise, we'll need to
> track down all problematic uses of __phys_to_virt() et al, because
> there will surely be more.
>

I guess it takes quite a bit of discipline to deal with overflow in
all 'end' calculations, although I suppose this is more common on
32-bit architectures.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr
@ 2017-11-29 10:33     ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-11-29 10:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 28 November 2017 at 20:41, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 21 November 2017 at 03:44, Chen Feng <puck.chen@hisilicon.com> wrote:
>> With kaslr and kasan enable both, I got the follow issue.
>>
>> [   16.130523s]kasan: reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000
>> [   16.142517s]___alloc_bootmem_nopanic:257
>> [   16.148284s]__alloc_memory_core_early:63, addr = 197fc7fc0
>> [   16.155670s]__alloc_memory_core_early:65, virt = ffffffffd7fc7fc0
>> [   16.163635s]__alloc_memory_core_early:67, toshow = ffffff8ffaff8ff8
>> [   16.171783s]__alloc_memory_core_early:69, show_phy = ffffffe2649f8ff8
>> [   16.180145s]Unable to handle kernel paging request at virtual address ffffff8ffaff8ff8
>> [   16.189971s]pgd = ffffffad9c507000
>> [   16.195220s][ffffff8ffaff8ff8] *pgd=0000000197fc8003, *pud=0000000197fc8003
>>
>> *reg->base = 100000000, phys_end =1c0000000,start = ffffffff40000000, end = ffffffc000000000*
>>
>> memstart_addr 0
>> ARM64_MEMSTART_ALIGN 0x40000000
>> memstart_offset_seed 0xffc7
>> PHYS_OFFSET = 0 - memstart_addr = 0 - 3E40000000 = FFFFFFC1C0000000
>>
>> reg->base = 0x100000000  -> 0xffffffff40000000
>> phys_end  = 0x1c0000000  -> 0xffffffc000000000  This is confused, end less than start.
>>
>
> This looks a bit weird because we add the PAGE_OFFSET, but it simply
> wraps at the top of the address space.
>
> So this code in kasan_init()
>
> void *start = (void *)__phys_to_virt(reg->base);
> void *end = (void *)__phys_to_virt(reg->base + reg->size);
>
> if (start >= end)
>      break;
>
> is essentially incorrect, because it translates an address that is
> strictly outside of the current memblock region. If the KASLR code
> happens to map DRAM all the way at the top of the linear region (which
> is what occurs in your case), end - 1 is the last valid address.
>
> So I think the minimal correct fix would be
>
> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
> index acba49fb5aac..3214aa9d90be 100644
> --- a/arch/arm64/mm/kasan_init.c
> +++ b/arch/arm64/mm/kasan_init.c
> @@ -216,7 +216,7 @@ void __init kasan_init(void)
>
>         for_each_memblock(memory, reg) {
>                 void *start = (void *)__phys_to_virt(reg->base);
> -               void *end = (void *)__phys_to_virt(reg->base + reg->size);
> +               void *end = start + reg->size;
>
>                 if (start >= end)
>                         break;
>

This does not actually fix the problem, end will be 0x0 which is still
< start. We'll need a '-1' in there somewhere to really fix it.

> given that mappings in the linear region are congruent with the
> underlying physical regions (unless I am missing something wrt special
> start/end values in memblock, but in that case, they should not be p2v
> translated before the evaluation)
>
> However, since having DRAM at the very top appears to break other things as well
>
> vmemmap : 0xffffffbf00000000 - 0xffffffc000000000   (     4 GB maximum)
>           0xffffffbfff000000 - 0xffffffbf00000000   (17592186040336 MB actual)
> memory  : 0xffffffffc0000000 - 0x               0   (  1024 MB)
>
> I will leave it to Will and/or Catalin to decide whether they prefer
> to follow your approach instead, and prevent KASLR from mapping DRAM
> all the way at the top of the address space. Otherwise, we'll need to
> track down all problematic uses of __phys_to_virt() et al, because
> there will surely be more.
>

I guess it takes quite a bit of discipline to deal with overflow in
all 'end' calculations, although I suppose this is more common on
32-bit architectures.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-11-29 10:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-21  3:44 [PATCH] arm64: kaslr: Fix kaslr end boundary of virt addr Chen Feng
2017-11-21  3:44 ` Chen Feng
2017-11-21  8:37 ` Chen Feng
2017-11-21  8:37   ` Chen Feng
2017-11-28 18:35 ` Will Deacon
2017-11-28 18:35   ` Will Deacon
2017-11-28 20:41 ` Ard Biesheuvel
2017-11-28 20:41   ` Ard Biesheuvel
2017-11-29  8:57   ` Chen Feng
2017-11-29  8:57     ` Chen Feng
2017-11-29 10:33   ` Ard Biesheuvel
2017-11-29 10:33     ` Ard Biesheuvel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.