[PATCHv6 0/8] Better page protections for arm64

[PATCHv6 0/8] Better page protections for arm64

[Laura Abbott]

Hi,

This is v6 of the series to add stricter page protections for arm64.
The goal is to have text be RO/NX and everything else be RW/NX.
The biggest change here is the addition of ioremap_exec for EFI
services. The series did its job and caught EFI services trying to
use regular RAM for code purposes.

Laura Abbott (8):
  arm64: Treat handle_arch_irq as a function pointer
  arm64: Switch to adrp for loading the stub vectors
  arm64: Move cpu_resume into the text section
  arm64: Move some head.text functions to executable section
  arm64: Factor out fixmap initialiation from ioremap
  arm64: use fixmap for text patching when text is RO
  arm64: efi: Use ioremap_exec for code sections
  arm64: add better page protections to arm64

 arch/arm64/Kconfig.debug            |  23 ++
 arch/arm64/include/asm/cacheflush.h |   7 +
 arch/arm64/include/asm/fixmap.h     |   8 +-
 arch/arm64/include/asm/insn.h       |   2 +
 arch/arm64/include/asm/io.h         |   1 +
 arch/arm64/include/asm/irq.h        |   1 -
 arch/arm64/include/asm/pgtable.h    |   1 +
 arch/arm64/kernel/efi.c             |  12 +-
 arch/arm64/kernel/entry.S           |   6 +-
 arch/arm64/kernel/head.S            | 410 +++++++++++++++++++-----------------
 arch/arm64/kernel/insn.c            |  72 ++++++-
 arch/arm64/kernel/irq.c             |   2 +
 arch/arm64/kernel/jump_label.c      |   2 +-
 arch/arm64/kernel/setup.c           |   1 +
 arch/arm64/kernel/sleep.S           |  36 +---
 arch/arm64/kernel/suspend.c         |   4 +-
 arch/arm64/kernel/vmlinux.lds.S     |  18 +-
 arch/arm64/mm/init.c                |   1 +
 arch/arm64/mm/ioremap.c             |  99 ++-------
 arch/arm64/mm/mm.h                  |   2 +
 arch/arm64/mm/mmu.c                 | 405 +++++++++++++++++++++++++++++++----
 21 files changed, 740 insertions(+), 373 deletions(-)

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6 1/8] arm64: Treat handle_arch_irq as a function pointer

[Laura Abbott]

handle_arch_irq isn't actually text, it's just a function pointer.
It doesn't need to be stored in the text section and doing so
causes problesm if we ever want to make the kernel text read only.
Declare handle_arch_irq as a proper function pointer stored in
the data section.

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm64/include/asm/irq.h | 1 -
 arch/arm64/kernel/entry.S    | 6 ++----
 arch/arm64/kernel/irq.c      | 2 ++
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index e1f7ecd..1eebf5b 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -3,7 +3,6 @@
 
 #include <asm-generic/irq.h>
 
-extern void (*handle_arch_irq)(struct pt_regs *);
 extern void migrate_irqs(void);
 extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
 
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 726b910..ee27f39 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -168,7 +168,8 @@ tsk	.req	x28		// current thread_info
  * Interrupt handling.
  */
 	.macro	irq_handler
-	ldr	x1, handle_arch_irq
+	adrp	x1, handle_arch_irq
+	ldr	x1, [x1, #:lo12:handle_arch_irq]
 	mov	x0, sp
 	blr	x1
 	.endm
@@ -695,6 +696,3 @@ ENTRY(sys_rt_sigreturn_wrapper)
 	mov	x0, sp
 	b	sys_rt_sigreturn
 ENDPROC(sys_rt_sigreturn_wrapper)
-
-ENTRY(handle_arch_irq)
-	.quad	0
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 071a6ec..240b75c 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -40,6 +40,8 @@ int arch_show_interrupts(struct seq_file *p, int prec)
 	return 0;
 }
 
+void (*handle_arch_irq)(struct pt_regs *) = NULL;
+
 void __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
 {
 	if (handle_arch_irq)
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6 2/8] arm64: Switch to adrp for loading the stub vectors

[Laura Abbott]

The hyp stub vectors are currently loaded using adr. This
instruction has a +/- 1MB range for the loading address. If
the alignment for sections is changed the address may be more
than 1MB away, resulting in reclocation errors. Switch to using
adrp for getting the address to ensure we aren't affected by the
location of the __hyp_stub_vectors.

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm64/kernel/head.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 0a6e4f9..10f5cc0 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -331,7 +331,8 @@ CPU_LE(	movk	x0, #0x30d0, lsl #16	)	// Clear EE and E0E on LE systems
 	msr	vttbr_el2, xzr
 
 	/* Hypervisor stub */
-	adr	x0, __hyp_stub_vectors
+	adrp	x0, __hyp_stub_vectors
+	add	x0, x0, #:lo12:__hyp_stub_vectors
 	msr	vbar_el2, x0
 
 	/* spsr */
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6 3/8] arm64: Move cpu_resume into the text section

[Laura Abbott]

The function cpu_resume currently lives in the .data section.
There's no reason for it to be there since we can use relative
instructions without a problem. Move a few cpu_resume data
structures out of the assembly file so the .data annotation
can be dropped completely and cpu_resume ends up in the read
only text section.

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Tested-by: Kees Cook <keescook@chromium.org>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
v6: Dropped the last bit of cruft after cpu_resume
---
 arch/arm64/kernel/sleep.S   | 36 ++++++------------------------------
 arch/arm64/kernel/suspend.c |  4 ++--
 2 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
index a564b44..ede186c 100644
--- a/arch/arm64/kernel/sleep.S
+++ b/arch/arm64/kernel/sleep.S
@@ -147,14 +147,12 @@ cpu_resume_after_mmu:
 	ret
 ENDPROC(cpu_resume_after_mmu)
 
-	.data
 ENTRY(cpu_resume)
 	bl	el2_setup		// if in EL2 drop to EL1 cleanly
 #ifdef CONFIG_SMP
 	mrs	x1, mpidr_el1
-	adr	x4, mpidr_hash_ptr
-	ldr	x5, [x4]
-	add	x8, x4, x5		// x8 = struct mpidr_hash phys address
+	adrp	x8, mpidr_hash
+	add x8, x8, #:lo12:mpidr_hash // x8 = struct mpidr_hash phys address
         /* retrieve mpidr_hash members to compute the hash */
 	ldr	x2, [x8, #MPIDR_HASH_MASK]
 	ldp	w3, w4, [x8, #MPIDR_HASH_SHIFTS]
@@ -164,14 +162,15 @@ ENTRY(cpu_resume)
 #else
 	mov	x7, xzr
 #endif
-	adr	x0, sleep_save_sp
+	adrp	x0, sleep_save_sp
+	add	x0, x0, #:lo12:sleep_save_sp
 	ldr	x0, [x0, #SLEEP_SAVE_SP_PHYS]
 	ldr	x0, [x0, x7, lsl #3]
 	/* load sp from context */
 	ldr	x2, [x0, #CPU_CTX_SP]
-	adr	x1, sleep_idmap_phys
+	adrp	x1, sleep_idmap_phys
 	/* load physical address of identity map page table in x1 */
-	ldr	x1, [x1]
+	ldr	x1, [x1, #:lo12:sleep_idmap_phys]
 	mov	sp, x2
 	/*
 	 * cpu_do_resume expects x0 to contain context physical address
@@ -180,26 +179,3 @@ ENTRY(cpu_resume)
 	bl	cpu_do_resume		// PC relative jump, MMU off
 	b	cpu_resume_mmu		// Resume MMU, never returns
 ENDPROC(cpu_resume)
-
-	.align 3
-mpidr_hash_ptr:
-	/*
-	 * offset of mpidr_hash symbol from current location
-	 * used to obtain run-time mpidr_hash address with MMU off
-         */
-	.quad	mpidr_hash - .
-/*
- * physical address of identity mapped page tables
- */
-	.type	sleep_idmap_phys, #object
-ENTRY(sleep_idmap_phys)
-	.quad	0
-/*
- * struct sleep_save_sp {
- *	phys_addr_t *save_ptr_stash;
- *	phys_addr_t save_ptr_stash_phys;
- * };
- */
-	.type	sleep_save_sp, #object
-ENTRY(sleep_save_sp)
-	.space	SLEEP_SAVE_SP_SZ	// struct sleep_save_sp
diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
index 13ad4db..3771b72 100644
--- a/arch/arm64/kernel/suspend.c
+++ b/arch/arm64/kernel/suspend.c
@@ -126,8 +126,8 @@ int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 	return ret;
 }
 
-extern struct sleep_save_sp sleep_save_sp;
-extern phys_addr_t sleep_idmap_phys;
+struct sleep_save_sp sleep_save_sp;
+phys_addr_t sleep_idmap_phys;
 
 static int __init cpu_suspend_init(void)
 {
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6 4/8] arm64: Move some head.text functions to executable section

[Laura Abbott]

The head.text section is intended to be run at early bootup
before any of the regular kernel mappings have been setup.
Parts of head.text may be freed back into the buddy allocator
due to TEXT_OFFSET so for security requirements this memory
must not be executable. The suspend/resume/hotplug code path
requires some of these head.S functions to run however which
means they need to be executable. Support these conflicting
requirements by moving the few head.text functions that need
to be executable to the text section which has the appropriate
page table permissions.

Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
v6: Dropped the latehead.text bit and just moved everything to the regular
text section
---
 arch/arm64/kernel/head.S | 407 ++++++++++++++++++++++++-----------------------
 1 file changed, 210 insertions(+), 197 deletions(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 10f5cc0..4b63d7a 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -238,7 +238,13 @@ ENTRY(stext)
 	mov	x0, x22
 	bl	lookup_processor_type
 	mov	x23, x0				// x23=current cpu_table
-	cbz	x23, __error_p			// invalid processor (x23=0)?
+	/*
+	 *	__error_p may end up out of range for cbz if text areas
+	 *	are aligned up to section sizes
+	 */
+	cbnz	x23, 1f				// invalid processor (x23=0)?
+	b	__error_p
+1:
 	bl	__vet_fdt
 	bl	__create_page_tables		// x25=TTBR0, x26=TTBR1
 	/*
@@ -250,13 +256,214 @@ ENTRY(stext)
 	 */
 	ldr	x27, __switch_data		// address to jump to after
 						// MMU has been enabled
-	adr	lr, __enable_mmu		// return (PIC) address
+	adrp	lr, __enable_mmu		// return (PIC) address
+	add	lr, lr, #:lo12:__enable_mmu
 	ldr	x12, [x23, #CPU_INFO_SETUP]
 	add	x12, x12, x28			// __virt_to_phys
 	br	x12				// initialise processor
 ENDPROC(stext)
 
 /*
+ * Determine validity of the x21 FDT pointer.
+ * The dtb must be 8-byte aligned and live in the first 512M of memory.
+ */
+__vet_fdt:
+	tst	x21, #0x7
+	b.ne	1f
+	cmp	x21, x24
+	b.lt	1f
+	mov	x0, #(1 << 29)
+	add	x0, x0, x24
+	cmp	x21, x0
+	b.ge	1f
+	ret
+1:
+	mov	x21, #0
+	ret
+ENDPROC(__vet_fdt)
+/*
+ * Macro to create a table entry to the next page.
+ *
+ *	tbl:	page table address
+ *	virt:	virtual address
+ *	shift:	#imm page table shift
+ *	ptrs:	#imm pointers per table page
+ *
+ * Preserves:	virt
+ * Corrupts:	tmp1, tmp2
+ * Returns:	tbl -> next level table page address
+ */
+	.macro	create_table_entry, tbl, virt, shift, ptrs, tmp1, tmp2
+	lsr	\tmp1, \virt, #\shift
+	and	\tmp1, \tmp1, #\ptrs - 1	// table index
+	add	\tmp2, \tbl, #PAGE_SIZE
+	orr	\tmp2, \tmp2, #PMD_TYPE_TABLE	// address of next table and entry type
+	str	\tmp2, [\tbl, \tmp1, lsl #3]
+	add	\tbl, \tbl, #PAGE_SIZE		// next level table page
+	.endm
+
+/*
+ * Macro to populate the PGD (and possibily PUD) for the corresponding
+ * block entry in the next level (tbl) for the given virtual address.
+ *
+ * Preserves:	tbl, next, virt
+ * Corrupts:	tmp1, tmp2
+ */
+	.macro	create_pgd_entry, tbl, virt, tmp1, tmp2
+	create_table_entry \tbl, \virt, PGDIR_SHIFT, PTRS_PER_PGD, \tmp1, \tmp2
+#if SWAPPER_PGTABLE_LEVELS == 3
+	create_table_entry \tbl, \virt, TABLE_SHIFT, PTRS_PER_PTE, \tmp1, \tmp2
+#endif
+	.endm
+
+/*
+ * Macro to populate block entries in the page table for the start..end
+ * virtual range (inclusive).
+ *
+ * Preserves:	tbl, flags
+ * Corrupts:	phys, start, end, pstate
+ */
+	.macro	create_block_map, tbl, flags, phys, start, end
+	lsr	\phys, \phys, #BLOCK_SHIFT
+	lsr	\start, \start, #BLOCK_SHIFT
+	and	\start, \start, #PTRS_PER_PTE - 1	// table index
+	orr	\phys, \flags, \phys, lsl #BLOCK_SHIFT	// table entry
+	lsr	\end, \end, #BLOCK_SHIFT
+	and	\end, \end, #PTRS_PER_PTE - 1		// table end index
+9999:	str	\phys, [\tbl, \start, lsl #3]		// store the entry
+	add	\start, \start, #1			// next entry
+	add	\phys, \phys, #BLOCK_SIZE		// next block
+	cmp	\start, \end
+	b.ls	9999b
+	.endm
+
+/*
+ * Setup the initial page tables. We only setup the barest amount which is
+ * required to get the kernel running. The following sections are required:
+ *   - identity mapping to enable the MMU (low address, TTBR0)
+ *   - first few MB of the kernel linear mapping to jump to once the MMU has
+ *     been enabled, including the FDT blob (TTBR1)
+ *   - pgd entry for fixed mappings (TTBR1)
+ */
+__create_page_tables:
+	pgtbl	x25, x26, x28			// idmap_pg_dir and swapper_pg_dir addresses
+	mov	x27, lr
+
+	/*
+	 * Invalidate the idmap and swapper page tables to avoid potential
+	 * dirty cache lines being evicted.
+	 */
+	mov	x0, x25
+	add	x1, x26, #SWAPPER_DIR_SIZE
+	bl	__inval_cache_range
+
+	/*
+	 * Clear the idmap and swapper page tables.
+	 */
+	mov	x0, x25
+	add	x6, x26, #SWAPPER_DIR_SIZE
+1:	stp	xzr, xzr, [x0], #16
+	stp	xzr, xzr, [x0], #16
+	stp	xzr, xzr, [x0], #16
+	stp	xzr, xzr, [x0], #16
+	cmp	x0, x6
+	b.lo	1b
+
+	ldr	x7, =MM_MMUFLAGS
+
+	/*
+	 * Create the identity mapping.
+	 */
+	mov	x0, x25				// idmap_pg_dir
+	ldr	x3, =KERNEL_START
+	add	x3, x3, x28			// __pa(KERNEL_START)
+	create_pgd_entry x0, x3, x5, x6
+	ldr	x6, =KERNEL_END
+	mov	x5, x3				// __pa(KERNEL_START)
+	add	x6, x6, x28			// __pa(KERNEL_END)
+	create_block_map x0, x7, x3, x5, x6
+
+	/*
+	 * Map the kernel image (starting with PHYS_OFFSET).
+	 */
+	mov	x0, x26				// swapper_pg_dir
+	mov	x5, #PAGE_OFFSET
+	create_pgd_entry x0, x5, x3, x6
+	ldr	x6, =KERNEL_END
+	mov	x3, x24				// phys offset
+	create_block_map x0, x7, x3, x5, x6
+
+	/*
+	 * Map the FDT blob (maximum 2MB; must be within 512MB of
+	 * PHYS_OFFSET).
+	 */
+	mov	x3, x21				// FDT phys address
+	and	x3, x3, #~((1 << 21) - 1)	// 2MB aligned
+	mov	x6, #PAGE_OFFSET
+	sub	x5, x3, x24			// subtract PHYS_OFFSET
+	tst	x5, #~((1 << 29) - 1)		// within 512MB?
+	csel	x21, xzr, x21, ne		// zero the FDT pointer
+	b.ne	1f
+	add	x5, x5, x6			// __va(FDT blob)
+	add	x6, x5, #1 << 21		// 2MB for the FDT blob
+	sub	x6, x6, #1			// inclusive range
+	create_block_map x0, x7, x3, x5, x6
+1:
+	/*
+	 * Since the page tables have been populated with non-cacheable
+	 * accesses (MMU disabled), invalidate the idmap and swapper page
+	 * tables again to remove any speculatively loaded cache lines.
+	 */
+	mov	x0, x25
+	add	x1, x26, #SWAPPER_DIR_SIZE
+	bl	__inval_cache_range
+
+	mov	lr, x27
+	ret
+ENDPROC(__create_page_tables)
+	.ltorg
+
+	.align	3
+	.type	__switch_data, %object
+__switch_data:
+	.quad	__mmap_switched
+	.quad	__bss_start			// x6
+	.quad	__bss_stop			// x7
+	.quad	processor_id			// x4
+	.quad	__fdt_pointer			// x5
+	.quad	memstart_addr			// x6
+	.quad	init_thread_union + THREAD_START_SP // sp
+
+/*
+ * The following fragment of code is executed with the MMU on in MMU mode, and
+ * uses absolute addresses; this is not position independent.
+ */
+__mmap_switched:
+	adr	x3, __switch_data + 8
+
+	ldp	x6, x7, [x3], #16
+1:	cmp	x6, x7
+	b.hs	2f
+	str	xzr, [x6], #8			// Clear BSS
+	b	1b
+2:
+	ldp	x4, x5, [x3], #16
+	ldr	x6, [x3], #8
+	ldr	x16, [x3]
+	mov	sp, x16
+	str	x22, [x4]			// Save processor ID
+	str	x21, [x5]			// Save FDT pointer
+	str	x24, [x6]			// Save PHYS_OFFSET
+	mov	x29, #0
+	b	start_kernel
+ENDPROC(__mmap_switched)
+
+/*
+ * end early head section, begin head code that is also used for
+ * hotplug and needs to have the same protections as the text region
+ */
+	.section ".text","ax"
+/*
  * If we're fortunate enough to boot@EL2, ensure that the world is
  * sane before dropping to EL1.
  *
@@ -493,183 +700,6 @@ ENDPROC(__calc_phys_offset)
 	.quad	PAGE_OFFSET
 
 /*
- * Macro to create a table entry to the next page.
- *
- *	tbl:	page table address
- *	virt:	virtual address
- *	shift:	#imm page table shift
- *	ptrs:	#imm pointers per table page
- *
- * Preserves:	virt
- * Corrupts:	tmp1, tmp2
- * Returns:	tbl -> next level table page address
- */
-	.macro	create_table_entry, tbl, virt, shift, ptrs, tmp1, tmp2
-	lsr	\tmp1, \virt, #\shift
-	and	\tmp1, \tmp1, #\ptrs - 1	// table index
-	add	\tmp2, \tbl, #PAGE_SIZE
-	orr	\tmp2, \tmp2, #PMD_TYPE_TABLE	// address of next table and entry type
-	str	\tmp2, [\tbl, \tmp1, lsl #3]
-	add	\tbl, \tbl, #PAGE_SIZE		// next level table page
-	.endm
-
-/*
- * Macro to populate the PGD (and possibily PUD) for the corresponding
- * block entry in the next level (tbl) for the given virtual address.
- *
- * Preserves:	tbl, next, virt
- * Corrupts:	tmp1, tmp2
- */
-	.macro	create_pgd_entry, tbl, virt, tmp1, tmp2
-	create_table_entry \tbl, \virt, PGDIR_SHIFT, PTRS_PER_PGD, \tmp1, \tmp2
-#if SWAPPER_PGTABLE_LEVELS == 3
-	create_table_entry \tbl, \virt, TABLE_SHIFT, PTRS_PER_PTE, \tmp1, \tmp2
-#endif
-	.endm
-
-/*
- * Macro to populate block entries in the page table for the start..end
- * virtual range (inclusive).
- *
- * Preserves:	tbl, flags
- * Corrupts:	phys, start, end, pstate
- */
-	.macro	create_block_map, tbl, flags, phys, start, end
-	lsr	\phys, \phys, #BLOCK_SHIFT
-	lsr	\start, \start, #BLOCK_SHIFT
-	and	\start, \start, #PTRS_PER_PTE - 1	// table index
-	orr	\phys, \flags, \phys, lsl #BLOCK_SHIFT	// table entry
-	lsr	\end, \end, #BLOCK_SHIFT
-	and	\end, \end, #PTRS_PER_PTE - 1		// table end index
-9999:	str	\phys, [\tbl, \start, lsl #3]		// store the entry
-	add	\start, \start, #1			// next entry
-	add	\phys, \phys, #BLOCK_SIZE		// next block
-	cmp	\start, \end
-	b.ls	9999b
-	.endm
-
-/*
- * Setup the initial page tables. We only setup the barest amount which is
- * required to get the kernel running. The following sections are required:
- *   - identity mapping to enable the MMU (low address, TTBR0)
- *   - first few MB of the kernel linear mapping to jump to once the MMU has
- *     been enabled, including the FDT blob (TTBR1)
- *   - pgd entry for fixed mappings (TTBR1)
- */
-__create_page_tables:
-	pgtbl	x25, x26, x28			// idmap_pg_dir and swapper_pg_dir addresses
-	mov	x27, lr
-
-	/*
-	 * Invalidate the idmap and swapper page tables to avoid potential
-	 * dirty cache lines being evicted.
-	 */
-	mov	x0, x25
-	add	x1, x26, #SWAPPER_DIR_SIZE
-	bl	__inval_cache_range
-
-	/*
-	 * Clear the idmap and swapper page tables.
-	 */
-	mov	x0, x25
-	add	x6, x26, #SWAPPER_DIR_SIZE
-1:	stp	xzr, xzr, [x0], #16
-	stp	xzr, xzr, [x0], #16
-	stp	xzr, xzr, [x0], #16
-	stp	xzr, xzr, [x0], #16
-	cmp	x0, x6
-	b.lo	1b
-
-	ldr	x7, =MM_MMUFLAGS
-
-	/*
-	 * Create the identity mapping.
-	 */
-	mov	x0, x25				// idmap_pg_dir
-	ldr	x3, =KERNEL_START
-	add	x3, x3, x28			// __pa(KERNEL_START)
-	create_pgd_entry x0, x3, x5, x6
-	ldr	x6, =KERNEL_END
-	mov	x5, x3				// __pa(KERNEL_START)
-	add	x6, x6, x28			// __pa(KERNEL_END)
-	create_block_map x0, x7, x3, x5, x6
-
-	/*
-	 * Map the kernel image (starting with PHYS_OFFSET).
-	 */
-	mov	x0, x26				// swapper_pg_dir
-	mov	x5, #PAGE_OFFSET
-	create_pgd_entry x0, x5, x3, x6
-	ldr	x6, =KERNEL_END
-	mov	x3, x24				// phys offset
-	create_block_map x0, x7, x3, x5, x6
-
-	/*
-	 * Map the FDT blob (maximum 2MB; must be within 512MB of
-	 * PHYS_OFFSET).
-	 */
-	mov	x3, x21				// FDT phys address
-	and	x3, x3, #~((1 << 21) - 1)	// 2MB aligned
-	mov	x6, #PAGE_OFFSET
-	sub	x5, x3, x24			// subtract PHYS_OFFSET
-	tst	x5, #~((1 << 29) - 1)		// within 512MB?
-	csel	x21, xzr, x21, ne		// zero the FDT pointer
-	b.ne	1f
-	add	x5, x5, x6			// __va(FDT blob)
-	add	x6, x5, #1 << 21		// 2MB for the FDT blob
-	sub	x6, x6, #1			// inclusive range
-	create_block_map x0, x7, x3, x5, x6
-1:
-	/*
-	 * Since the page tables have been populated with non-cacheable
-	 * accesses (MMU disabled), invalidate the idmap and swapper page
-	 * tables again to remove any speculatively loaded cache lines.
-	 */
-	mov	x0, x25
-	add	x1, x26, #SWAPPER_DIR_SIZE
-	bl	__inval_cache_range
-
-	mov	lr, x27
-	ret
-ENDPROC(__create_page_tables)
-	.ltorg
-
-	.align	3
-	.type	__switch_data, %object
-__switch_data:
-	.quad	__mmap_switched
-	.quad	__bss_start			// x6
-	.quad	__bss_stop			// x7
-	.quad	processor_id			// x4
-	.quad	__fdt_pointer			// x5
-	.quad	memstart_addr			// x6
-	.quad	init_thread_union + THREAD_START_SP // sp
-
-/*
- * The following fragment of code is executed with the MMU on in MMU mode, and
- * uses absolute addresses; this is not position independent.
- */
-__mmap_switched:
-	adr	x3, __switch_data + 8
-
-	ldp	x6, x7, [x3], #16
-1:	cmp	x6, x7
-	b.hs	2f
-	str	xzr, [x6], #8			// Clear BSS
-	b	1b
-2:
-	ldp	x4, x5, [x3], #16
-	ldr	x6, [x3], #8
-	ldr	x16, [x3]
-	mov	sp, x16
-	str	x22, [x4]			// Save processor ID
-	str	x21, [x5]			// Save FDT pointer
-	str	x24, [x6]			// Save PHYS_OFFSET
-	mov	x29, #0
-	b	start_kernel
-ENDPROC(__mmap_switched)
-
-/*
  * Exception handling. Something went wrong and we can't proceed. We ought to
  * tell the user, but since we don't have any guarantee that we're even
  * running on the right architecture, we do virtually nothing.
@@ -717,21 +747,4 @@ __lookup_processor_type_data:
 	.quad	cpu_table
 	.size	__lookup_processor_type_data, . - __lookup_processor_type_data
 
-/*
- * Determine validity of the x21 FDT pointer.
- * The dtb must be 8-byte aligned and live in the first 512M of memory.
- */
-__vet_fdt:
-	tst	x21, #0x7
-	b.ne	1f
-	cmp	x21, x24
-	b.lt	1f
-	mov	x0, #(1 << 29)
-	add	x0, x0, x24
-	cmp	x21, x0
-	b.ge	1f
-	ret
-1:
-	mov	x21, #0
-	ret
-ENDPROC(__vet_fdt)
+
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6 4/8] arm64: Move some head.text functions to executable section

[Mark Rutland]

Hi Laura,

On Fri, Nov 21, 2014 at 09:50:41PM +0000, Laura Abbott wrote:
> The head.text section is intended to be run at early bootup
> before any of the regular kernel mappings have been setup.
> Parts of head.text may be freed back into the buddy allocator
> due to TEXT_OFFSET so for security requirements this memory
> must not be executable. The suspend/resume/hotplug code path
> requires some of these head.S functions to run however which
> means they need to be executable. Support these conflicting
> requirements by moving the few head.text functions that need
> to be executable to the text section which has the appropriate
> page table permissions.
> 
> Tested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>

Other than a minor nit below this looks good to me, and I'm not seeing
any issues with boot, hotplug, or idle, so:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>

Thanks for putting this together!

> ---
> v6: Dropped the latehead.text bit and just moved everything to the regular
> text section
> ---
>  arch/arm64/kernel/head.S | 407 ++++++++++++++++++++++++-----------------------
>  1 file changed, 210 insertions(+), 197 deletions(-)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 10f5cc0..4b63d7a 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -238,7 +238,13 @@ ENTRY(stext)
>         mov     x0, x22
>         bl      lookup_processor_type
>         mov     x23, x0                         // x23=current cpu_table
> -       cbz     x23, __error_p                  // invalid processor (x23=0)?
> +       /*
> +        *      __error_p may end up out of range for cbz if text areas
> +        *      are aligned up to section sizes
> +        */

Nit: We don't need to align the comment text (we don't for the comment a
few lines later, or any others in this file). It would be nice to keep
that consistent.

Thanks,
Mark.

[PATCHv6 4/8] arm64: Move some head.text functions to executable section

[Will Deacon]

On Wed, Nov 26, 2014 at 04:30:27PM +0000, Mark Rutland wrote:
> On Fri, Nov 21, 2014 at 09:50:41PM +0000, Laura Abbott wrote:
> > The head.text section is intended to be run at early bootup
> > before any of the regular kernel mappings have been setup.
> > Parts of head.text may be freed back into the buddy allocator
> > due to TEXT_OFFSET so for security requirements this memory
> > must not be executable. The suspend/resume/hotplug code path
> > requires some of these head.S functions to run however which
> > means they need to be executable. Support these conflicting
> > requirements by moving the few head.text functions that need
> > to be executable to the text section which has the appropriate
> > page table permissions.
> > 
> > Tested-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
> 
> Other than a minor nit below this looks good to me, and I'm not seeing
> any issues with boot, hotplug, or idle, so:
> 
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Tested-by: Mark Rutland <mark.rutland@arm.com>
> 
> Thanks for putting this together!
> 
> > ---
> > v6: Dropped the latehead.text bit and just moved everything to the regular
> > text section
> > ---
> >  arch/arm64/kernel/head.S | 407 ++++++++++++++++++++++++-----------------------
> >  1 file changed, 210 insertions(+), 197 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index 10f5cc0..4b63d7a 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -238,7 +238,13 @@ ENTRY(stext)
> >         mov     x0, x22
> >         bl      lookup_processor_type
> >         mov     x23, x0                         // x23=current cpu_table
> > -       cbz     x23, __error_p                  // invalid processor (x23=0)?
> > +       /*
> > +        *      __error_p may end up out of range for cbz if text areas
> > +        *      are aligned up to section sizes
> > +        */
> 
> Nit: We don't need to align the comment text (we don't for the comment a
> few lines later, or any others in this file). It would be nice to keep
> that consistent.

Applied with this fixup.

Will

[PATCHv6 5/8] arm64: Factor out fixmap initialiation from ioremap

[Laura Abbott]

The fixmap API was originally added for arm64 for
early_ioremap purposes. It can be used for other purposes too
so move the initialization from ioremap to somewhere more
generic. This makes it obvious where the fixmap is being set
up and allows for a cleaner implementation of __set_fixmap.

Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm64/include/asm/fixmap.h |  7 +--
 arch/arm64/kernel/setup.c       |  1 +
 arch/arm64/mm/ioremap.c         | 93 ++--------------------------------------
 arch/arm64/mm/mmu.c             | 94 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 93 deletions(-)

diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
index 5f7bfe6..db26a2f2 100644
--- a/arch/arm64/include/asm/fixmap.h
+++ b/arch/arm64/include/asm/fixmap.h
@@ -56,10 +56,11 @@ enum fixed_addresses {
 
 #define FIXMAP_PAGE_IO     __pgprot(PROT_DEVICE_nGnRE)
 
-extern void __early_set_fixmap(enum fixed_addresses idx,
-			       phys_addr_t phys, pgprot_t flags);
+void __init early_fixmap_init(void);
 
-#define __set_fixmap __early_set_fixmap
+#define __early_set_fixmap __set_fixmap
+
+extern void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
 
 #include <asm-generic/fixmap.h>
 
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 2437196..fb457cb 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -376,6 +376,7 @@ void __init setup_arch(char **cmdline_p)
 
 	*cmdline_p = boot_command_line;
 
+	early_fixmap_init();
 	early_ioremap_init();
 
 	parse_early_param();
diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
index 4a07630..cbb99c8 100644
--- a/arch/arm64/mm/ioremap.c
+++ b/arch/arm64/mm/ioremap.c
@@ -103,97 +103,10 @@ void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
 }
 EXPORT_SYMBOL(ioremap_cache);
 
-static pte_t bm_pte[PTRS_PER_PTE] __page_aligned_bss;
-#if CONFIG_ARM64_PGTABLE_LEVELS > 2
-static pmd_t bm_pmd[PTRS_PER_PMD] __page_aligned_bss;
-#endif
-#if CONFIG_ARM64_PGTABLE_LEVELS > 3
-static pud_t bm_pud[PTRS_PER_PUD] __page_aligned_bss;
-#endif
-
-static inline pud_t * __init early_ioremap_pud(unsigned long addr)
-{
-	pgd_t *pgd;
-
-	pgd = pgd_offset_k(addr);
-	BUG_ON(pgd_none(*pgd) || pgd_bad(*pgd));
-
-	return pud_offset(pgd, addr);
-}
-
-static inline pmd_t * __init early_ioremap_pmd(unsigned long addr)
-{
-	pud_t *pud = early_ioremap_pud(addr);
-
-	BUG_ON(pud_none(*pud) || pud_bad(*pud));
-
-	return pmd_offset(pud, addr);
-}
-
-static inline pte_t * __init early_ioremap_pte(unsigned long addr)
-{
-	pmd_t *pmd = early_ioremap_pmd(addr);
-
-	BUG_ON(pmd_none(*pmd) || pmd_bad(*pmd));
-
-	return pte_offset_kernel(pmd, addr);
-}
-
+/*
+ * Must be called after early_fixmap_init
+ */
 void __init early_ioremap_init(void)
 {
-	pgd_t *pgd;
-	pud_t *pud;
-	pmd_t *pmd;
-	unsigned long addr = fix_to_virt(FIX_BTMAP_BEGIN);
-
-	pgd = pgd_offset_k(addr);
-	pgd_populate(&init_mm, pgd, bm_pud);
-	pud = pud_offset(pgd, addr);
-	pud_populate(&init_mm, pud, bm_pmd);
-	pmd = pmd_offset(pud, addr);
-	pmd_populate_kernel(&init_mm, pmd, bm_pte);
-
-	/*
-	 * The boot-ioremap range spans multiple pmds, for which
-	 * we are not prepared:
-	 */
-	BUILD_BUG_ON((__fix_to_virt(FIX_BTMAP_BEGIN) >> PMD_SHIFT)
-		     != (__fix_to_virt(FIX_BTMAP_END) >> PMD_SHIFT));
-
-	if (pmd != early_ioremap_pmd(fix_to_virt(FIX_BTMAP_END))) {
-		WARN_ON(1);
-		pr_warn("pmd %p != %p\n",
-			pmd, early_ioremap_pmd(fix_to_virt(FIX_BTMAP_END)));
-		pr_warn("fix_to_virt(FIX_BTMAP_BEGIN): %08lx\n",
-			fix_to_virt(FIX_BTMAP_BEGIN));
-		pr_warn("fix_to_virt(FIX_BTMAP_END):   %08lx\n",
-			fix_to_virt(FIX_BTMAP_END));
-
-		pr_warn("FIX_BTMAP_END:       %d\n", FIX_BTMAP_END);
-		pr_warn("FIX_BTMAP_BEGIN:     %d\n",
-			FIX_BTMAP_BEGIN);
-	}
-
 	early_ioremap_setup();
 }
-
-void __init __early_set_fixmap(enum fixed_addresses idx,
-			       phys_addr_t phys, pgprot_t flags)
-{
-	unsigned long addr = __fix_to_virt(idx);
-	pte_t *pte;
-
-	if (idx >= __end_of_fixed_addresses) {
-		BUG();
-		return;
-	}
-
-	pte = early_ioremap_pte(addr);
-
-	if (pgprot_val(flags))
-		set_pte(pte, pfn_pte(phys >> PAGE_SHIFT, flags));
-	else {
-		pte_clear(&init_mm, addr, pte);
-		flush_tlb_kernel_range(addr, addr+PAGE_SIZE);
-	}
-}
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index f4f8b50..6032f3e 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -28,6 +28,7 @@
 #include <linux/io.h>
 
 #include <asm/cputype.h>
+#include <asm/fixmap.h>
 #include <asm/sections.h>
 #include <asm/setup.h>
 #include <asm/sizes.h>
@@ -463,3 +464,96 @@ void vmemmap_free(unsigned long start, unsigned long end)
 {
 }
 #endif	/* CONFIG_SPARSEMEM_VMEMMAP */
+
+static pte_t bm_pte[PTRS_PER_PTE] __page_aligned_bss;
+#if CONFIG_ARM64_PGTABLE_LEVELS > 2
+static pmd_t bm_pmd[PTRS_PER_PMD] __page_aligned_bss;
+#endif
+#if CONFIG_ARM64_PGTABLE_LEVELS > 3
+static pud_t bm_pud[PTRS_PER_PUD] __page_aligned_bss;
+#endif
+
+static inline pud_t * fixmap_pud(unsigned long addr)
+{
+	pgd_t *pgd = pgd_offset_k(addr);
+
+	BUG_ON(pgd_none(*pgd) || pgd_bad(*pgd));
+
+	return pud_offset(pgd, addr);
+}
+
+static inline pmd_t * fixmap_pmd(unsigned long addr)
+{
+	pud_t *pud = fixmap_pud(addr);
+
+	BUG_ON(pud_none(*pud) || pud_bad(*pud));
+
+	return pmd_offset(pud, addr);
+}
+
+static inline pte_t * fixmap_pte(unsigned long addr)
+{
+	pmd_t *pmd = fixmap_pmd(addr);
+
+	BUG_ON(pmd_none(*pmd) || pmd_bad(*pmd));
+
+	return pte_offset_kernel(pmd, addr);
+}
+
+void __init early_fixmap_init(void)
+{
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+	unsigned long addr = FIXADDR_START;
+
+	pgd = pgd_offset_k(addr);
+	pgd_populate(&init_mm, pgd, bm_pud);
+	pud = pud_offset(pgd, addr);
+	pud_populate(&init_mm, pud, bm_pmd);
+	pmd = pmd_offset(pud, addr);
+	pmd_populate_kernel(&init_mm, pmd, bm_pte);
+
+	/*
+	 * The boot-ioremap range spans multiple pmds, for which
+	 * we are not preparted:
+	 */
+	BUILD_BUG_ON((__fix_to_virt(FIX_BTMAP_BEGIN) >> PMD_SHIFT)
+		     != (__fix_to_virt(FIX_BTMAP_END) >> PMD_SHIFT));
+
+	if ((pmd != fixmap_pmd(fix_to_virt(FIX_BTMAP_BEGIN)))
+	     || pmd != fixmap_pmd(fix_to_virt(FIX_BTMAP_END))) {
+		WARN_ON(1);
+		pr_warn("pmd %p != %p, %p\n",
+			pmd, fixmap_pmd(fix_to_virt(FIX_BTMAP_BEGIN)),
+			fixmap_pmd(fix_to_virt(FIX_BTMAP_END)));
+		pr_warn("fix_to_virt(FIX_BTMAP_BEGIN): %08lx\n",
+			fix_to_virt(FIX_BTMAP_BEGIN));
+		pr_warn("fix_to_virt(FIX_BTMAP_END):   %08lx\n",
+			fix_to_virt(FIX_BTMAP_END));
+
+		pr_warn("FIX_BTMAP_END:       %d\n", FIX_BTMAP_END);
+		pr_warn("FIX_BTMAP_BEGIN:     %d\n", FIX_BTMAP_BEGIN);
+	}
+}
+
+void __set_fixmap(enum fixed_addresses idx,
+			       phys_addr_t phys, pgprot_t flags)
+{
+	unsigned long addr = __fix_to_virt(idx);
+	pte_t *pte;
+
+	if (idx >= __end_of_fixed_addresses) {
+		BUG();
+		return;
+	}
+
+	pte = fixmap_pte(addr);
+
+	if (pgprot_val(flags)) {
+		set_pte(pte, pfn_pte(phys >> PAGE_SHIFT, flags));
+	} else {
+		pte_clear(&init_mm, addr, pte);
+		flush_tlb_kernel_range(addr, addr+PAGE_SIZE);
+	}
+}
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6 6/8] arm64: use fixmap for text patching when text is RO

[Laura Abbott]

When kernel text is marked as read only, it cannot be modified directly.
Use a fixmap to modify the text instead in a similar manner to
x86 and arm.

Reviewed-by: Kees Cook <keescook@chromium.org>
Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm64/include/asm/fixmap.h |  1 +
 arch/arm64/include/asm/insn.h   |  2 ++
 arch/arm64/kernel/insn.c        | 72 +++++++++++++++++++++++++++++++++++++++--
 arch/arm64/kernel/jump_label.c  |  2 +-
 4 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
index db26a2f2..2cd4b0d 100644
--- a/arch/arm64/include/asm/fixmap.h
+++ b/arch/arm64/include/asm/fixmap.h
@@ -48,6 +48,7 @@ enum fixed_addresses {
 
 	FIX_BTMAP_END = __end_of_permanent_fixed_addresses,
 	FIX_BTMAP_BEGIN = FIX_BTMAP_END + TOTAL_FIX_BTMAPS - 1,
+	FIX_TEXT_POKE0,
 	__end_of_fixed_addresses
 };
 
diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 56a9e63..f66853b 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -282,6 +282,7 @@ bool aarch64_insn_is_nop(u32 insn);
 
 int aarch64_insn_read(void *addr, u32 *insnp);
 int aarch64_insn_write(void *addr, u32 insn);
+int aarch64_insn_write_early(void *addr, u32 insn);
 enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn);
 u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
 				  u32 insn, u64 imm);
@@ -352,6 +353,7 @@ u32 aarch64_insn_gen_logical_shifted_reg(enum aarch64_insn_register dst,
 bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn);
 
 int aarch64_insn_patch_text_nosync(void *addr, u32 insn);
+int __aarch64_insn_patch_text_nosync(void *addr, u32 insn, bool early);
 int aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt);
 int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt);
 #endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 8cd27fe..b2cad38 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -19,12 +19,15 @@
 #include <linux/bitops.h>
 #include <linux/compiler.h>
 #include <linux/kernel.h>
+#include <linux/mm.h>
 #include <linux/smp.h>
+#include <linux/spinlock.h>
 #include <linux/stop_machine.h>
 #include <linux/uaccess.h>
 
 #include <asm/cacheflush.h>
 #include <asm/debug-monitors.h>
+#include <asm/fixmap.h>
 #include <asm/insn.h>
 
 #define AARCH64_INSN_SF_BIT	BIT(31)
@@ -72,6 +75,36 @@ bool __kprobes aarch64_insn_is_nop(u32 insn)
 	}
 }
 
+static DEFINE_SPINLOCK(patch_lock);
+
+static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
+{
+	unsigned long uintaddr = (uintptr_t) addr;
+	bool module = !core_kernel_text(uintaddr);
+	struct page *page;
+
+	if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
+		page = vmalloc_to_page(addr);
+	else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA))
+		page = virt_to_page(addr);
+	else
+		return addr;
+
+	if (flags)
+		spin_lock_irqsave(&patch_lock, *flags);
+
+	set_fixmap(fixmap, page_to_phys(page));
+
+	return (void *) (__fix_to_virt(fixmap) + (uintaddr & ~PAGE_MASK));
+}
+
+static void __kprobes patch_unmap(int fixmap, unsigned long *flags)
+{
+	clear_fixmap(fixmap);
+
+	if (flags)
+		spin_unlock_irqrestore(&patch_lock, *flags);
+}
 /*
  * In ARMv8-A, A64 instructions have a fixed length of 32 bits and are always
  * little-endian.
@@ -88,10 +121,34 @@ int __kprobes aarch64_insn_read(void *addr, u32 *insnp)
 	return ret;
 }
 
+static int __kprobes __aarch64_insn_write(void *addr, u32 insn, bool patch)
+{
+	void *waddr = addr;
+	unsigned long flags;
+	int ret;
+
+	if (patch)
+		waddr = patch_map(addr, FIX_TEXT_POKE0, &flags);
+
+	ret = probe_kernel_write(waddr, &insn, AARCH64_INSN_SIZE);
+
+	if (waddr != addr)
+		patch_unmap(FIX_TEXT_POKE0, &flags);
+
+	return ret;
+}
+
 int __kprobes aarch64_insn_write(void *addr, u32 insn)
 {
 	insn = cpu_to_le32(insn);
-	return probe_kernel_write(addr, &insn, AARCH64_INSN_SIZE);
+	return __aarch64_insn_write(addr, insn, true);
+}
+
+int __kprobes aarch64_insn_write_early(void *addr, u32 insn)
+{
+	insn = cpu_to_le32(insn);
+	return __aarch64_insn_write(addr, insn, false);
+
 }
 
 static bool __kprobes __aarch64_insn_hotpatch_safe(u32 insn)
@@ -124,7 +181,7 @@ bool __kprobes aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn)
 	       __aarch64_insn_hotpatch_safe(new_insn);
 }
 
-int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn)
+int __kprobes __aarch64_insn_patch_text_nosync(void *addr, u32 insn, bool early)
 {
 	u32 *tp = addr;
 	int ret;
@@ -133,7 +190,11 @@ int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn)
 	if ((uintptr_t)tp & 0x3)
 		return -EINVAL;
 
-	ret = aarch64_insn_write(tp, insn);
+	if (early)
+		ret = aarch64_insn_write_early(tp, insn);
+	else
+		ret = aarch64_insn_write(tp, insn);
+
 	if (ret == 0)
 		flush_icache_range((uintptr_t)tp,
 				   (uintptr_t)tp + AARCH64_INSN_SIZE);
@@ -141,6 +202,11 @@ int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn)
 	return ret;
 }
 
+int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn)
+{
+	return __aarch64_insn_patch_text_nosync(addr, insn, false);
+}
+
 struct aarch64_insn_patch {
 	void		**text_addrs;
 	u32		*new_insns;
diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c
index 263a166..9ac30bb 100644
--- a/arch/arm64/kernel/jump_label.c
+++ b/arch/arm64/kernel/jump_label.c
@@ -38,7 +38,7 @@ static void __arch_jump_label_transform(struct jump_entry *entry,
 	}
 
 	if (is_static)
-		aarch64_insn_patch_text_nosync(addr, insn);
+		__aarch64_insn_patch_text_nosync(addr, insn, true);
 	else
 		aarch64_insn_patch_text(&addr, &insn, 1);
 }
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6 6/8] arm64: use fixmap for text patching when text is RO

[Mark Rutland]

Hi Laura,

It looks like "early" means before we've set up the strict page
permissions, though as far as I can tell the fixmap will already be
available before we do any patching. Could we not always use the fixmap
for patching? Then we only need the patch_{map,unmap} additions, and not
the changes to distinguish the early cases.

>From testing on Juno with defconfig, all of the early patches were
avoidable NOP -> NOP changes as part of static key initialisation, which
I think we can skip similarly to x86 (I'll send a patch shortly). All other
patches were not early and went via the fixmap.

Even with the avoidable NOP -> NOP patching I did not see a noticeable
boot time difference from forcing the use of the fixmap.

Thanks,
Mark.

On Fri, Nov 21, 2014 at 09:50:43PM +0000, Laura Abbott wrote:
> When kernel text is marked as read only, it cannot be modified directly.
> Use a fixmap to modify the text instead in a similar manner to
> x86 and arm.
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Tested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
> ---
>  arch/arm64/include/asm/fixmap.h |  1 +
>  arch/arm64/include/asm/insn.h   |  2 ++
>  arch/arm64/kernel/insn.c        | 72 +++++++++++++++++++++++++++++++++++++++--
>  arch/arm64/kernel/jump_label.c  |  2 +-
>  4 files changed, 73 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
> index db26a2f2..2cd4b0d 100644
> --- a/arch/arm64/include/asm/fixmap.h
> +++ b/arch/arm64/include/asm/fixmap.h
> @@ -48,6 +48,7 @@ enum fixed_addresses {
>  
>  	FIX_BTMAP_END = __end_of_permanent_fixed_addresses,
>  	FIX_BTMAP_BEGIN = FIX_BTMAP_END + TOTAL_FIX_BTMAPS - 1,
> +	FIX_TEXT_POKE0,
>  	__end_of_fixed_addresses
>  };
>  
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index 56a9e63..f66853b 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -282,6 +282,7 @@ bool aarch64_insn_is_nop(u32 insn);
>  
>  int aarch64_insn_read(void *addr, u32 *insnp);
>  int aarch64_insn_write(void *addr, u32 insn);
> +int aarch64_insn_write_early(void *addr, u32 insn);
>  enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn);
>  u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>  				  u32 insn, u64 imm);
> @@ -352,6 +353,7 @@ u32 aarch64_insn_gen_logical_shifted_reg(enum aarch64_insn_register dst,
>  bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn);
>  
>  int aarch64_insn_patch_text_nosync(void *addr, u32 insn);
> +int __aarch64_insn_patch_text_nosync(void *addr, u32 insn, bool early);
>  int aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt);
>  int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt);
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> index 8cd27fe..b2cad38 100644
> --- a/arch/arm64/kernel/insn.c
> +++ b/arch/arm64/kernel/insn.c
> @@ -19,12 +19,15 @@
>  #include <linux/bitops.h>
>  #include <linux/compiler.h>
>  #include <linux/kernel.h>
> +#include <linux/mm.h>
>  #include <linux/smp.h>
> +#include <linux/spinlock.h>
>  #include <linux/stop_machine.h>
>  #include <linux/uaccess.h>
>  
>  #include <asm/cacheflush.h>
>  #include <asm/debug-monitors.h>
> +#include <asm/fixmap.h>
>  #include <asm/insn.h>
>  
>  #define AARCH64_INSN_SF_BIT	BIT(31)
> @@ -72,6 +75,36 @@ bool __kprobes aarch64_insn_is_nop(u32 insn)
>  	}
>  }
>  
> +static DEFINE_SPINLOCK(patch_lock);
> +
> +static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
> +{
> +	unsigned long uintaddr = (uintptr_t) addr;
> +	bool module = !core_kernel_text(uintaddr);
> +	struct page *page;
> +
> +	if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
> +		page = vmalloc_to_page(addr);
> +	else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA))
> +		page = virt_to_page(addr);
> +	else
> +		return addr;
> +
> +	if (flags)
> +		spin_lock_irqsave(&patch_lock, *flags);
> +
> +	set_fixmap(fixmap, page_to_phys(page));
> +
> +	return (void *) (__fix_to_virt(fixmap) + (uintaddr & ~PAGE_MASK));
> +}
> +
> +static void __kprobes patch_unmap(int fixmap, unsigned long *flags)
> +{
> +	clear_fixmap(fixmap);
> +
> +	if (flags)
> +		spin_unlock_irqrestore(&patch_lock, *flags);
> +}
>  /*
>   * In ARMv8-A, A64 instructions have a fixed length of 32 bits and are always
>   * little-endian.
> @@ -88,10 +121,34 @@ int __kprobes aarch64_insn_read(void *addr, u32 *insnp)
>  	return ret;
>  }
>  
> +static int __kprobes __aarch64_insn_write(void *addr, u32 insn, bool patch)
> +{
> +	void *waddr = addr;
> +	unsigned long flags;
> +	int ret;
> +
> +	if (patch)
> +		waddr = patch_map(addr, FIX_TEXT_POKE0, &flags);
> +
> +	ret = probe_kernel_write(waddr, &insn, AARCH64_INSN_SIZE);
> +
> +	if (waddr != addr)
> +		patch_unmap(FIX_TEXT_POKE0, &flags);
> +
> +	return ret;
> +}
> +
>  int __kprobes aarch64_insn_write(void *addr, u32 insn)
>  {
>  	insn = cpu_to_le32(insn);
> -	return probe_kernel_write(addr, &insn, AARCH64_INSN_SIZE);
> +	return __aarch64_insn_write(addr, insn, true);
> +}
> +
> +int __kprobes aarch64_insn_write_early(void *addr, u32 insn)
> +{
> +	insn = cpu_to_le32(insn);
> +	return __aarch64_insn_write(addr, insn, false);
> +
>  }
>  
>  static bool __kprobes __aarch64_insn_hotpatch_safe(u32 insn)
> @@ -124,7 +181,7 @@ bool __kprobes aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn)
>  	       __aarch64_insn_hotpatch_safe(new_insn);
>  }
>  
> -int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn)
> +int __kprobes __aarch64_insn_patch_text_nosync(void *addr, u32 insn, bool early)
>  {
>  	u32 *tp = addr;
>  	int ret;
> @@ -133,7 +190,11 @@ int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn)
>  	if ((uintptr_t)tp & 0x3)
>  		return -EINVAL;
>  
> -	ret = aarch64_insn_write(tp, insn);
> +	if (early)
> +		ret = aarch64_insn_write_early(tp, insn);
> +	else
> +		ret = aarch64_insn_write(tp, insn);
> +
>  	if (ret == 0)
>  		flush_icache_range((uintptr_t)tp,
>  				   (uintptr_t)tp + AARCH64_INSN_SIZE);
> @@ -141,6 +202,11 @@ int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn)
>  	return ret;
>  }
>  
> +int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn)
> +{
> +	return __aarch64_insn_patch_text_nosync(addr, insn, false);
> +}
> +
>  struct aarch64_insn_patch {
>  	void		**text_addrs;
>  	u32		*new_insns;
> diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c
> index 263a166..9ac30bb 100644
> --- a/arch/arm64/kernel/jump_label.c
> +++ b/arch/arm64/kernel/jump_label.c
> @@ -38,7 +38,7 @@ static void __arch_jump_label_transform(struct jump_entry *entry,
>  	}
>  
>  	if (is_static)
> -		aarch64_insn_patch_text_nosync(addr, insn);
> +		__aarch64_insn_patch_text_nosync(addr, insn, true);
>  	else
>  		aarch64_insn_patch_text(&addr, &insn, 1);
>  }
> -- 
> Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
> 
> 

[PATCHv6 6/8] arm64: use fixmap for text patching when text is RO

[Laura Abbott]

On 11/25/2014 9:04 AM, Mark Rutland wrote:
> Hi Laura,
>
> It looks like "early" means before we've set up the strict page
> permissions, though as far as I can tell the fixmap will already be
> available before we do any patching. Could we not always use the fixmap
> for patching? Then we only need the patch_{map,unmap} additions, and not
> the changes to distinguish the early cases.
>
>  From testing on Juno with defconfig, all of the early patches were
> avoidable NOP -> NOP changes as part of static key initialisation, which
> I think we can skip similarly to x86 (I'll send a patch shortly). All other
> patches were not early and went via the fixmap.
>
> Even with the avoidable NOP -> NOP patching I did not see a noticeable
> boot time difference from forcing the use of the fixmap.
>

I was basing it off of the arm version which needed the early option.
If arm64 doesn't need it I'll drop it.

> Thanks,
> Mark.
>

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCHv6 6/8] arm64: use fixmap for text patching when text is RO

[Mark Rutland]

On Tue, Nov 25, 2014 at 06:54:14PM +0000, Laura Abbott wrote:
> On 11/25/2014 9:04 AM, Mark Rutland wrote:
> > Hi Laura,
> >
> > It looks like "early" means before we've set up the strict page
> > permissions, though as far as I can tell the fixmap will already be
> > available before we do any patching. Could we not always use the fixmap
> > for patching? Then we only need the patch_{map,unmap} additions, and not
> > the changes to distinguish the early cases.
> >
> >  From testing on Juno with defconfig, all of the early patches were
> > avoidable NOP -> NOP changes as part of static key initialisation, which
> > I think we can skip similarly to x86 (I'll send a patch shortly). All other
> > patches were not early and went via the fixmap.
> >
> > Even with the avoidable NOP -> NOP patching I did not see a noticeable
> > boot time difference from forcing the use of the fixmap.
> >
> 
> I was basing it off of the arm version which needed the early option.
> If arm64 doesn't need it I'll drop it.

Given that it only determines whether or not to use the fixmap, and we
can always use the fixmap, I think we can drop it.

Thanks,
Mark.

[PATCHv6 6/8] arm64: use fixmap for text patching when text is RO

[Catalin Marinas]

On Wed, Nov 26, 2014 at 04:18:02PM +0000, Mark Rutland wrote:
> On Tue, Nov 25, 2014 at 06:54:14PM +0000, Laura Abbott wrote:
> > On 11/25/2014 9:04 AM, Mark Rutland wrote:
> > > Hi Laura,
> > >
> > > It looks like "early" means before we've set up the strict page
> > > permissions, though as far as I can tell the fixmap will already be
> > > available before we do any patching. Could we not always use the fixmap
> > > for patching? Then we only need the patch_{map,unmap} additions, and not
> > > the changes to distinguish the early cases.
> > >
> > >  From testing on Juno with defconfig, all of the early patches were
> > > avoidable NOP -> NOP changes as part of static key initialisation, which
> > > I think we can skip similarly to x86 (I'll send a patch shortly). All other
> > > patches were not early and went via the fixmap.
> > >
> > > Even with the avoidable NOP -> NOP patching I did not see a noticeable
> > > boot time difference from forcing the use of the fixmap.
> > >
> > 
> > I was basing it off of the arm version which needed the early option.
> > If arm64 doesn't need it I'll drop it.
> 
> Given that it only determines whether or not to use the fixmap, and we
> can always use the fixmap, I think we can drop it.

BTW, do we ever expect a write to the temporary POKE0 fixmap to fail? If
not (I don't think it would fail), we can just override the
__probe_kernel_write() to use set_fixmap() (maybe only when this feature
is enabled).

-- 
Catalin

[PATCHv6 7/8] arm64: efi: Use ioremap_exec for code sections

[Laura Abbott]

ioremap is not guaranteed to return memory with proper
execution permissions. Introduce ioremap_exec which will
ensure that permission bits are set as expected for EFI
code sections.

Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm64/include/asm/io.h      |  1 +
 arch/arm64/include/asm/pgtable.h |  1 +
 arch/arm64/kernel/efi.c          | 12 +++++++++++-
 arch/arm64/mm/ioremap.c          | 11 +++++++++++
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 79f1d519..7dd8465 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -230,6 +230,7 @@ extern void __memset_io(volatile void __iomem *, int, size_t);
 extern void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot);
 extern void __iounmap(volatile void __iomem *addr);
 extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
+extern void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size);
 
 #define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
 #define ioremap_nocache(addr, size)	__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 41a43bf..9b1d9d0 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -65,6 +65,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
 #define PROT_DEVICE_nGnRE	(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_DEVICE_nGnRE))
 #define PROT_NORMAL_NC		(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL_NC))
 #define PROT_NORMAL		(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL))
+#define PROT_NORMAL_EXEC	(PROT_DEFAULT | PTE_UXN | PTE_ATTRINDX(MT_NORMAL))
 
 #define PROT_SECT_DEVICE_nGnRE	(PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_DEVICE_nGnRE))
 #define PROT_SECT_NORMAL	(PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_NORMAL))
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 95c49eb..9e41f95 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -47,6 +47,14 @@ static int __init is_normal_ram(efi_memory_desc_t *md)
 	return 0;
 }
 
+static int __init is_code(efi_memory_desc_t *md)
+{
+	if (md->attribute & EFI_RUNTIME_SERVICES_CODE)
+		return 1;
+	return 0;
+}
+
+
 static void __init efi_setup_idmap(void)
 {
 	struct memblock_region *r;
@@ -338,7 +346,9 @@ static int __init remap_region(efi_memory_desc_t *md, void **new)
 	memrange_efi_to_native(&paddr, &npages);
 	size = npages << PAGE_SHIFT;
 
-	if (is_normal_ram(md))
+	if (is_code(md))
+		vaddr = (__force u64)ioremap_exec(paddr, size);
+	else if (is_normal_ram(md))
 		vaddr = (__force u64)ioremap_cache(paddr, size);
 	else
 		vaddr = (__force u64)ioremap(paddr, size);
diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
index cbb99c8..b998441 100644
--- a/arch/arm64/mm/ioremap.c
+++ b/arch/arm64/mm/ioremap.c
@@ -103,6 +103,17 @@ void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
 }
 EXPORT_SYMBOL(ioremap_cache);
 
+void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size)
+{
+	/* For normal memory we already have a cacheable mapping. */
+	if (pfn_valid(__phys_to_pfn(phys_addr)))
+		return (void __iomem *)__phys_to_virt(phys_addr);
+
+	return __ioremap_caller(phys_addr, size, __pgprot(PROT_NORMAL_EXEC),
+				__builtin_return_address(0));
+}
+EXPORT_SYMBOL(ioremap_exec);
+
 /*
  * Must be called after early_fixmap_init
  */
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6 7/8] arm64: efi: Use ioremap_exec for code sections

[Mark Rutland]

Hi Laura,

On Fri, Nov 21, 2014 at 09:50:44PM +0000, Laura Abbott wrote:
> ioremap is not guaranteed to return memory with proper
> execution permissions. Introduce ioremap_exec which will
> ensure that permission bits are set as expected for EFI
> code sections.
> 
> Tested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
> ---
>  arch/arm64/include/asm/io.h      |  1 +
>  arch/arm64/include/asm/pgtable.h |  1 +
>  arch/arm64/kernel/efi.c          | 12 +++++++++++-
>  arch/arm64/mm/ioremap.c          | 11 +++++++++++
>  4 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
> index 79f1d519..7dd8465 100644
> --- a/arch/arm64/include/asm/io.h
> +++ b/arch/arm64/include/asm/io.h
> @@ -230,6 +230,7 @@ extern void __memset_io(volatile void __iomem *, int, size_t);
>  extern void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot);
>  extern void __iounmap(volatile void __iomem *addr);
>  extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
> +extern void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size);
>  
>  #define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
>  #define ioremap_nocache(addr, size)	__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 41a43bf..9b1d9d0 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -65,6 +65,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
>  #define PROT_DEVICE_nGnRE	(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_DEVICE_nGnRE))
>  #define PROT_NORMAL_NC		(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL_NC))
>  #define PROT_NORMAL		(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL))
> +#define PROT_NORMAL_EXEC	(PROT_DEFAULT | PTE_UXN | PTE_ATTRINDX(MT_NORMAL))
>  
>  #define PROT_SECT_DEVICE_nGnRE	(PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_DEVICE_nGnRE))
>  #define PROT_SECT_NORMAL	(PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_NORMAL))
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 95c49eb..9e41f95 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -47,6 +47,14 @@ static int __init is_normal_ram(efi_memory_desc_t *md)
>  	return 0;
>  }
>  
> +static int __init is_code(efi_memory_desc_t *md)
> +{
> +	if (md->attribute & EFI_RUNTIME_SERVICES_CODE)
> +		return 1;
> +	return 0;
> +}
> +
> +
>  static void __init efi_setup_idmap(void)
>  {
>  	struct memblock_region *r;
> @@ -338,7 +346,9 @@ static int __init remap_region(efi_memory_desc_t *md, void **new)
>  	memrange_efi_to_native(&paddr, &npages);
>  	size = npages << PAGE_SHIFT;
>  
> -	if (is_normal_ram(md))
> +	if (is_code(md))
> +		vaddr = (__force u64)ioremap_exec(paddr, size);
> +	else if (is_normal_ram(md))
>  		vaddr = (__force u64)ioremap_cache(paddr, size);
>  	else
>  		vaddr = (__force u64)ioremap(paddr, size);

All of the above looks fine to me.

> diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
> index cbb99c8..b998441 100644
> --- a/arch/arm64/mm/ioremap.c
> +++ b/arch/arm64/mm/ioremap.c
> @@ -103,6 +103,17 @@ void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
>  }
>  EXPORT_SYMBOL(ioremap_cache);
>  
> +void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size)
> +{
> +	/* For normal memory we already have a cacheable mapping. */
> +	if (pfn_valid(__phys_to_pfn(phys_addr)))
> +		return (void __iomem *)__phys_to_virt(phys_addr);

Is this guaranteed to be executable in all cases once the stricter page
permissions are in force?

Thanks,
Mark.

> +
> +	return __ioremap_caller(phys_addr, size, __pgprot(PROT_NORMAL_EXEC),
> +				__builtin_return_address(0));
> +}
> +EXPORT_SYMBOL(ioremap_exec);
> +
>  /*
>   * Must be called after early_fixmap_init
>   */
> -- 
> Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
> 
> 

[PATCHv6 7/8] arm64: efi: Use ioremap_exec for code sections

[Laura Abbott]

On 11/25/2014 9:26 AM, Mark Rutland wrote:
> Hi Laura,
>
> On Fri, Nov 21, 2014 at 09:50:44PM +0000, Laura Abbott wrote:
>> ioremap is not guaranteed to return memory with proper
>> execution permissions. Introduce ioremap_exec which will
>> ensure that permission bits are set as expected for EFI
>> code sections.
>>
>> Tested-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
>> ---
>>   arch/arm64/include/asm/io.h      |  1 +
>>   arch/arm64/include/asm/pgtable.h |  1 +
>>   arch/arm64/kernel/efi.c          | 12 +++++++++++-
>>   arch/arm64/mm/ioremap.c          | 11 +++++++++++
>>   4 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
>> index 79f1d519..7dd8465 100644
>> --- a/arch/arm64/include/asm/io.h
>> +++ b/arch/arm64/include/asm/io.h
>> @@ -230,6 +230,7 @@ extern void __memset_io(volatile void __iomem *, int, size_t);
>>   extern void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot);
>>   extern void __iounmap(volatile void __iomem *addr);
>>   extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
>> +extern void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size);
>>
>>   #define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
>>   #define ioremap_nocache(addr, size)	__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index 41a43bf..9b1d9d0 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -65,6 +65,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
>>   #define PROT_DEVICE_nGnRE	(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_DEVICE_nGnRE))
>>   #define PROT_NORMAL_NC		(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL_NC))
>>   #define PROT_NORMAL		(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL))
>> +#define PROT_NORMAL_EXEC	(PROT_DEFAULT | PTE_UXN | PTE_ATTRINDX(MT_NORMAL))
>>
>>   #define PROT_SECT_DEVICE_nGnRE	(PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_DEVICE_nGnRE))
>>   #define PROT_SECT_NORMAL	(PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_NORMAL))
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index 95c49eb..9e41f95 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -47,6 +47,14 @@ static int __init is_normal_ram(efi_memory_desc_t *md)
>>   	return 0;
>>   }
>>
>> +static int __init is_code(efi_memory_desc_t *md)
>> +{
>> +	if (md->attribute & EFI_RUNTIME_SERVICES_CODE)
>> +		return 1;
>> +	return 0;
>> +}
>> +
>> +
>>   static void __init efi_setup_idmap(void)
>>   {
>>   	struct memblock_region *r;
>> @@ -338,7 +346,9 @@ static int __init remap_region(efi_memory_desc_t *md, void **new)
>>   	memrange_efi_to_native(&paddr, &npages);
>>   	size = npages << PAGE_SHIFT;
>>
>> -	if (is_normal_ram(md))
>> +	if (is_code(md))
>> +		vaddr = (__force u64)ioremap_exec(paddr, size);
>> +	else if (is_normal_ram(md))
>>   		vaddr = (__force u64)ioremap_cache(paddr, size);
>>   	else
>>   		vaddr = (__force u64)ioremap(paddr, size);
>
> All of the above looks fine to me.
>
>> diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
>> index cbb99c8..b998441 100644
>> --- a/arch/arm64/mm/ioremap.c
>> +++ b/arch/arm64/mm/ioremap.c
>> @@ -103,6 +103,17 @@ void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
>>   }
>>   EXPORT_SYMBOL(ioremap_cache);
>>
>> +void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size)
>> +{
>> +	/* For normal memory we already have a cacheable mapping. */
>> +	if (pfn_valid(__phys_to_pfn(phys_addr)))
>> +		return (void __iomem *)__phys_to_virt(phys_addr);
>
> Is this guaranteed to be executable in all cases once the stricter page
> permissions are in force?
>

No but the updated version of the patch which adds the stricter page
permission adds a call to update the page permissions to be executable.
  
> Thanks,
> Mark.
>

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCHv6 7/8] arm64: efi: Use ioremap_exec for code sections

[Catalin Marinas]

On Fri, Nov 21, 2014 at 09:50:44PM +0000, Laura Abbott wrote:
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 95c49eb..9e41f95 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -47,6 +47,14 @@ static int __init is_normal_ram(efi_memory_desc_t *md)
>  	return 0;
>  }
>  
> +static int __init is_code(efi_memory_desc_t *md)
> +{
> +	if (md->attribute & EFI_RUNTIME_SERVICES_CODE)
> +		return 1;
> +	return 0;
> +}
> +
> +
>  static void __init efi_setup_idmap(void)
>  {
>  	struct memblock_region *r;
> @@ -338,7 +346,9 @@ static int __init remap_region(efi_memory_desc_t *md, void **new)
>  	memrange_efi_to_native(&paddr, &npages);
>  	size = npages << PAGE_SHIFT;
>  
> -	if (is_normal_ram(md))
> +	if (is_code(md))
> +		vaddr = (__force u64)ioremap_exec(paddr, size);
> +	else if (is_normal_ram(md))
>  		vaddr = (__force u64)ioremap_cache(paddr, size);
>  	else
>  		vaddr = (__force u64)ioremap(paddr, size);

What I don't understand is that currently the above remap_region()
function only uses ioremap_cache() and ignores any code attributes. So
we end up with PROT_NORMAL which is non-executable.

Is the current remap_region() function broken or we don't expect it to
be called with any EFI_RUNTIME_SERVICES_CODE region?

> diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
> index cbb99c8..b998441 100644
> --- a/arch/arm64/mm/ioremap.c
> +++ b/arch/arm64/mm/ioremap.c
> @@ -103,6 +103,17 @@ void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
>  }
>  EXPORT_SYMBOL(ioremap_cache);
>  
> +void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size)
> +{
> +	/* For normal memory we already have a cacheable mapping. */
> +	if (pfn_valid(__phys_to_pfn(phys_addr)))
> +		return (void __iomem *)__phys_to_virt(phys_addr);
> +
> +	return __ioremap_caller(phys_addr, size, __pgprot(PROT_NORMAL_EXEC),
> +				__builtin_return_address(0));

In addition to what I said above, do we expect ioremap_exec() to be
called on non-pfn_valid() pages?

-- 
Catalin

[PATCHv6 7/8] arm64: efi: Use ioremap_exec for code sections

[Ard Biesheuvel]

On 2 December 2014 at 18:15, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Fri, Nov 21, 2014 at 09:50:44PM +0000, Laura Abbott wrote:
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index 95c49eb..9e41f95 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -47,6 +47,14 @@ static int __init is_normal_ram(efi_memory_desc_t *md)
>>       return 0;
>>  }
>>
>> +static int __init is_code(efi_memory_desc_t *md)
>> +{
>> +     if (md->attribute & EFI_RUNTIME_SERVICES_CODE)
>> +             return 1;
>> +     return 0;
>> +}
>> +
>> +
>>  static void __init efi_setup_idmap(void)
>>  {
>>       struct memblock_region *r;
>> @@ -338,7 +346,9 @@ static int __init remap_region(efi_memory_desc_t *md, void **new)
>>       memrange_efi_to_native(&paddr, &npages);
>>       size = npages << PAGE_SHIFT;
>>
>> -     if (is_normal_ram(md))
>> +     if (is_code(md))
>> +             vaddr = (__force u64)ioremap_exec(paddr, size);
>> +     else if (is_normal_ram(md))
>>               vaddr = (__force u64)ioremap_cache(paddr, size);
>>       else
>>               vaddr = (__force u64)ioremap(paddr, size);
>
> What I don't understand is that currently the above remap_region()
> function only uses ioremap_cache() and ignores any code attributes. So
> we end up with PROT_NORMAL which is non-executable.
>

ioremap_cache() reuses the linear mapping if it covers the ioremapped
physical address.
That is why it currently works by accident.

> Is the current remap_region() function broken or we don't expect it to
> be called with any EFI_RUNTIME_SERVICES_CODE region?
>

It is called with such regions, but once we remove the UEFI reserved
regions from the linear mapping, this breaks down.
My virtmap series for 3.20 (for kexec with efi, mostly) removes all of
this code, in fact, so this patch will not even be necessary in that
case.

>> diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
>> index cbb99c8..b998441 100644
>> --- a/arch/arm64/mm/ioremap.c
>> +++ b/arch/arm64/mm/ioremap.c
>> @@ -103,6 +103,17 @@ void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
>>  }
>>  EXPORT_SYMBOL(ioremap_cache);
>>
>> +void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size)
>> +{
>> +     /* For normal memory we already have a cacheable mapping. */
>> +     if (pfn_valid(__phys_to_pfn(phys_addr)))
>> +             return (void __iomem *)__phys_to_virt(phys_addr);
>> +
>> +     return __ioremap_caller(phys_addr, size, __pgprot(PROT_NORMAL_EXEC),
>> +                             __builtin_return_address(0));
>
> In addition to what I said above, do we expect ioremap_exec() to be
> called on non-pfn_valid() pages?
>

I think introducing ioremap_exec() strictly for EFI at this point may
be redundant.
I intend to send the next version of the EFI virtmap for kexec series tomorrow.

-- 
Ard.

[PATCHv6] arm64: add better page protections to arm64

[Laura Abbott]

Add page protections for arm64 similar to those in arm.
This is for security reasons to prevent certain classes
of exploits. The current method:

- Map all memory as either RWX or RW. We round to the nearest
  section to avoid creating page tables before everything is mapped
- Once everything is mapped, if either end of the RWX section should
  not be X, we split the PMD and remap as necessary
- When initmem is to be freed, we change the permissions back to
  RW (using stop machine if necessary to flush the TLB)
- If CONFIG_DEBUG_RODATA is set, the read only sections are set
  read only.

Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
v6: Adjusted the macros in vmlinux.lds.S. Changed the create_mapping functions
to take an allocation parameter and dropped the __ref from everything. Fix
pointed out by Steve Capper in split_pud. Introduction of adjust_exec_mem
for fixing up ioremap_exec as needed.
---
 arch/arm64/Kconfig.debug            |  23 +++
 arch/arm64/include/asm/cacheflush.h |   5 +
 arch/arm64/kernel/vmlinux.lds.S     |  18 ++-
 arch/arm64/mm/init.c                |   1 +
 arch/arm64/mm/ioremap.c             |   9 +-
 arch/arm64/mm/mm.h                  |   4 +
 arch/arm64/mm/mmu.c                 | 311 +++++++++++++++++++++++++++++++-----
 7 files changed, 325 insertions(+), 46 deletions(-)

diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index 0a12933..867fe6f1 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -54,4 +54,27 @@ config DEBUG_SET_MODULE_RONX
           against certain classes of kernel exploits.
           If in doubt, say "N".
 
+config DEBUG_RODATA
+	bool "Make kernel text and rodata read-only"
+	help
+	  If this is set, kernel text and rodata will be made read-only. This
+	  is to help catch accidental or malicious attempts to change the
+	  kernel's executable code. Additionally splits rodata from kernel
+	  text so it can be made explicitly non-executable.
+
+          If in doubt, say Y
+
+config DEBUG_ALIGN_RODATA
+	depends on DEBUG_RODATA && !ARM64_64K_PAGES
+	bool "Align linker sections up to SECTION_SIZE"
+	help
+	  If this option is enabled, sections that may potentially be marked as
+	  read only or non-executable will be aligned up to the section size of
+	  the kernel. This prevents sections from being split into pages and
+	  avoids a potential TLB penalty. The downside is an increase in
+	  alignment and potentially wasted space. Turn on this option if
+	  performance is more important than memory pressure.
+
+	  If in doubt, say N
+
 endmenu
diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
index 689b637..81a5e4d 100644
--- a/arch/arm64/include/asm/cacheflush.h
+++ b/arch/arm64/include/asm/cacheflush.h
@@ -152,4 +152,9 @@ int set_memory_ro(unsigned long addr, int numpages);
 int set_memory_rw(unsigned long addr, int numpages);
 int set_memory_x(unsigned long addr, int numpages);
 int set_memory_nx(unsigned long addr, int numpages);
+
+#ifdef CONFIG_DEBUG_RODATA
+void mark_rodata_ro(void);
+#endif
+
 #endif
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index edf8715..6b132f9 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -8,6 +8,7 @@
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/page.h>
+#include <asm/pgtable.h>
 
 #include "image.h"
 
@@ -32,6 +33,15 @@ jiffies = jiffies_64;
 	*(.hyp.text)					\
 	VMLINUX_SYMBOL(__hyp_text_end) = .;
 
+
+#ifdef CONFIG_DEBUG_ALIGN_RODATA
+#define ALIGN_DEBUG_RO			. = ALIGN(1<<SECTION_SHIFT);
+#define ALIGN_DEBUG_RO_MIN(min)		ALIGN_DEBUG_RO
+#else
+#define ALIGN_DEBUG_RO
+#define ALIGN_DEBUG_RO_MIN(min)		. = ALIGN(min);
+#endif
+
 SECTIONS
 {
 	/*
@@ -54,6 +64,7 @@ SECTIONS
 		_text = .;
 		HEAD_TEXT
 	}
+	ALIGN_DEBUG_RO
 	.text : {			/* Real text segment		*/
 		_stext = .;		/* Text and read-only data	*/
 			__exception_text_start = .;
@@ -70,19 +81,22 @@ SECTIONS
 		*(.got)			/* Global offset table		*/
 	}
 
+	ALIGN_DEBUG_RO
 	RO_DATA(PAGE_SIZE)
 	EXCEPTION_TABLE(8)
 	NOTES
+	ALIGN_DEBUG_RO
 	_etext = .;			/* End of text and rodata section */
 
-	. = ALIGN(PAGE_SIZE);
+	ALIGN_DEBUG_RO_MIN(PAGE_SIZE)
 	__init_begin = .;
 
 	INIT_TEXT_SECTION(8)
 	.exit.text : {
 		ARM_EXIT_KEEP(EXIT_TEXT)
 	}
-	. = ALIGN(16);
+
+	ALIGN_DEBUG_RO_MIN(16)
 	.init.data : {
 		INIT_DATA
 		INIT_SETUP(16)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 494297c..61f44c7 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -324,6 +324,7 @@ void __init mem_init(void)
 
 void free_initmem(void)
 {
+	fixup_init();
 	free_initmem_default(0);
 }
 
diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
index b998441..265dc2d 100644
--- a/arch/arm64/mm/ioremap.c
+++ b/arch/arm64/mm/ioremap.c
@@ -29,6 +29,8 @@
 #include <asm/tlbflush.h>
 #include <asm/pgalloc.h>
 
+#include "mm.h"
+
 static void __iomem *__ioremap_caller(phys_addr_t phys_addr, size_t size,
 				      pgprot_t prot, void *caller)
 {
@@ -106,8 +108,11 @@ EXPORT_SYMBOL(ioremap_cache);
 void __iomem *ioremap_exec(phys_addr_t phys_addr, size_t size)
 {
 	/* For normal memory we already have a cacheable mapping. */
-	if (pfn_valid(__phys_to_pfn(phys_addr)))
-		return (void __iomem *)__phys_to_virt(phys_addr);
+	if (pfn_valid(__phys_to_pfn(phys_addr))) {
+		unsigned long val = __phys_to_virt(phys_addr);
+		adjust_exec_mem(val, val + size);
+		return (void __iomem *)val;
+	}
 
 	return __ioremap_caller(phys_addr, size, __pgprot(PROT_NORMAL_EXEC),
 				__builtin_return_address(0));
diff --git a/arch/arm64/mm/mm.h b/arch/arm64/mm/mm.h
index d519f4f..b4eda23 100644
--- a/arch/arm64/mm/mm.h
+++ b/arch/arm64/mm/mm.h
@@ -1,2 +1,6 @@
 extern void __init bootmem_init(void);
 extern void __init arm64_swiotlb_init(void);
+
+void fixup_init(void);
+void adjust_exec_mem(unsigned long start, unsigned long end);
+
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6032f3e..cc84d9c 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -26,6 +26,7 @@
 #include <linux/memblock.h>
 #include <linux/fs.h>
 #include <linux/io.h>
+#include <linux/stop_machine.h>
 
 #include <asm/cputype.h>
 #include <asm/fixmap.h>
@@ -137,17 +138,50 @@ static void __init *early_alloc(unsigned long sz)
 	return ptr;
 }
 
-static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr,
+/*
+ * remap a PMD into pages
+ */
+static noinline void split_pmd(pmd_t *pmd,
+				void *(*alloc)(unsigned long size))
+{
+	pte_t *pte, *start_pte;
+	unsigned long pfn;
+	int i = 0;
+
+	start_pte = pte = alloc(PTRS_PER_PTE*sizeof(pte_t));
+	BUG_ON(!pte);
+
+	pfn = pmd_pfn(*pmd);
+
+	do {
+		/*
+		 * Need to have the least restrictive permissions available
+		 * permissions will be fixed up later
+		 */
+		set_pte(pte, pfn_pte(pfn, PAGE_KERNEL_EXEC));
+		pfn++;
+	} while (pte++, i++, i < PTRS_PER_PTE);
+
+
+	__pmd_populate(pmd, __pa(start_pte), PMD_TYPE_TABLE);
+	flush_tlb_all();
+}
+
+static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
 				  unsigned long end, unsigned long pfn,
-				  pgprot_t prot)
+				  pgprot_t prot,
+				  void *(*alloc)(unsigned long size))
 {
 	pte_t *pte;
 
 	if (pmd_none(*pmd)) {
-		pte = early_alloc(PTRS_PER_PTE * sizeof(pte_t));
+		pte = alloc(PTRS_PER_PTE * sizeof(pte_t));
+		BUG_ON(!pte);
 		__pmd_populate(pmd, __pa(pte), PMD_TYPE_TABLE);
 	}
-	BUG_ON(pmd_bad(*pmd));
+
+	if (pmd_bad(*pmd))
+		split_pmd(pmd, alloc);
 
 	pte = pte_offset_kernel(pmd, addr);
 	do {
@@ -156,29 +190,41 @@ static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr,
 	} while (pte++, addr += PAGE_SIZE, addr != end);
 }
 
-static void __init alloc_init_pmd(pud_t *pud, unsigned long addr,
+void split_pud(pud_t *old_pud, pmd_t *pmd)
+{
+	unsigned long addr = pud_pfn(*old_pud) << PAGE_SHIFT;
+	pgprot_t prot = __pgprot(pud_val(*old_pud) ^ addr);
+	int i = 0;
+
+	do {
+		set_pmd(pmd, __pmd(addr | prot));
+		addr += PMD_SIZE;
+	} while (pmd++, i++, i < PTRS_PER_PMD);
+}
+
+static void alloc_init_pmd(pud_t *pud, unsigned long addr,
 				  unsigned long end, phys_addr_t phys,
-				  int map_io)
+				  pgprot_t sect_prot, pgprot_t pte_prot,
+				  void *(*alloc)(unsigned long size))
 {
 	pmd_t *pmd;
 	unsigned long next;
-	pmdval_t prot_sect;
-	pgprot_t prot_pte;
-
-	if (map_io) {
-		prot_sect = PROT_SECT_DEVICE_nGnRE;
-		prot_pte = __pgprot(PROT_DEVICE_nGnRE);
-	} else {
-		prot_sect = PROT_SECT_NORMAL_EXEC;
-		prot_pte = PAGE_KERNEL_EXEC;
-	}
 
 	/*
 	 * Check for initial section mappings in the pgd/pud and remove them.
 	 */
 	if (pud_none(*pud) || pud_bad(*pud)) {
-		pmd = early_alloc(PTRS_PER_PMD * sizeof(pmd_t));
+		pmd = alloc(PTRS_PER_PMD * sizeof(pmd_t));
+		BUG_ON(!pmd);
+		if (pud_sect(*pud)) {
+			/*
+			 * need to have the 1G of mappings continue to be
+			 * present
+			 */
+			split_pud(pud, pmd);
+		}
 		pud_populate(&init_mm, pud, pmd);
+		flush_tlb_all();
 	}
 
 	pmd = pmd_offset(pud, addr);
@@ -186,8 +232,8 @@ static void __init alloc_init_pmd(pud_t *pud, unsigned long addr,
 		next = pmd_addr_end(addr, end);
 		/* try section mapping first */
 		if (((addr | next | phys) & ~SECTION_MASK) == 0) {
-			pmd_t old_pmd =*pmd;
-			set_pmd(pmd, __pmd(phys | prot_sect));
+			pmd_t old_pmd = *pmd;
+			set_pmd(pmd, __pmd(phys | sect_prot));
 			/*
 			 * Check for previous table entries created during
 			 * boot (__create_page_tables) and flush them.
@@ -196,21 +242,42 @@ static void __init alloc_init_pmd(pud_t *pud, unsigned long addr,
 				flush_tlb_all();
 		} else {
 			alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys),
-				       prot_pte);
+					pte_prot, alloc);
 		}
 		phys += next - addr;
 	} while (pmd++, addr = next, addr != end);
 }
 
-static void __init alloc_init_pud(pgd_t *pgd, unsigned long addr,
-				  unsigned long end, phys_addr_t phys,
-				  int map_io)
+static inline bool use_1G_block(unsigned long addr, unsigned long next,
+			unsigned long phys, pgprot_t sect_prot,
+			pgprot_t pte_prot)
+{
+	if (PAGE_SHIFT != 12)
+		return false;
+
+	if (((addr | next | phys) & ~PUD_MASK) != 0)
+		return false;
+
+	/*
+	 * The assumption here is that if the memory is anything other
+	 * than normal we should not be using a block type
+	 */
+	return ((sect_prot & PMD_ATTRINDX_MASK) ==
+				PMD_ATTRINDX(MT_NORMAL)) &&
+			((pte_prot & PTE_ATTRINDX_MASK) ==
+				PTE_ATTRINDX(MT_NORMAL));
+}
+
+static void alloc_init_pud(pgd_t *pgd, unsigned long addr,
+				  unsigned long end, unsigned long phys,
+				  pgprot_t sect_prot, pgprot_t pte_prot,
+				  void *(*alloc)(unsigned long size))
 {
 	pud_t *pud;
 	unsigned long next;
 
 	if (pgd_none(*pgd)) {
-		pud = early_alloc(PTRS_PER_PUD * sizeof(pud_t));
+		pud = alloc(PTRS_PER_PUD * sizeof(pud_t));
 		pgd_populate(&init_mm, pgd, pud);
 	}
 	BUG_ON(pgd_bad(*pgd));
@@ -222,10 +289,9 @@ static void __init alloc_init_pud(pgd_t *pgd, unsigned long addr,
 		/*
 		 * For 4K granule only, attempt to put down a 1GB block
 		 */
-		if (!map_io && (PAGE_SHIFT == 12) &&
-		    ((addr | next | phys) & ~PUD_MASK) == 0) {
+		if (use_1G_block(addr, next, phys, sect_prot, pte_prot)) {
 			pud_t old_pud = *pud;
-			set_pud(pud, __pud(phys | PROT_SECT_NORMAL_EXEC));
+			set_pud(pud, __pud(phys | sect_prot));
 
 			/*
 			 * If we have an old value for a pud, it will
@@ -240,7 +306,8 @@ static void __init alloc_init_pud(pgd_t *pgd, unsigned long addr,
 				flush_tlb_all();
 			}
 		} else {
-			alloc_init_pmd(pud, addr, next, phys, map_io);
+			alloc_init_pmd(pud, addr, next, phys, sect_prot,
+					pte_prot, alloc);
 		}
 		phys += next - addr;
 	} while (pud++, addr = next, addr != end);
@@ -250,9 +317,11 @@ static void __init alloc_init_pud(pgd_t *pgd, unsigned long addr,
  * Create the page directory entries and any necessary page tables for the
  * mapping specified by 'md'.
  */
-static void __init __create_mapping(pgd_t *pgd, phys_addr_t phys,
-				    unsigned long virt, phys_addr_t size,
-				    int map_io)
+static void __create_mapping(pgd_t *pgd, phys_addr_t phys,
+				  unsigned long virt,
+				  phys_addr_t size,
+				  pgprot_t sect_prot, pgprot_t pte_prot,
+				  void *(*alloc)(unsigned long size))
 {
 	unsigned long addr, length, end, next;
 
@@ -262,32 +331,109 @@ static void __init __create_mapping(pgd_t *pgd, phys_addr_t phys,
 	end = addr + length;
 	do {
 		next = pgd_addr_end(addr, end);
-		alloc_init_pud(pgd, addr, next, phys, map_io);
+		alloc_init_pud(pgd, addr, next, phys, sect_prot, pte_prot,
+				alloc);
 		phys += next - addr;
 	} while (pgd++, addr = next, addr != end);
 }
 
-static void __init create_mapping(phys_addr_t phys, unsigned long virt,
-				  phys_addr_t size)
+void __init create_id_mapping(phys_addr_t addr, phys_addr_t size, int map_io)
+{
+	pgprot_t sect_prot = PROT_SECT_NORMAL_EXEC;
+	pgprot_t pte_prot = PAGE_KERNEL_EXEC;
+
+	if ((addr >> PGDIR_SHIFT) >= ARRAY_SIZE(idmap_pg_dir)) {
+		pr_warn("BUG: not creating id mapping for %pa\n", &addr);
+		return;
+	}
+
+	if (map_io) {
+		sect_prot = PROT_SECT_DEVICE_nGnRE;
+		pte_prot = __pgprot(PROT_DEVICE_nGnRE);
+	}
+
+	__create_mapping(&idmap_pg_dir[pgd_index(addr)],
+			 addr, addr, size, sect_prot, pte_prot, early_alloc);
+}
+
+static void *late_alloc(unsigned long size)
+{
+	BUG_ON(size > PAGE_SIZE);
+	return (void *)__get_free_page(PGALLOC_GFP);
+}
+
+static void __ref create_mapping(phys_addr_t phys, unsigned long virt,
+				  phys_addr_t size,
+				  pgprot_t sect_prot, pgprot_t pte_prot)
 {
 	if (virt < VMALLOC_START) {
 		pr_warn("BUG: not creating mapping for %pa@0x%016lx - outside kernel range\n",
 			&phys, virt);
 		return;
 	}
-	__create_mapping(pgd_offset_k(virt & PAGE_MASK), phys, virt, size, 0);
+
+	return __create_mapping(pgd_offset_k(virt & PAGE_MASK), phys, virt,
+				size, sect_prot, pte_prot, early_alloc);
 }
 
-void __init create_id_mapping(phys_addr_t addr, phys_addr_t size, int map_io)
+static void create_mapping_late(phys_addr_t phys, unsigned long virt,
+				  phys_addr_t size,
+				  pgprot_t sect_prot, pgprot_t pte_prot)
 {
-	if ((addr >> PGDIR_SHIFT) >= ARRAY_SIZE(idmap_pg_dir)) {
-		pr_warn("BUG: not creating id mapping for %pa\n", &addr);
+	if (virt < VMALLOC_START) {
+		pr_warn("BUG: not creating mapping for %pa@0x%016lx - outside kernel range\n",
+			&phys, virt);
 		return;
 	}
-	__create_mapping(&idmap_pg_dir[pgd_index(addr)],
-			 addr, addr, size, map_io);
+
+	return __create_mapping(pgd_offset_k(virt & PAGE_MASK), phys, virt,
+				size, sect_prot, pte_prot, late_alloc);
 }
 
+#ifdef CONFIG_DEBUG_RODATA
+static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
+{
+	/*
+	 * Set up the executable regions using the existing section mappings
+	 * for now. This will get more fine grained later once all memory
+	 * is mapped
+	 */
+	unsigned long kernel_x_start = round_down(__pa(_stext), SECTION_SIZE);
+	unsigned long kernel_x_end = round_up(__pa(__init_end), SECTION_SIZE);
+
+	if (end < kernel_x_start) {
+		create_mapping(start, __phys_to_virt(start),
+			end - start, PROT_SECT_NORMAL, PAGE_KERNEL);
+	} else if (start >= kernel_x_end) {
+		create_mapping(start, __phys_to_virt(start),
+			end - start, PROT_SECT_NORMAL, PAGE_KERNEL);
+	} else {
+		if (start < kernel_x_start)
+			create_mapping(start, __phys_to_virt(start),
+				kernel_x_start - start,
+				PROT_SECT_NORMAL,
+				PAGE_KERNEL);
+		create_mapping(kernel_x_start,
+				__phys_to_virt(kernel_x_start),
+				kernel_x_end - kernel_x_start,
+				PROT_SECT_NORMAL_EXEC, PAGE_KERNEL_EXEC);
+		if (kernel_x_end < end)
+			create_mapping(kernel_x_end,
+				__phys_to_virt(kernel_x_end),
+				end - kernel_x_end,
+				PROT_SECT_NORMAL,
+				PAGE_KERNEL);
+	}
+
+}
+#else
+static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
+{
+	create_mapping(start, __phys_to_virt(start), end - start,
+			PROT_SECT_NORMAL_EXEC, PAGE_KERNEL_EXEC);
+}
+#endif
+
 static void __init map_mem(void)
 {
 	struct memblock_region *reg;
@@ -332,14 +478,94 @@ static void __init map_mem(void)
 			memblock_set_current_limit(limit);
 		}
 #endif
-
-		create_mapping(start, __phys_to_virt(start), end - start);
+		__map_memblock(start, end);
 	}
 
 	/* Limit no longer required. */
 	memblock_set_current_limit(MEMBLOCK_ALLOC_ANYWHERE);
 }
 
+void __init fixup_executable(void)
+{
+#ifdef CONFIG_DEBUG_RODATA
+	/* now that we are actually fully mapped, make the start/end more fine grained */
+	if (!IS_ALIGNED((unsigned long)_stext, SECTION_SIZE)) {
+		unsigned long aligned_start = round_down(__pa(_stext),
+							SECTION_SIZE);
+
+		create_mapping(aligned_start, __phys_to_virt(aligned_start),
+				__pa(_stext) - aligned_start,
+				PROT_SECT_NORMAL,
+				PAGE_KERNEL);
+	}
+
+	if (!IS_ALIGNED((unsigned long)__init_end, SECTION_SIZE)) {
+		unsigned long aligned_end = round_up(__pa(__init_end),
+							SECTION_SIZE);
+		create_mapping(__pa(__init_end), (unsigned long)__init_end,
+				aligned_end - __pa(__init_end),
+				PROT_SECT_NORMAL,
+				PAGE_KERNEL);
+	}
+#endif
+}
+
+#ifdef CONFIG_DEBUG_RODATA
+void mark_rodata_ro(void)
+{
+	create_mapping_late(__pa(_stext), (unsigned long)_stext,
+				(unsigned long)_etext - (unsigned long)_stext,
+				PROT_SECT_NORMAL_EXEC | PMD_SECT_RDONLY,
+				PAGE_KERNEL_EXEC | PTE_RDONLY);
+
+}
+#endif
+
+struct flush_addr {
+	unsigned long start;
+	unsigned long end;
+};
+
+static int __flush_mappings(void *val)
+{
+	struct flush_addr *data = val;
+
+	flush_tlb_kernel_range(data->start, data->end);
+	return 0;
+}
+
+static void adjust_mem(unsigned long vstart, unsigned long vend,
+			phys_addr_t phys,
+			pgprot_t sect_prot, pgprot_t pte_prot)
+{
+	struct flush_addr f;
+
+	create_mapping_late(phys, vstart, vend - vstart,
+			sect_prot, pte_prot);
+
+	if (!IS_ALIGNED(vstart, SECTION_SIZE) || !IS_ALIGNED(vend, SECTION_SIZE)) {
+		f.start = vstart;
+		f.end = vend;
+		stop_machine(__flush_mappings, &f, NULL);
+	}
+
+}
+
+void fixup_init(void)
+{
+	adjust_mem((unsigned long)__init_begin, (unsigned long)__init_end,
+			__pa(__init_begin),
+			PROT_SECT_NORMAL,
+			PAGE_KERNEL);
+}
+
+void adjust_exec_mem(unsigned long start, unsigned long end)
+{
+	adjust_mem(start, end, __pa(start),
+			PROT_SECT_NORMAL_EXEC,
+			PAGE_KERNEL_EXEC);
+}
+
 /*
  * paging_init() sets up the page tables, initialises the zone memory
  * maps and sets up the zero page.
@@ -349,6 +575,7 @@ void __init paging_init(void)
 	void *zero_page;
 
 	map_mem();
+	fixup_executable();
 
 	/*
 	 * Finally flush the caches and tlb to ensure that we're in a
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCHv6] arm64: add better page protections to arm64

[Catalin Marinas]

On Fri, Nov 21, 2014 at 09:50:45PM +0000, Laura Abbott wrote:
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -26,6 +26,7 @@
>  #include <linux/memblock.h>
>  #include <linux/fs.h>
>  #include <linux/io.h>
> +#include <linux/stop_machine.h>
> 
>  #include <asm/cputype.h>
>  #include <asm/fixmap.h>
> @@ -137,17 +138,50 @@ static void __init *early_alloc(unsigned long sz)
>         return ptr;
>  }
> 
> -static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr,
> +/*
> + * remap a PMD into pages
> + */
> +static noinline void split_pmd(pmd_t *pmd,
> +                               void *(*alloc)(unsigned long size))

Please drop the inline/noinline annotations, just let the compiler do
the work.

> +{
> +       pte_t *pte, *start_pte;
> +       unsigned long pfn;
> +       int i = 0;
> +
> +       start_pte = pte = alloc(PTRS_PER_PTE*sizeof(pte_t));
> +       BUG_ON(!pte);
> +
> +       pfn = pmd_pfn(*pmd);
> +
> +       do {
> +               /*
> +                * Need to have the least restrictive permissions available
> +                * permissions will be fixed up later
> +                */
> +               set_pte(pte, pfn_pte(pfn, PAGE_KERNEL_EXEC));
> +               pfn++;
> +       } while (pte++, i++, i < PTRS_PER_PTE);
> +
> +
> +       __pmd_populate(pmd, __pa(start_pte), PMD_TYPE_TABLE);
> +       flush_tlb_all();
> +}

For consistency, can we not have split_pmd similar to split_pud (i.e.
avoid allocation in split_pmd, just populate with the given pmd)?

> +
> +static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>                                   unsigned long end, unsigned long pfn,
> -                                 pgprot_t prot)
> +                                 pgprot_t prot,
> +                                 void *(*alloc)(unsigned long size))
>  {
>         pte_t *pte;
> 
>         if (pmd_none(*pmd)) {
> -               pte = early_alloc(PTRS_PER_PTE * sizeof(pte_t));
> +               pte = alloc(PTRS_PER_PTE * sizeof(pte_t));
> +               BUG_ON(!pte);

I wonder whether we should put the BUG_ON in the alloc functions to keep
the code simpler.

>                 __pmd_populate(pmd, __pa(pte), PMD_TYPE_TABLE);
>         }
> -       BUG_ON(pmd_bad(*pmd));
> +
> +       if (pmd_bad(*pmd))
> +               split_pmd(pmd, alloc);

I would use pmd_sect(*pmd) instead, it gives an indication on why it
needs splitting.

> +static inline bool use_1G_block(unsigned long addr, unsigned long next,
> +                       unsigned long phys, pgprot_t sect_prot,
> +                       pgprot_t pte_prot)
> +{
> +       if (PAGE_SHIFT != 12)
> +               return false;
> +
> +       if (((addr | next | phys) & ~PUD_MASK) != 0)
> +               return false;
> +
> +       /*
> +        * The assumption here is that if the memory is anything other
> +        * than normal we should not be using a block type
> +        */

Why? I know the original code had a !map_io check but I don't remember
why we actually need this limitation.

> +#ifdef CONFIG_DEBUG_RODATA
> +static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
> +{
> +       /*
> +        * Set up the executable regions using the existing section mappings
> +        * for now. This will get more fine grained later once all memory
> +        * is mapped
> +        */
> +       unsigned long kernel_x_start = round_down(__pa(_stext), SECTION_SIZE);
> +       unsigned long kernel_x_end = round_up(__pa(__init_end), SECTION_SIZE);
> +
> +       if (end < kernel_x_start) {
> +               create_mapping(start, __phys_to_virt(start),
> +                       end - start, PROT_SECT_NORMAL, PAGE_KERNEL);
> +       } else if (start >= kernel_x_end) {
> +               create_mapping(start, __phys_to_virt(start),
> +                       end - start, PROT_SECT_NORMAL, PAGE_KERNEL);
> +       } else {
> +               if (start < kernel_x_start)
> +                       create_mapping(start, __phys_to_virt(start),
> +                               kernel_x_start - start,
> +                               PROT_SECT_NORMAL,
> +                               PAGE_KERNEL);
> +               create_mapping(kernel_x_start,
> +                               __phys_to_virt(kernel_x_start),
> +                               kernel_x_end - kernel_x_start,
> +                               PROT_SECT_NORMAL_EXEC, PAGE_KERNEL_EXEC);
> +               if (kernel_x_end < end)
> +                       create_mapping(kernel_x_end,
> +                               __phys_to_virt(kernel_x_end),
> +                               end - kernel_x_end,
> +                               PROT_SECT_NORMAL,
> +                               PAGE_KERNEL);
> +       }

Do we care about the init section? It will get freed eventually (though
I guess we could spot early bugs modifying such code).

Anyway, can you not do something similar here for the rest of the kernel
text to avoid re-adjusting the page attributes later?

> +struct flush_addr {
> +       unsigned long start;
> +       unsigned long end;
> +};
> +
> +static int __flush_mappings(void *val)
> +{
> +       struct flush_addr *data = val;
> +
> +       flush_tlb_kernel_range(data->start, data->end);
> +       return 0;
> +}
> +
> +static void adjust_mem(unsigned long vstart, unsigned long vend,
> +                       phys_addr_t phys,
> +                       pgprot_t sect_prot, pgprot_t pte_prot)
> +{
> +       struct flush_addr f;
> +
> +       create_mapping_late(phys, vstart, vend - vstart,
> +                       sect_prot, pte_prot);
> +
> +       if (!IS_ALIGNED(vstart, SECTION_SIZE) || !IS_ALIGNED(vend, SECTION_SIZE)) {
> +               f.start = vstart;
> +               f.end = vend;
> +               stop_machine(__flush_mappings, &f, NULL);
> +       }

Maybe this was discussed on earlier versions of this patch but why do
you need stop_machine() here? A TLB flush would be broadcast by hardware
automatically to all the other CPUs.

I need to go again through this patch and see if there is a better
option than overloading the alloc_init_*() functions for adjusting ptes
(or maybe avoid adjusting altogether if we create them with the right
attributes from the beginning).

-- 
Catalin

[PATCHv6] arm64: add better page protections to arm64

[Laura Abbott]

Reviving this because I finally got the time to look at it again

On 12/2/2014 10:28 AM, Catalin Marinas wrote:
> On Fri, Nov 21, 2014 at 09:50:45PM +0000, Laura Abbott wrote:
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -26,6 +26,7 @@
>>   #include <linux/memblock.h>
>>   #include <linux/fs.h>
>>   #include <linux/io.h>
>> +#include <linux/stop_machine.h>
>>
>>   #include <asm/cputype.h>
>>   #include <asm/fixmap.h>
>> @@ -137,17 +138,50 @@ static void __init *early_alloc(unsigned long sz)
>>          return ptr;
>>   }
>>
>> -static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr,
>> +/*
>> + * remap a PMD into pages
>> + */
>> +static noinline void split_pmd(pmd_t *pmd,
>> +                               void *(*alloc)(unsigned long size))
>
> Please drop the inline/noinline annotations, just let the compiler do
> the work.
>

Yes, this was left over from debugging.

>> +{
>> +       pte_t *pte, *start_pte;
>> +       unsigned long pfn;
>> +       int i = 0;
>> +
>> +       start_pte = pte = alloc(PTRS_PER_PTE*sizeof(pte_t));
>> +       BUG_ON(!pte);
>> +
>> +       pfn = pmd_pfn(*pmd);
>> +
>> +       do {
>> +               /*
>> +                * Need to have the least restrictive permissions available
>> +                * permissions will be fixed up later
>> +                */
>> +               set_pte(pte, pfn_pte(pfn, PAGE_KERNEL_EXEC));
>> +               pfn++;
>> +       } while (pte++, i++, i < PTRS_PER_PTE);
>> +
>> +
>> +       __pmd_populate(pmd, __pa(start_pte), PMD_TYPE_TABLE);
>> +       flush_tlb_all();
>> +}
>
> For consistency, can we not have split_pmd similar to split_pud (i.e.
> avoid allocation in split_pmd, just populate with the given pmd)?
>

Sure

>> +
>> +static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>>                                    unsigned long end, unsigned long pfn,
>> -                                 pgprot_t prot)
>> +                                 pgprot_t prot,
>> +                                 void *(*alloc)(unsigned long size))
>>   {
>>          pte_t *pte;
>>
>>          if (pmd_none(*pmd)) {
>> -               pte = early_alloc(PTRS_PER_PTE * sizeof(pte_t));
>> +               pte = alloc(PTRS_PER_PTE * sizeof(pte_t));
>> +               BUG_ON(!pte);
>
> I wonder whether we should put the BUG_ON in the alloc functions to keep
> the code simpler.
>

Done

>>                  __pmd_populate(pmd, __pa(pte), PMD_TYPE_TABLE);
>>          }
>> -       BUG_ON(pmd_bad(*pmd));
>> +
>> +       if (pmd_bad(*pmd))
>> +               split_pmd(pmd, alloc);
>
> I would use pmd_sect(*pmd) instead, it gives an indication on why it
> needs splitting.
>

Done
  
>> +static inline bool use_1G_block(unsigned long addr, unsigned long next,
>> +                       unsigned long phys, pgprot_t sect_prot,
>> +                       pgprot_t pte_prot)
>> +{
>> +       if (PAGE_SHIFT != 12)
>> +               return false;
>> +
>> +       if (((addr | next | phys) & ~PUD_MASK) != 0)
>> +               return false;
>> +
>> +       /*
>> +        * The assumption here is that if the memory is anything other
>> +        * than normal we should not be using a block type
>> +        */
>
> Why? I know the original code had a !map_io check but I don't remember
> why we actually need this limitation.
>

I think this point becomes moot with the latest set of Ard's patches
which drop create_id_mapping. I'll drop the check for normal memory.
  
>> +#ifdef CONFIG_DEBUG_RODATA
>> +static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
>> +{
>> +       /*
>> +        * Set up the executable regions using the existing section mappings
>> +        * for now. This will get more fine grained later once all memory
>> +        * is mapped
>> +        */
>> +       unsigned long kernel_x_start = round_down(__pa(_stext), SECTION_SIZE);
>> +       unsigned long kernel_x_end = round_up(__pa(__init_end), SECTION_SIZE);
>> +
>> +       if (end < kernel_x_start) {
>> +               create_mapping(start, __phys_to_virt(start),
>> +                       end - start, PROT_SECT_NORMAL, PAGE_KERNEL);
>> +       } else if (start >= kernel_x_end) {
>> +               create_mapping(start, __phys_to_virt(start),
>> +                       end - start, PROT_SECT_NORMAL, PAGE_KERNEL);
>> +       } else {
>> +               if (start < kernel_x_start)
>> +                       create_mapping(start, __phys_to_virt(start),
>> +                               kernel_x_start - start,
>> +                               PROT_SECT_NORMAL,
>> +                               PAGE_KERNEL);
>> +               create_mapping(kernel_x_start,
>> +                               __phys_to_virt(kernel_x_start),
>> +                               kernel_x_end - kernel_x_start,
>> +                               PROT_SECT_NORMAL_EXEC, PAGE_KERNEL_EXEC);
>> +               if (kernel_x_end < end)
>> +                       create_mapping(kernel_x_end,
>> +                               __phys_to_virt(kernel_x_end),
>> +                               end - kernel_x_end,
>> +                               PROT_SECT_NORMAL,
>> +                               PAGE_KERNEL);
>> +       }
>
> Do we care about the init section? It will get freed eventually (though
> I guess we could spot early bugs modifying such code).
>
> Anyway, can you not do something similar here for the rest of the kernel
> text to avoid re-adjusting the page attributes later?
>

I was going for the strongest protections possible here and it matches
what arm32 does. I'm not quite sure I understand your second point
about "do something similar here"

>> +struct flush_addr {
>> +       unsigned long start;
>> +       unsigned long end;
>> +};
>> +
>> +static int __flush_mappings(void *val)
>> +{
>> +       struct flush_addr *data = val;
>> +
>> +       flush_tlb_kernel_range(data->start, data->end);
>> +       return 0;
>> +}
>> +
>> +static void adjust_mem(unsigned long vstart, unsigned long vend,
>> +                       phys_addr_t phys,
>> +                       pgprot_t sect_prot, pgprot_t pte_prot)
>> +{
>> +       struct flush_addr f;
>> +
>> +       create_mapping_late(phys, vstart, vend - vstart,
>> +                       sect_prot, pte_prot);
>> +
>> +       if (!IS_ALIGNED(vstart, SECTION_SIZE) || !IS_ALIGNED(vend, SECTION_SIZE)) {
>> +               f.start = vstart;
>> +               f.end = vend;
>> +               stop_machine(__flush_mappings, &f, NULL);
>> +       }
>
> Maybe this was discussed on earlier versions of this patch but why do
> you need stop_machine() here? A TLB flush would be broadcast by hardware
> automatically to all the other CPUs.
>

Yes, you are right there the broadcasting should take care of everything.
I'll drop it.

> I need to go again through this patch and see if there is a better
> option than overloading the alloc_init_*() functions for adjusting ptes
> (or maybe avoid adjusting altogether if we create them with the right
> attributes from the beginning).
>

If we round up everything to section size we can theoretically do things
in place although that breaks the existing expectation of DEBUG_RODATA
which expects the RO portion to happen in mark_rodata_ro. We run into
a chicken and egg problem if we need to go to the pte level since not
all memory is mapped yet.

I have another version of this based on Ard's series of stable UEFI
virtual mappings for kexec. I'll send it out sometime today.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCHv6 0/8] Better page protections for arm64

[Will Deacon]

On Fri, Nov 21, 2014 at 09:50:37PM +0000, Laura Abbott wrote:
> Hi,

Hi Laura,

> This is v6 of the series to add stricter page protections for arm64.
> The goal is to have text be RO/NX and everything else be RW/NX.
> The biggest change here is the addition of ioremap_exec for EFI
> services. The series did its job and caught EFI services trying to
> use regular RAM for code purposes.

I've taken patches 1-3 + 5 of this series. I appreciate that the `meat' is
in the later parts of the series, but I think there's still some review
needed there and it makes sense to take the independent parts first.

Cheers,

Will

[PATCHv6 0/8] Better page protections for arm64

[Kees Cook]

On Fri, Nov 21, 2014 at 1:50 PM, Laura Abbott <lauraa@codeaurora.org> wrote:
> Hi,
>
> This is v6 of the series to add stricter page protections for arm64.
> The goal is to have text be RO/NX and everything else be RW/NX.
> The biggest change here is the addition of ioremap_exec for EFI
> services. The series did its job and caught EFI services trying to
> use regular RAM for code purposes.
>
> Laura Abbott (8):
>   arm64: Treat handle_arch_irq as a function pointer
>   arm64: Switch to adrp for loading the stub vectors
>   arm64: Move cpu_resume into the text section
>   arm64: Move some head.text functions to executable section
>   arm64: Factor out fixmap initialiation from ioremap
>   arm64: use fixmap for text patching when text is RO
>   arm64: efi: Use ioremap_exec for code sections
>   arm64: add better page protections to arm64

Retesting this went fine. v6 continues to work well for my devices.

Thanks!

-Kees

>
>  arch/arm64/Kconfig.debug            |  23 ++
>  arch/arm64/include/asm/cacheflush.h |   7 +
>  arch/arm64/include/asm/fixmap.h     |   8 +-
>  arch/arm64/include/asm/insn.h       |   2 +
>  arch/arm64/include/asm/io.h         |   1 +
>  arch/arm64/include/asm/irq.h        |   1 -
>  arch/arm64/include/asm/pgtable.h    |   1 +
>  arch/arm64/kernel/efi.c             |  12 +-
>  arch/arm64/kernel/entry.S           |   6 +-
>  arch/arm64/kernel/head.S            | 410 +++++++++++++++++++-----------------
>  arch/arm64/kernel/insn.c            |  72 ++++++-
>  arch/arm64/kernel/irq.c             |   2 +
>  arch/arm64/kernel/jump_label.c      |   2 +-
>  arch/arm64/kernel/setup.c           |   1 +
>  arch/arm64/kernel/sleep.S           |  36 +---
>  arch/arm64/kernel/suspend.c         |   4 +-
>  arch/arm64/kernel/vmlinux.lds.S     |  18 +-
>  arch/arm64/mm/init.c                |   1 +
>  arch/arm64/mm/ioremap.c             |  99 ++-------
>  arch/arm64/mm/mm.h                  |   2 +
>  arch/arm64/mm/mmu.c                 | 405 +++++++++++++++++++++++++++++++----
>  21 files changed, 740 insertions(+), 373 deletions(-)
>
> --
> Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>



-- 
Kees Cook
Chrome OS Security