arm64 CONFIG_DEBUG_SET_MODULE_RONX support

It's useful to be able to change individual bits in ptes at times. Introduce functions for this and update existing pte_mk* functions to use these primatives.

Signed-off-by Laura Abbott <[email protected]>

---
 arch/arm64/include/asm/pgtable.h | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

arch/arm64/include/asm/pgtable.h +19 -14

index ffe1ba0..ca41449 100644
@@ -149,46 +149,51 @@ extern struct page *empty_zero_page;
 #define pte_valid_not_user(pte) \
 	((pte_val(pte) & (PTE_VALID | PTE_USER)) == PTE_VALID)
 
-static inline pte_t pte_wrprotect(pte_t pte)
+static pte_t clear_pte_bit(pte_t pte, pgprot_t prot)
 {
-	pte_val(pte) &= ~PTE_WRITE;
+	pte_val(pte) &= ~pgprot_val(prot);
 	return pte;
 }
 
-static inline pte_t pte_mkwrite(pte_t pte)
+static pte_t set_pte_bit(pte_t pte, pgprot_t prot)
 {
-	pte_val(pte) |= PTE_WRITE;
+	pte_val(pte) |= pgprot_val(prot);
 	return pte;
 }
 
+static inline pte_t pte_wrprotect(pte_t pte)
+{
+	return clear_pte_bit(pte, __pgprot(PTE_WRITE));
+}
+
+static inline pte_t pte_mkwrite(pte_t pte)
+{
+	return set_pte_bit(pte, __pgprot(PTE_WRITE));
+}
+
 static inline pte_t pte_mkclean(pte_t pte)
 {
-	pte_val(pte) &= ~PTE_DIRTY;
-	return pte;
+	return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
 }
 
 static inline pte_t pte_mkdirty(pte_t pte)
 {
-	pte_val(pte) |= PTE_DIRTY;
-	return pte;
+	return set_pte_bit(pte, __pgprot(PTE_DIRTY));
 }
 
 static inline pte_t pte_mkold(pte_t pte)
 {
-	pte_val(pte) &= ~PTE_AF;
-	return pte;
+	return clear_pte_bit(pte, __pgprot(PTE_AF));
 }
 
 static inline pte_t pte_mkyoung(pte_t pte)
 {
-	pte_val(pte) |= PTE_AF;
-	return pte;
+	return set_pte_bit(pte, __pgprot(PTE_AF));
 }
 
 static inline pte_t pte_mkspecial(pte_t pte)
 {
-	pte_val(pte) |= PTE_SPECIAL;
-	return pte;
+	return set_pte_bit(pte, __pgprot(PTE_SPECIAL));
 }
 
 static inline void set_pte(pte_t *ptep, pte_t pte)

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

In a similar fashion to other architecture, add the infrastructure and Kconfig to enable DEBUG_SET_MODULE_RONX support. When enabled, module ranges will be marked read-only/no-execute as appropriate.

arch/arm64/mm/pageattr.c new file +22 -0

new file mode 100644
index 0000000..13573da
@@ -0,0 +1,118 @@
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+
+#include <asm/pgtable.h>
+#include <asm/tlbflush.h>
+
+static int __change_memory(pte_t *ptep, pgtable_t token, unsigned long addr,
+			pgprot_t prot, bool set)

arch/arm64/mm/pageattr.c new file +23 -0

+static int set_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
+			void *data)
+{
+	pgprot_t prot = (pgprot_t)data;
+
+	return __change_memory(ptep, token, addr, prot, true);
+}
+
+static int clear_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
+			void *data)
+{
+	pgprot_t prot = (pgprot_t)data;
+
+	return __change_memory(ptep, token, addr, prot, false);
+}
+
+static int change_memory_common(unsigned long addr, int numpages,
+				pgprot_t prot, bool set)
+{
+	unsigned long start = addr;
+	unsigned long size = PAGE_SIZE*numpages;
+	unsigned long end = start + size;
+	int ret;

arch/arm64/mm/pageattr.c new file +13 -0

+	flush_tlb_kernel_range(start, end);
+	return ret;
+}
+
+static int change_memory_set_bit(unsigned long addr, int numpages,
+					pgprot_t prot)
+{
+	return change_memory_common(addr, numpages, prot, true);
+}
+
+static int change_memory_clear_bit(unsigned long addr, int numpages,
+					pgprot_t prot)
+{

arch/arm64/mm/pageattr.c new file +16 -0

+	return change_memory_set_bit(addr, numpages, __pgprot(PTE_RDONLY));
+}
+EXPORT_SYMBOL_GPL(set_memory_ro);
+
+int set_memory_rw(unsigned long addr, int numpages)
+{
+	int ret;
+
+	ret = change_memory_set_bit(addr, numpages, __pgprot(PTE_WRITE));
+	if (ret)
+		return ret;
+
+	return change_memory_clear_bit(addr, numpages, __pgprot(PTE_RDONLY));
+}
+EXPORT_SYMBOL_GPL(set_memory_rw);
+

Hi Laura,

Quoted 4 lines · Laura Abbott

In a similar fashion to other architecture, add the infrastructure and Kconfig to enable DEBUG_SET_MODULE_RONX support. When enabled, module ranges will be marked read-only/no-execute as appropriate.

[...]

Quoted 32 lines

arch/arm64/mm/pageattr.c new file +26 -0

new file mode 100644
index 0000000..13573da
@@ -0,0 +1,118 @@
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+
+#include <asm/pgtable.h>
+#include <asm/tlbflush.h>
+
+static int __change_memory(pte_t *ptep, pgtable_t token, unsigned long addr,
+			pgprot_t prot, bool set)
+{
+	pte_t pte;
+
+	if (set)

arch/arm64/mm/pageattr.c line 32

Quoted 6 lines

arch/arm64/mm/pageattr.c new file +6 -0

+		pte = set_pte_bit(*ptep, prot);
+	else
+		pte = clear_pte_bit(*ptep, prot);
+	set_pte(ptep, pte);
+	return 0;
+}

Why do we need to propagate the token this far?

Quoted 27 lines

arch/arm64/mm/pageattr.c new file +27 -0

+static int set_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
+			void *data)
+{
+	pgprot_t prot = (pgprot_t)data;
+
+	return __change_memory(ptep, token, addr, prot, true);
+}
+
+static int clear_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
+			void *data)
+{
+	pgprot_t prot = (pgprot_t)data;
+
+	return __change_memory(ptep, token, addr, prot, false);
+}
+
+static int change_memory_common(unsigned long addr, int numpages,
+				pgprot_t prot, bool set)
+{
+	unsigned long start = addr;
+	unsigned long size = PAGE_SIZE*numpages;
+	unsigned long end = start + size;
+	int ret;
+
+	if (!is_module_address(start) || !is_module_address(end))
+		return -EINVAL;
+

arch/arm64/mm/pageattr.c line 66

Quoted 6 lines

arch/arm64/mm/pageattr.c new file +6 -0

+	if (set)
+		ret = apply_to_page_range(&init_mm, start, size,
+					set_page_range, (void *)prot);
+	else
+		ret = apply_to_page_range(&init_mm, start, size,
+					clear_page_range, (void *)prot);

What if addr isn't page-aligned?

Quoted 21 lines

arch/arm64/mm/pageattr.c new file +21 -0

+	flush_tlb_kernel_range(start, end);
+	return ret;
+}
+
+static int change_memory_set_bit(unsigned long addr, int numpages,
+					pgprot_t prot)
+{
+	return change_memory_common(addr, numpages, prot, true);
+}
+
+static int change_memory_clear_bit(unsigned long addr, int numpages,
+					pgprot_t prot)
+{
+	return change_memory_common(addr, numpages, prot, false);
+}
+
+int set_memory_ro(unsigned long addr, int numpages)
+{
+	int ret;
+
+	ret = change_memory_clear_bit(addr, numpages, __pgprot(PTE_WRITE));

arch/arm64/mm/pageattr.c line 88

Quoted 2 lines

arch/arm64/mm/pageattr.c new file +2 -0

+	if (ret)
+		return ret;

I'm slightly uncomfortable with this. Can other cores make the pages writable again here before we've set R/O? I think this would be neater if, instead of explicit set/clear operations, we had a single function that takes a set mask and a clear mask and writes the page table once (we also avoid the extra TLBI that way)

Quoted 21 lines

arch/arm64/mm/pageattr.c new file +21 -0

+	return change_memory_set_bit(addr, numpages, __pgprot(PTE_RDONLY));
+}
+EXPORT_SYMBOL_GPL(set_memory_ro);
+
+int set_memory_rw(unsigned long addr, int numpages)
+{
+	int ret;
+
+	ret = change_memory_set_bit(addr, numpages, __pgprot(PTE_WRITE));
+	if (ret)
+		return ret;
+
+	return change_memory_clear_bit(addr, numpages, __pgprot(PTE_RDONLY));
+}
+EXPORT_SYMBOL_GPL(set_memory_rw);
+
+int set_memory_nx(unsigned long addr, int numpages)
+{
+	return change_memory_set_bit(addr, numpages, __pgprot(PTE_PXN));
+}
+EXPORT_SYMBOL_GPL(set_memory_nx);

arch/arm64/mm/pageattr.c line 117

Quoted 5 lines

arch/arm64/mm/pageattr.c new file +5 -0

+
+int set_memory_x(unsigned long addr, int numpages)
+{
+	return change_memory_clear_bit(addr, numpages, __pgprot(PTE_PXN));
+}

What about UXN? (I don't have the ARM ARM to hand, but we may want to set both)

Will

Quoted 16 lines · Will Deacon

arch/arm64/mm/pageattr.c line 32

Quoted 12 lines · Laura Abbott

arch/arm64/mm/pageattr.c new file +12 -0

+static int __change_memory(pte_t *ptep, pgtable_t token, unsigned long addr,
+			pgprot_t prot, bool set)
+{
+	pte_t pte;
+
+	if (set)
+		pte = set_pte_bit(*ptep, prot);
+	else
+		pte = clear_pte_bit(*ptep, prot);
+	set_pte(ptep, pte);
+	return 0;
+}

Why do we need to propagate the token this far?

Not quite sure I understand the comment here.

Quoted 3 lines

What about UXN? (I don't have the ARM ARM to hand, but we may want to set both)

Both PAGE_KERNEL and PAGE_KERNEL_EXEC already set PTE_UXN. It seems redundant to add it again given this is currently restricted to module addresses. It's cheap to add it though for paranoia's sake.