[PATCH 1/3] powerpc/mm/radix: Refactor radix__mark_rodata_ro()

[PATCH 1/3] powerpc/mm/radix: Refactor radix__mark_rodata_ro()

[Michael Ellerman]

Move the core logic into a helper, so we can use it for changing permissions
other than _PAGE_WRITE.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/mm/pgtable-radix.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
index 8c13e4282308..336e52ec652c 100644
--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c
@@ -112,10 +112,9 @@ int radix__map_kernel_page(unsigned long ea, unsigned long pa,
 }
 
 #ifdef CONFIG_STRICT_KERNEL_RWX
-void radix__mark_rodata_ro(void)
+void radix__change_memory_range(unsigned long start, unsigned long end,
+				unsigned long clear)
 {
-	unsigned long start = (unsigned long)_stext;
-	unsigned long end = (unsigned long)__init_begin;
 	unsigned long idx;
 	pgd_t *pgdp;
 	pud_t *pudp;
@@ -125,7 +124,8 @@ void radix__mark_rodata_ro(void)
 	start = ALIGN_DOWN(start, PAGE_SIZE);
 	end = PAGE_ALIGN(end); // aligns up
 
-	pr_devel("marking ro start %lx, end %lx\n", start, end);
+	pr_debug("Changing flags on range %lx-%lx removing 0x%lx\n",
+		 start, end, clear);
 
 	for (idx = start; idx < end; idx += PAGE_SIZE) {
 		pgdp = pgd_offset_k(idx);
@@ -147,11 +147,21 @@ void radix__mark_rodata_ro(void)
 		if (!ptep)
 			continue;
 update_the_pte:
-		radix__pte_update(&init_mm, idx, ptep, _PAGE_WRITE, 0, 0);
+		radix__pte_update(&init_mm, idx, ptep, clear, 0, 0);
 	}
 
 	radix__flush_tlb_kernel_range(start, end);
 }
+
+void radix__mark_rodata_ro(void)
+{
+	unsigned long start, end;
+
+	start = (unsigned long)_stext;
+	end = (unsigned long)__init_begin;
+
+	radix__change_memory_range(start, end, _PAGE_WRITE);
+}
 #endif /* CONFIG_STRICT_KERNEL_RWX */
 
 static inline void __meminit print_mapping(unsigned long start,
-- 
2.7.4

[PATCH 2/3] powerpc/mm/hash: Refactor hash__mark_rodata_ro()

[Michael Ellerman]

Move the core logic into a helper, so we can use it for changing other
permissions.

We also change the logic to align start down, and end up. This means
calling the function with a range will expand that range to be at
least 1 mmu_linear_psize page in size. We need that so we can use it
on __init_begin ...  __init_end which is not a full page in size.

This should always work for _stext/__init_begin, because we align
__init_begin to _stext + 16M in the linker script.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/mm/pgtable-hash64.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/mm/pgtable-hash64.c b/arch/powerpc/mm/pgtable-hash64.c
index 188b4107584d..73019c52141f 100644
--- a/arch/powerpc/mm/pgtable-hash64.c
+++ b/arch/powerpc/mm/pgtable-hash64.c
@@ -425,33 +425,39 @@ int hash__has_transparent_hugepage(void)
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
 #ifdef CONFIG_STRICT_KERNEL_RWX
-void hash__mark_rodata_ro(void)
+static bool hash__change_memory_range(unsigned long start, unsigned long end,
+				      unsigned long newpp)
 {
-	unsigned long start = (unsigned long)_stext;
-	unsigned long end = (unsigned long)__init_begin;
 	unsigned long idx;
 	unsigned int step, shift;
-	unsigned long newpp = PP_RXXX;
 
 	shift = mmu_psize_defs[mmu_linear_psize].shift;
 	step = 1 << shift;
 
-	start = ((start + step - 1) >> shift) << shift;
-	end = (end >> shift) << shift;
+	start = ALIGN_DOWN(start, step);
+	end = ALIGN(end, step); // aligns up
 
-	pr_devel("marking ro start %lx, end %lx, step %x\n",
-			start, end, step);
+	if (start >= end)
+		return false;
 
-	if (start == end) {
-		pr_warn("could not set rodata ro, relocate the start"
-			" of the kernel to a 0x%x boundary\n", step);
-		return;
-	}
+	pr_debug("Changing page protection on range 0x%lx-0x%lx, to 0x%lx, step 0x%x\n",
+		 start, end, newpp, step);
 
 	for (idx = start; idx < end; idx += step)
 		/* Not sure if we can do much with the return value */
 		mmu_hash_ops.hpte_updateboltedpp(newpp, idx, mmu_linear_psize,
 							mmu_kernel_ssize);
 
+	return true;
+}
+
+void hash__mark_rodata_ro(void)
+{
+	unsigned long start, end;
+
+	start = (unsigned long)_stext;
+	end = (unsigned long)__init_begin;
+
+	WARN_ON(!hash__change_memory_range(start, end, PP_RXXX));
 }
 #endif
-- 
2.7.4

[PATCH 3/3] powerpc/mm: Mark __init memory no-execute when STRICT_KERNEL_RWX=y

[Michael Ellerman]

Currently even with STRICT_KERNEL_RWX we leave the __init text marked
executable after init, which is bad.

Add a hook to mark it NX (no-execute) before we free it, and implement
it for radix and hash.

Note that we use __init_end as the end address, not _einittext,
because overlaps_kernel_text() uses __init_end, because there are
additional executable sections other than .init.text between
__init_begin and __init_end.

Tested on radix and hash with:

  0:mon> p $__init_begin
  *** 400 exception occurred

Fixes: 1e0fc9d1eb2b ("powerpc/Kconfig: Enable STRICT_KERNEL_RWX for some configs")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/book3s/64/hash.h    |  1 +
 arch/powerpc/include/asm/book3s/64/pgtable.h |  7 +++++++
 arch/powerpc/include/asm/book3s/64/radix.h   |  1 +
 arch/powerpc/mm/mem.c                        |  1 +
 arch/powerpc/mm/pgtable-hash64.c             | 12 ++++++++++++
 arch/powerpc/mm/pgtable-radix.c              |  8 ++++++++
 arch/powerpc/mm/pgtable_64.c                 |  8 ++++++++
 7 files changed, 38 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
index 0ce513f2926f..36fc7bfe9e11 100644
--- a/arch/powerpc/include/asm/book3s/64/hash.h
+++ b/arch/powerpc/include/asm/book3s/64/hash.h
@@ -91,6 +91,7 @@ static inline int hash__pgd_bad(pgd_t pgd)
 }
 #ifdef CONFIG_STRICT_KERNEL_RWX
 extern void hash__mark_rodata_ro(void);
+extern void hash__mark_initmem_nx(void);
 #endif
 
 extern void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index c0737c86a362..3d562b210c65 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1192,5 +1192,12 @@ static inline const int pud_pfn(pud_t pud)
 	BUILD_BUG();
 	return 0;
 }
+
+#ifdef CONFIG_STRICT_KERNEL_RWX
+void mark_initmem_nx(void);
+#else
+static inline void mark_initmem_nx(void) { }
+#endif
+
 #endif /* __ASSEMBLY__ */
 #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h b/arch/powerpc/include/asm/book3s/64/radix.h
index 487709ff6875..544440b5aff3 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -118,6 +118,7 @@
 
 #ifdef CONFIG_STRICT_KERNEL_RWX
 extern void radix__mark_rodata_ro(void);
+extern void radix__mark_initmem_nx(void);
 #endif
 
 static inline unsigned long __radix_pte_update(pte_t *ptep, unsigned long clr,
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 8541f18694a4..46b4e67d2372 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -402,6 +402,7 @@ void __init mem_init(void)
 void free_initmem(void)
 {
 	ppc_md.progress = ppc_printk_progress;
+	mark_initmem_nx();
 	free_initmem_default(POISON_FREE_INITMEM);
 }
 
diff --git a/arch/powerpc/mm/pgtable-hash64.c b/arch/powerpc/mm/pgtable-hash64.c
index 73019c52141f..443a2c66a304 100644
--- a/arch/powerpc/mm/pgtable-hash64.c
+++ b/arch/powerpc/mm/pgtable-hash64.c
@@ -460,4 +460,16 @@ void hash__mark_rodata_ro(void)
 
 	WARN_ON(!hash__change_memory_range(start, end, PP_RXXX));
 }
+
+void hash__mark_initmem_nx(void)
+{
+	unsigned long start, end, pp;
+
+	start = (unsigned long)__init_begin;
+	end = (unsigned long)__init_end;
+
+	pp = htab_convert_pte_flags(pgprot_val(PAGE_KERNEL));
+
+	WARN_ON(!hash__change_memory_range(start, end, pp));
+}
 #endif
diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
index 336e52ec652c..5cc50d47ce3f 100644
--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c
@@ -162,6 +162,14 @@ void radix__mark_rodata_ro(void)
 
 	radix__change_memory_range(start, end, _PAGE_WRITE);
 }
+
+void radix__mark_initmem_nx(void)
+{
+	unsigned long start = (unsigned long)__init_begin;
+	unsigned long end = (unsigned long)__init_end;
+
+	radix__change_memory_range(start, end, _PAGE_EXEC);
+}
 #endif /* CONFIG_STRICT_KERNEL_RWX */
 
 static inline void __meminit print_mapping(unsigned long start,
diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
index 5c0b795d656c..0736e94c7615 100644
--- a/arch/powerpc/mm/pgtable_64.c
+++ b/arch/powerpc/mm/pgtable_64.c
@@ -505,4 +505,12 @@ void mark_rodata_ro(void)
 	else
 		hash__mark_rodata_ro();
 }
+
+void mark_initmem_nx(void)
+{
+	if (radix_enabled())
+		radix__mark_initmem_nx();
+	else
+		hash__mark_initmem_nx();
+}
 #endif
-- 
2.7.4

Re: [PATCH 3/3] powerpc/mm: Mark __init memory no-execute when STRICT_KERNEL_RWX=y

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]

Hi Michael,

[auto build test ERROR on powerpc/next]
[also build test ERROR on next-20170714]
[cannot apply to v4.12]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michael-Ellerman/powerpc-mm-radix-Refactor-radix__mark_rodata_ro/20170715-043340
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/mm/mem.c: In function 'free_initmem':
>> arch/powerpc/mm/mem.c:413:2: error: implicit declaration of function 'mark_initmem_nx' [-Werror=implicit-function-declaration]
     mark_initmem_nx();
     ^~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors

vim +/mark_initmem_nx +413 arch/powerpc/mm/mem.c

   409	
   410	void free_initmem(void)
   411	{
   412		ppc_md.progress = ppc_printk_progress;
 > 413		mark_initmem_nx();
   414		free_initmem_default(POISON_FREE_INITMEM);
   415	}
   416	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6292 bytes --]

Re: [PATCH 1/3] powerpc/mm/radix: Refactor radix__mark_rodata_ro()

[Balbir Singh]

On Fri, Jul 14, 2017 at 4:51 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Move the core logic into a helper, so we can use it for changing permissions
> other than _PAGE_WRITE.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/mm/pgtable-radix.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
> index 8c13e4282308..336e52ec652c 100644
> --- a/arch/powerpc/mm/pgtable-radix.c
> +++ b/arch/powerpc/mm/pgtable-radix.c
> @@ -112,10 +112,9 @@ int radix__map_kernel_page(unsigned long ea, unsigned long pa,
>  }
>
>  #ifdef CONFIG_STRICT_KERNEL_RWX
> -void radix__mark_rodata_ro(void)
> +void radix__change_memory_range(unsigned long start, unsigned long end,
> +                               unsigned long clear)
>  {
> -       unsigned long start = (unsigned long)_stext;
> -       unsigned long end = (unsigned long)__init_begin;
>         unsigned long idx;
>         pgd_t *pgdp;
>         pud_t *pudp;
> @@ -125,7 +124,8 @@ void radix__mark_rodata_ro(void)
>         start = ALIGN_DOWN(start, PAGE_SIZE);
>         end = PAGE_ALIGN(end); // aligns up
>
> -       pr_devel("marking ro start %lx, end %lx\n", start, end);
> +       pr_debug("Changing flags on range %lx-%lx removing 0x%lx\n",
> +                start, end, clear);
>
>         for (idx = start; idx < end; idx += PAGE_SIZE) {
>                 pgdp = pgd_offset_k(idx);
> @@ -147,11 +147,21 @@ void radix__mark_rodata_ro(void)
>                 if (!ptep)
>                         continue;
>  update_the_pte:
> -               radix__pte_update(&init_mm, idx, ptep, _PAGE_WRITE, 0, 0);
> +               radix__pte_update(&init_mm, idx, ptep, clear, 0, 0);
>         }
>
>         radix__flush_tlb_kernel_range(start, end);
>  }
> +
> +void radix__mark_rodata_ro(void)
> +{
> +       unsigned long start, end;
> +
> +       start = (unsigned long)_stext;
> +       end = (unsigned long)__init_begin;
> +
> +       radix__change_memory_range(start, end, _PAGE_WRITE);
> +}
>  #endif /* CONFIG_STRICT_KERNEL_RWX */
>

Looks good, I had something similar for the patches to enable
relocation, but that's a
a larger chunk.

Reviewed-by: Balbir Singh <bsingharora@gmail.com>

Re: [PATCH 2/3] powerpc/mm/hash: Refactor hash__mark_rodata_ro()

[Balbir Singh]

On Fri, Jul 14, 2017 at 4:51 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Move the core logic into a helper, so we can use it for changing other
> permissions.
>
> We also change the logic to align start down, and end up. This means
> calling the function with a range will expand that range to be at
> least 1 mmu_linear_psize page in size. We need that so we can use it
> on __init_begin ...  __init_end which is not a full page in size.
>
> This should always work for _stext/__init_begin, because we align
> __init_begin to _stext + 16M in the linker script.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---

Reviewed-by: Balbir Singh <bsingharora@gmail.com>

Re: [PATCH 3/3] powerpc/mm: Mark __init memory no-execute when STRICT_KERNEL_RWX=y

[Michael Ellerman]

kbuild test robot <lkp@intel.com> writes:

> Hi Michael,
>
> [auto build test ERROR on powerpc/next]
> [also build test ERROR on next-20170714]
> [cannot apply to v4.12]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Michael-Ellerman/powerpc-mm-radix-Refactor-radix__mark_rodata_ro/20170715-043340
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: powerpc-allnoconfig (attached as .config)
> compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=powerpc 
>
> All errors (new ones prefixed by >>):
>
>    arch/powerpc/mm/mem.c: In function 'free_initmem':
>>> arch/powerpc/mm/mem.c:413:2: error: implicit declaration of function 'mark_initmem_nx' [-Werror=implicit-function-declaration]
>      mark_initmem_nx();
>      ^~~~~~~~~~~~~~~
>    cc1: all warnings being treated as errors

Gah, 32-bit.

Fixed with:

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 3d562b210c65..d1da415e283c 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1193,11 +1193,5 @@ static inline const int pud_pfn(pud_t pud)
 	return 0;
 }
 
-#ifdef CONFIG_STRICT_KERNEL_RWX
-void mark_initmem_nx(void);
-#else
-static inline void mark_initmem_nx(void) { }
-#endif
-
 #endif /* __ASSEMBLY__ */
 #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index dd01212935ac..afae9a336136 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -80,6 +80,13 @@ unsigned long vmalloc_to_phys(void *vmalloc_addr);
 
 void pgtable_cache_add(unsigned shift, void (*ctor)(void *));
 void pgtable_cache_init(void);
+
+#ifdef CONFIG_STRICT_KERNEL_RWX
+void mark_initmem_nx(void);
+#else
+static inline void mark_initmem_nx(void) { }
+#endif
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_PGTABLE_H */

Re: [1/3] powerpc/mm/radix: Refactor radix__mark_rodata_ro()

[Michael Ellerman]

On Fri, 2017-07-14 at 06:51:21 UTC, Michael Ellerman wrote:
> Move the core logic into a helper, so we can use it for changing permissions
> other than _PAGE_WRITE.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Balbir Singh <bsingharora@gmail.com>

Series applied to powerpc fixes.

https://git.kernel.org/powerpc/c/b134bd90286dc9f2952c35a91ab405

cheers

Re: [PATCH 3/3] powerpc/mm: Mark __init memory no-execute when STRICT_KERNEL_RWX=y

[Christophe LEROY]

Le 14/07/2017 à 08:51, Michael Ellerman a écrit :
> Currently even with STRICT_KERNEL_RWX we leave the __init text marked
> executable after init, which is bad.
> 
> Add a hook to mark it NX (no-execute) before we free it, and implement
> it for radix and hash.
> 
> Note that we use __init_end as the end address, not _einittext,
> because overlaps_kernel_text() uses __init_end, because there are
> additional executable sections other than .init.text between
> __init_begin and __init_end.
> 
> Tested on radix and hash with:
> 
>    0:mon> p $__init_begin
>    *** 400 exception occurred
> 
> Fixes: 1e0fc9d1eb2b ("powerpc/Kconfig: Enable STRICT_KERNEL_RWX for some configs")
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>   arch/powerpc/include/asm/book3s/64/hash.h    |  1 +
>   arch/powerpc/include/asm/book3s/64/pgtable.h |  7 +++++++
>   arch/powerpc/include/asm/book3s/64/radix.h   |  1 +
>   arch/powerpc/mm/mem.c                        |  1 +
>   arch/powerpc/mm/pgtable-hash64.c             | 12 ++++++++++++
>   arch/powerpc/mm/pgtable-radix.c              |  8 ++++++++
>   arch/powerpc/mm/pgtable_64.c                 |  8 ++++++++
>   7 files changed, 38 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
> index 0ce513f2926f..36fc7bfe9e11 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash.h
> @@ -91,6 +91,7 @@ static inline int hash__pgd_bad(pgd_t pgd)
>   }
>   #ifdef CONFIG_STRICT_KERNEL_RWX
>   extern void hash__mark_rodata_ro(void);
> +extern void hash__mark_initmem_nx(void);
>   #endif
>   
>   extern void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index c0737c86a362..3d562b210c65 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -1192,5 +1192,12 @@ static inline const int pud_pfn(pud_t pud)
>   	BUILD_BUG();
>   	return 0;
>   }
> +
> +#ifdef CONFIG_STRICT_KERNEL_RWX
> +void mark_initmem_nx(void);
> +#else
> +static inline void mark_initmem_nx(void) { }
> +#endif
> +

Why do we want to limit that to CONFIG_STRICT_KERNEL_RWX ?
Only the kernel text is marked X, even without CONFIG_STRICT_KERNEL_RWX 
(at least on PPC32), so I believe we should clear X on init text in any 
case, shouldn't we ?

Christophe

>   #endif /* __ASSEMBLY__ */
>   #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */
> diff --git a/arch/powerpc/include/asm/book3s/64/radix.h b/arch/powerpc/include/asm/book3s/64/radix.h
> index 487709ff6875..544440b5aff3 100644
> --- a/arch/powerpc/include/asm/book3s/64/radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/radix.h
> @@ -118,6 +118,7 @@
>   
>   #ifdef CONFIG_STRICT_KERNEL_RWX
>   extern void radix__mark_rodata_ro(void);
> +extern void radix__mark_initmem_nx(void);
>   #endif
>   
>   static inline unsigned long __radix_pte_update(pte_t *ptep, unsigned long clr,
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 8541f18694a4..46b4e67d2372 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -402,6 +402,7 @@ void __init mem_init(void)
>   void free_initmem(void)
>   {
>   	ppc_md.progress = ppc_printk_progress;
> +	mark_initmem_nx();
>   	free_initmem_default(POISON_FREE_INITMEM);
>   }
>   
> diff --git a/arch/powerpc/mm/pgtable-hash64.c b/arch/powerpc/mm/pgtable-hash64.c
> index 73019c52141f..443a2c66a304 100644
> --- a/arch/powerpc/mm/pgtable-hash64.c
> +++ b/arch/powerpc/mm/pgtable-hash64.c
> @@ -460,4 +460,16 @@ void hash__mark_rodata_ro(void)
>   
>   	WARN_ON(!hash__change_memory_range(start, end, PP_RXXX));
>   }
> +
> +void hash__mark_initmem_nx(void)
> +{
> +	unsigned long start, end, pp;
> +
> +	start = (unsigned long)__init_begin;
> +	end = (unsigned long)__init_end;
> +
> +	pp = htab_convert_pte_flags(pgprot_val(PAGE_KERNEL));
> +
> +	WARN_ON(!hash__change_memory_range(start, end, pp));
> +}
>   #endif
> diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
> index 336e52ec652c..5cc50d47ce3f 100644
> --- a/arch/powerpc/mm/pgtable-radix.c
> +++ b/arch/powerpc/mm/pgtable-radix.c
> @@ -162,6 +162,14 @@ void radix__mark_rodata_ro(void)
>   
>   	radix__change_memory_range(start, end, _PAGE_WRITE);
>   }
> +
> +void radix__mark_initmem_nx(void)
> +{
> +	unsigned long start = (unsigned long)__init_begin;
> +	unsigned long end = (unsigned long)__init_end;
> +
> +	radix__change_memory_range(start, end, _PAGE_EXEC);
> +}
>   #endif /* CONFIG_STRICT_KERNEL_RWX */
>   
>   static inline void __meminit print_mapping(unsigned long start,
> diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
> index 5c0b795d656c..0736e94c7615 100644
> --- a/arch/powerpc/mm/pgtable_64.c
> +++ b/arch/powerpc/mm/pgtable_64.c
> @@ -505,4 +505,12 @@ void mark_rodata_ro(void)
>   	else
>   		hash__mark_rodata_ro();
>   }
> +
> +void mark_initmem_nx(void)
> +{
> +	if (radix_enabled())
> +		radix__mark_initmem_nx();
> +	else
> +		hash__mark_initmem_nx();
> +}
>   #endif
> 

Re: [PATCH 3/3] powerpc/mm: Mark __init memory no-execute when STRICT_KERNEL_RWX=y

[Michael Ellerman]

Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 14/07/2017 =C3=A0 08:51, Michael Ellerman a =C3=A9crit :
>> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc=
/include/asm/book3s/64/pgtable.h
>> index c0737c86a362..3d562b210c65 100644
>> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
>> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
>> @@ -1192,5 +1192,12 @@ static inline const int pud_pfn(pud_t pud)
>>   	BUILD_BUG();
>>   	return 0;
>>   }
>> +
>> +#ifdef CONFIG_STRICT_KERNEL_RWX
>> +void mark_initmem_nx(void);
>> +#else
>> +static inline void mark_initmem_nx(void) { }
>> +#endif
>> +
>
> Why do we want to limit that to CONFIG_STRICT_KERNEL_RWX ?
> Only the kernel text is marked X, even without CONFIG_STRICT_KERNEL_RWX=20
> (at least on PPC32), so I believe we should clear X on init text in any=20
> case, shouldn't we ?

You're right, but ..

On 64-bit when STRICT_KERNEL_RWX=3Dn we make no effort to ensure the
start/end of the init text is on a page boundary.

eg. on 64-bit hash we will typically use a 16M page to map the whole
kernel, text/data/init_text/etc.

So yes we *should* always mark it no-execute but in practice we can't
because it's not page aligned.

But if that's different on (some?) 32-bit then we could introduce a new
CONFIG symbol that is enabled in the right cases.

cheers

Re: [PATCH 3/3] powerpc/mm: Mark __init memory no-execute when STRICT_KERNEL_RWX=y

[Christophe LEROY]

Le 09/08/2017 à 04:29, Michael Ellerman a écrit :
> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>> Le 14/07/2017 à 08:51, Michael Ellerman a écrit :
>>> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
>>> index c0737c86a362..3d562b210c65 100644
>>> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
>>> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
>>> @@ -1192,5 +1192,12 @@ static inline const int pud_pfn(pud_t pud)
>>>    	BUILD_BUG();
>>>    	return 0;
>>>    }
>>> +
>>> +#ifdef CONFIG_STRICT_KERNEL_RWX
>>> +void mark_initmem_nx(void);
>>> +#else
>>> +static inline void mark_initmem_nx(void) { }
>>> +#endif
>>> +
>>
>> Why do we want to limit that to CONFIG_STRICT_KERNEL_RWX ?
>> Only the kernel text is marked X, even without CONFIG_STRICT_KERNEL_RWX
>> (at least on PPC32), so I believe we should clear X on init text in any
>> case, shouldn't we ?
> 
> You're right, but ..
> 
> On 64-bit when STRICT_KERNEL_RWX=n we make no effort to ensure the
> start/end of the init text is on a page boundary.
> 
> eg. on 64-bit hash we will typically use a 16M page to map the whole
> kernel, text/data/init_text/etc.

Some of the 32 bit also use some huge mapings like BATs or large pages, 
in which case it is pointless but not harmfull to fix the page tables 
anyway.
At least it is correct for the ones that use regular pages, and kernel 
can also be started with nobats or noltlbs at command line, in which 
case it is usefull to have the page tables correct.

> 
> So yes we *should* always mark it no-execute but in practice we can't
> because it's not page aligned.

On 32 bit it seems to always be aligned to the normal page size, so no 
problem.

> 
> But if that's different on (some?) 32-bit then we could introduce a new
> CONFIG symbol that is enabled in the right cases.

For the time being, I have added an "|| CONFIG_PPC32 " on the ifdef, is 
that OK ?
See https://patchwork.ozlabs.org/patch/796625/

Christophe

Re: [PATCH 3/3] powerpc/mm: Mark __init memory no-execute when STRICT_KERNEL_RWX=y

[Michael Ellerman]

Christophe LEROY <christophe.leroy@c-s.fr> writes:
...
> At least it is correct for the ones that use regular pages, and kernel 
> can also be started with nobats or noltlbs at command line, in which 
> case it is usefull to have the page tables correct.

Yep OK.

>> So yes we *should* always mark it no-execute but in practice we can't
>> because it's not page aligned.
>
> On 32 bit it seems to always be aligned to the normal page size, so no 
> problem.
>
>> But if that's different on (some?) 32-bit then we could introduce a new
>> CONFIG symbol that is enabled in the right cases.
>
> For the time being, I have added an "|| CONFIG_PPC32 " on the ifdef, is 
> that OK ?
> See https://patchwork.ozlabs.org/patch/796625/

Yeah looks fine.

cheers