diff mbox series

[2/2] powerpc/strict_rwx: fixup region splitting

Message ID 20170927095111.10626-2-bsingharora@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series [1/2] powerpc/strict_rwx: quirks for STRICT_RWX patches | expand

Commit Message

Balbir Singh Sept. 27, 2017, 9:51 a.m. UTC 
We were aggressive with splitting regions and missed the case
when _stext and __init_begin completely overlap addr and addr+mapping.

This patch fixes that case and allows us to keep the largest possible
mapping through the overlap. The patch also simplies the check
into a function

Fixes: 7614ff3 ("powerpc/mm/radix: Implement STRICT_RWX/mark_rodata_ro() for Radix")

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
 arch/powerpc/mm/pgtable-radix.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

Comments

Michael Ellerman Oct. 4, 2017, 11:14 a.m. UTC | #1 
Balbir Singh <bsingharora@gmail.com> writes:

> We were aggressive with splitting regions and missed the case
> when _stext and __init_begin completely overlap addr and addr+mapping.
>
> This patch fixes that case and allows us to keep the largest possible
> mapping through the overlap. The patch also simplies the check
> into a function

Please do the fix in one patch, which we can backport if necessary, and
then move it into a function in a separate patch.

cheers
Balbir Singh Oct. 16, 2017, 1:03 a.m. UTC | #2 
On Wed, 04 Oct 2017 22:14:17 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Balbir Singh <bsingharora@gmail.com> writes:
> 
> > We were aggressive with splitting regions and missed the case
> > when _stext and __init_begin completely overlap addr and addr+mapping.
> >
> > This patch fixes that case and allows us to keep the largest possible
> > mapping through the overlap. The patch also simplies the check
> > into a function  
> 
> Please do the fix in one patch, which we can backport if necessary, and
> then move it into a function in a separate patch.
> 

I've tried for a while now, although this looks like its touching more
code, this is more elegant. Anything else involves changing the two
if conditions almost identically.

Balbir Singh.
diff mbox series

Patch

diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
index c2a2b46..ea0da3b 100644
--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c
@@ -210,17 +210,36 @@  static inline void __meminit print_mapping(unsigned long start,
 	pr_info("Mapped 0x%016lx-0x%016lx with %s pages\n", start, end, buf);
 }
 
+static inline int __meminit
+should_split_mapping_size(unsigned long addr, unsigned long mapping_size)
+{
+#ifdef CONFIG_STRICT_KERNEL_RWX
+	/*
+	 * If addr, addr+mapping overlap the text region and
+	 * _stext and __init_begin end up lying between
+	 * addr, addr+mapping split the mapping size down
+	 * further
+	 */
+	if ((addr < __pa_symbol(__init_begin)) &&
+		(addr + mapping_size) > __pa_symbol(_stext)) {
+
+		if (((addr + mapping_size) <= __pa_symbol(__init_begin)) &&
+			(addr >= __pa_symbol(_stext)))
+			return 0;
+
+		return 1;
+	}
+#endif
+	return 0;
+}
+
+
 static int __meminit create_physical_mapping(unsigned long start,
 					     unsigned long end)
 {
 	unsigned long vaddr, addr, mapping_size = 0;
 	pgprot_t prot;
 	unsigned long max_mapping_size;
-#ifdef CONFIG_STRICT_KERNEL_RWX
-	int split_text_mapping = 1;
-#else
-	int split_text_mapping = 0;
-#endif
 
 	start = _ALIGN_UP(start, PAGE_SIZE);
 	for (addr = start; addr < end; addr += mapping_size) {
@@ -242,16 +261,14 @@  static int __meminit create_physical_mapping(unsigned long start,
 		else
 			mapping_size = PAGE_SIZE;
 
-		if (split_text_mapping && (mapping_size == PUD_SIZE) &&
-			(addr <= __pa_symbol(__init_begin)) &&
-			(addr + mapping_size) >= __pa_symbol(_stext)) {
+		if ((mapping_size == PUD_SIZE) &&
+			should_split_mapping_size(addr, mapping_size)) {
 			max_mapping_size = PMD_SIZE;
 			goto retry;
 		}
 
-		if (split_text_mapping && (mapping_size == PMD_SIZE) &&
-		    (addr <= __pa_symbol(__init_begin)) &&
-		    (addr + mapping_size) >= __pa_symbol(_stext))
+		if ((mapping_size == PMD_SIZE) &&
+			should_split_mapping_size(addr, mapping_size))
 			mapping_size = PAGE_SIZE;
 
 		if (mapping_size != previous_size) {