[PATCH 0/3] Make kexec work with CONFIG_DEBUG_RODATA enabled

[PATCH 0/3] Make kexec work with CONFIG_DEBUG_RODATA enabled

[Nikolay Borisov]

This patch makes kexec work on a kernel that has its .text section set to read
only. The main reason of doing it is because there is a patch in the making 
(http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/244779.html)
which aims to make it possible to flag the kernel's code section as read-only,
rendering kexec inoperable.

The first patch does the actual functional changes while the 2nd one is
optional and is only to be used in case you have already applied the aforementioned
CONFIG_DEBUG_RODATA patch

Patch 3 is also optional and tries to make the assembly code a bit more "sane"
by introducing a .struct to describe the parameters for the kernel relocation
stub. 

Those patches have been tested on linux 3.14 on THUMB-2/non-THUMB-2 host
kernels. 

Patch 1 and 3 apply cleanly on 3.15-rc3 and Patch 2 applies cleanly, provided
that Kees' patch is applied as well. 

Kees also managed to test that on a QEMU 2.0 based machine. 

Nikolay Borisov (3):
  ARM: kexec: Make kexec work with read-only kernel .text section
  ARM: Kconfig: remove dependence of CONFIG_DEBUG_RODATA on !kexec
  ARM: kexec: Use .struct to describe relocate_new_kernel parameter
    layout

 arch/arm/kernel/machine_kexec.c   | 54 ++++++++++++++++++++++++++++-----------
 arch/arm/kernel/relocate_kernel.S | 47 +++++++++++++++++-----------------
 arch/arm/mm/Kconfig               |  2 +-
 3 files changed, 63 insertions(+), 40 deletions(-)

-- 
1.8.1.5

[PATCH 1/3] ARM: kexec: Make kexec work with read-only kernel .text section

[Nikolay Borisov]

Currently, as part of preparing the machine to boot a new kernel, Arm's kexec
backend will patch some memory location which are allocated in the kernel's
.text section. This will work only if the aforementioned section is not set
to read-only, otherwise an OOPS will be produced  and kexec won't work.

This patch modifies the way those memory location are being patched by first
copying them from the .text section (the template) to a special buffer and
then patching the values in the already-copied buffer, thus obviating the
need to touch the kernel's code section.

Finally, given the fact that code now works irrespective whether the kexec stub
is RO or not just move it to .rodata

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
Tested-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/kernel/machine_kexec.c   | 58 +++++++++++++++++++++++++++++----------
 arch/arm/kernel/relocate_kernel.S |  3 ++
 2 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index f0d180d..ee55e2e 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -22,13 +22,15 @@
 extern void relocate_new_kernel(void);
 extern const unsigned int relocate_new_kernel_size;
 
-extern unsigned long kexec_start_address;
-extern unsigned long kexec_indirection_page;
-extern unsigned long kexec_mach_type;
-extern unsigned long kexec_boot_atags;
+extern const unsigned long kexec_start_address;
+extern const unsigned long kexec_indirection_page;
+extern const unsigned long kexec_mach_type;
+extern const unsigned long kexec_boot_atags;
 
 static atomic_t waiting_for_crash_ipi;
 
+static unsigned long dt_mem;
+
 /*
  * Provide a dummy crash_notes definition while crash dump arrives to arm.
  * This prevents breakage of crash_notes attribute in kernel/ksysfs.c.
@@ -63,8 +65,13 @@ int machine_kexec_prepare(struct kimage *image)
 		if (err)
 			return err;
 
+		/*
+		 * We used to write directly to kexec_atag_boots
+		 * this is forbidden when .text section is RO, cache it
+		 * in a static var instead
+		 */
 		if (be32_to_cpu(header) == OF_DT_HEADER)
-			kexec_boot_atags = current_segment->mem;
+			dt_mem = current_segment->mem;
 	}
 	return 0;
 }
@@ -139,9 +146,36 @@ void machine_crash_shutdown(struct pt_regs *regs)
  */
 void (*kexec_reinit)(void);
 
+/*
+ * Instead of patching the kernel .text (which might be Read-only by
+ * CONFIG_DEBUG_RODATA), patch the already-copied template.
+ */
+static void patch_boot_parameters(char *copied_template, struct kimage *image) {
+
+	unsigned long page_list = image->head & PAGE_MASK;
+	uintptr_t base = (uintptr_t)relocate_new_kernel & ~(uintptr_t)1;
+	uintptr_t start_addr_offset = (uintptr_t)&kexec_start_address - base;
+	uintptr_t indir_page_offset = (uintptr_t)&kexec_indirection_page - base;
+	uintptr_t mach_type_offset = (uintptr_t)&kexec_mach_type - base;
+	uintptr_t boot_atags_offset = (uintptr_t)&kexec_boot_atags - base;
+
+#define patch_value(offset,res) \
+	*(unsigned long *)(copied_template + (offset)) = (res)
+
+	patch_value(start_addr_offset, image->start);
+	patch_value(indir_page_offset, page_list);
+	patch_value(mach_type_offset, machine_arch_type);
+
+	if (!dt_mem)
+		patch_value(boot_atags_offset, image->start -
+				 KEXEC_ARM_ZIMAGE_OFFSET +
+				 KEXEC_ARM_ATAGS_OFFSET);
+	else
+		patch_value(boot_atags_offset, dt_mem);
+}
+
 void machine_kexec(struct kimage *image)
 {
-	unsigned long page_list;
 	unsigned long reboot_code_buffer_phys;
 	unsigned long reboot_entry = (unsigned long)relocate_new_kernel;
 	unsigned long reboot_entry_phys;
@@ -155,21 +189,12 @@ void machine_kexec(struct kimage *image)
 	 */
 	BUG_ON(num_online_cpus() > 1);
 
-	page_list = image->head & PAGE_MASK;
 
 	/* we need both effective and real address here */
 	reboot_code_buffer_phys =
 	    page_to_pfn(image->control_code_page) << PAGE_SHIFT;
 	reboot_code_buffer = page_address(image->control_code_page);
 
-	/* Prepare parameters for reboot_code_buffer*/
-	kexec_start_address = image->start;
-	kexec_indirection_page = page_list;
-	kexec_mach_type = machine_arch_type;
-	if (!kexec_boot_atags)
-		kexec_boot_atags = image->start - KEXEC_ARM_ZIMAGE_OFFSET + KEXEC_ARM_ATAGS_OFFSET;
-
-
 	/* copy our kernel relocation code to the control code page */
 	reboot_entry = fncpy(reboot_code_buffer,
 			     reboot_entry,
@@ -177,6 +202,9 @@ void machine_kexec(struct kimage *image)
 	reboot_entry_phys = (unsigned long)reboot_entry +
 		(reboot_code_buffer_phys - (unsigned long)reboot_code_buffer);
 
+	/* Prepare parameters for reboot_code_buffer*/
+	patch_boot_parameters(reboot_code_buffer, image);
+
 	printk(KERN_INFO "Bye!\n");
 
 	if (kexec_reinit)
diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S
index 9585896..a8eb0e0 100644
--- a/arch/arm/kernel/relocate_kernel.S
+++ b/arch/arm/kernel/relocate_kernel.S
@@ -5,6 +5,9 @@
 #include <linux/linkage.h>
 #include <asm/kexec.h>
 
+/* Exectution of this template in place is a bug: make it non-executable: */
+	.section .rodata, "a"
+
 	.align	3	/* not needed for this code, but keeps fncpy() happy */
 
 ENTRY(relocate_new_kernel)
-- 
1.8.1.5

[PATCH 2/3] ARM: Kconfig: remove dependence of CONFIG_DEBUG_RODATA on !kexec

[Nikolay Borisov]

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
---
 arch/arm/mm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 3c7adea..ed58326 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -970,7 +970,7 @@ config ARM_KERNMEM_PERMS
 
 config DEBUG_RODATA
 	bool "Make kernel text and rodata read-only"
-	depends on ARM_KERNMEM_PERMS && KEXEC=n && KPROBES=n
+	depends on ARM_KERNMEM_PERMS && KPROBES=n
 	default y
 	help
 	  If this is set, kernel text and rodata will be made read-only.
-- 
1.8.1.5

[PATCH 3/3] ARM: kexec: Use .struct to describe relocate_new_kernel parameter layout

[Nikolay Borisov]

Since we no longer require poking into the kernel's section there is no need to
allocate space for 4 longs (16 bytes). So use a .struct directive which would
just place the labels at the correct offset but won't allocate space in the
kernel for them (saves 16 bytes!)

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
---
 arch/arm/kernel/machine_kexec.c   | 26 ++++++++++-------------
 arch/arm/kernel/relocate_kernel.S | 44 ++++++++++++++++++---------------------
 2 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index ee55e2e..7c0e4a2 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -22,10 +22,10 @@
 extern void relocate_new_kernel(void);
 extern const unsigned int relocate_new_kernel_size;
 
-extern const unsigned long kexec_start_address;
-extern const unsigned long kexec_indirection_page;
-extern const unsigned long kexec_mach_type;
-extern const unsigned long kexec_boot_atags;
+extern const unsigned long kexec_start_address_offset;
+extern const unsigned long kexec_indirection_page_offset;
+extern const unsigned long kexec_mach_type_offset;
+extern const unsigned long kexec_boot_atags_offset;
 
 static atomic_t waiting_for_crash_ipi;
 
@@ -153,25 +153,21 @@ void (*kexec_reinit)(void);
 static void patch_boot_parameters(char *copied_template, struct kimage *image) {
 
 	unsigned long page_list = image->head & PAGE_MASK;
-	uintptr_t base = (uintptr_t)relocate_new_kernel & ~(uintptr_t)1;
-	uintptr_t start_addr_offset = (uintptr_t)&kexec_start_address - base;
-	uintptr_t indir_page_offset = (uintptr_t)&kexec_indirection_page - base;
-	uintptr_t mach_type_offset = (uintptr_t)&kexec_mach_type - base;
-	uintptr_t boot_atags_offset = (uintptr_t)&kexec_boot_atags - base;
 
+#define get_offset(sym) ((uintptr_t)&(sym) + relocate_new_kernel_size)
 #define patch_value(offset,res) \
-	*(unsigned long *)(copied_template + (offset)) = (res)
+	*(unsigned long *)(copied_template + get_offset(offset)) = (res)
 
-	patch_value(start_addr_offset, image->start);
-	patch_value(indir_page_offset, page_list);
-	patch_value(mach_type_offset, machine_arch_type);
+	patch_value(kexec_start_address_offset, image->start);
+	patch_value(kexec_indirection_page_offset, page_list);
+	patch_value(kexec_mach_type_offset, machine_arch_type);
 
 	if (!dt_mem)
-		patch_value(boot_atags_offset, image->start -
+		patch_value(kexec_boot_atags_offset, image->start -
 				 KEXEC_ARM_ZIMAGE_OFFSET +
 				 KEXEC_ARM_ATAGS_OFFSET);
 	else
-		patch_value(boot_atags_offset, dt_mem);
+		patch_value(kexec_boot_atags_offset, dt_mem);
 }
 
 void machine_kexec(struct kimage *image)
diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S
index a8eb0e0..fbb22a7 100644
--- a/arch/arm/kernel/relocate_kernel.S
+++ b/arch/arm/kernel/relocate_kernel.S
@@ -12,8 +12,8 @@
 
 ENTRY(relocate_new_kernel)
 
-	ldr	r0,kexec_indirection_page
-	ldr	r1,kexec_start_address
+	ldr	r0,kexec_indirection_page_offset + relocate_new_kernel_size
+	ldr	r1,kexec_start_address_offset + relocate_new_kernel_size
 
 	/*
 	 * If there is no indirection page (we are doing crashdumps)
@@ -60,36 +60,32 @@ ENTRY(relocate_new_kernel)
 	/* Jump to relocated kernel */
 	mov lr,r1
 	mov r0,#0
-	ldr r1,kexec_mach_type
-	ldr r2,kexec_boot_atags
+	ldr r1,kexec_mach_type_offset + relocate_new_kernel_size
+	ldr r2,kexec_boot_atags_offset + relocate_new_kernel_size
  ARM(	mov pc, lr	)
  THUMB(	bx lr		)
 
-	.align
-
-	.globl kexec_start_address
-kexec_start_address:
-	.long	0x0
-
-	.globl kexec_indirection_page
-kexec_indirection_page:
-	.long	0x0
-
-	.globl kexec_mach_type
-kexec_mach_type:
-	.long	0x0
-
-	/* phy addr of the atags for the new kernel */
-	.globl kexec_boot_atags
-kexec_boot_atags:
-	.long	0x0
 
 ENDPROC(relocate_new_kernel)
 
-relocate_new_kernel_end:
+
 
 	.globl relocate_new_kernel_size
 relocate_new_kernel_size:
-	.long relocate_new_kernel_end - relocate_new_kernel
+	.long . - relocate_new_kernel
+
+	.align
+	
+	.struct 4 /* offsets are from relocate_new_kernel_size */
+ 
+	.macro parameter name /* declare space for a word-sized parameter */
+	.globl \name
+	\name:	.long 0
+	.endm
+ 
+parameter kexec_start_address_offset
+parameter kexec_indirection_page_offset
+parameter kexec_mach_type_offset
+parameter kexec_boot_atags_offset
 
 
-- 
1.8.1.5

[PATCH 0/3] Make kexec work with CONFIG_DEBUG_RODATA enabled

[Will Deacon]

Hi Nikolay,

On Mon, Apr 28, 2014 at 10:31:44AM +0100, Nikolay Borisov wrote:
> This patch makes kexec work on a kernel that has its .text section set to read
> only. The main reason of doing it is because there is a patch in the making 
> (http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/244779.html)
> which aims to make it possible to flag the kernel's code section as read-only,
> rendering kexec inoperable.
> 
> The first patch does the actual functional changes while the 2nd one is
> optional and is only to be used in case you have already applied the aforementioned
> CONFIG_DEBUG_RODATA patch
> 
> Patch 3 is also optional and tries to make the assembly code a bit more "sane"
> by introducing a .struct to describe the parameters for the kernel relocation
> stub. 
> 
> Those patches have been tested on linux 3.14 on THUMB-2/non-THUMB-2 host
> kernels. 
> 
> Patch 1 and 3 apply cleanly on 3.15-rc3 and Patch 2 applies cleanly, provided
> that Kees' patch is applied as well. 
> 
> Kees also managed to test that on a QEMU 2.0 based machine. 

Wouldn't it be far simpler to set_kernel_text_rw() on a kexec? At this
point, we've committed to replacing our image, so whether we can write to
the .text of the old image before we clobber it entirely seems somewhat
moot to me.

Will

[PATCH 0/3] Make kexec work with CONFIG_DEBUG_RODATA enabled

[Nikolay Borisov]

> From: Will Deacon [mailto:will.deacon at arm.com]
> Hi Nikolay,
>
> On Mon, Apr 28, 2014 at 10:31:44AM +0100, Nikolay Borisov wrote:
> > This patch makes kexec work on a kernel that has its .text section
> set to read
> > only. The main reason of doing it is because there is a patch in the
> making
> > (http://lists.infradead.org/pipermail/linux-arm-kernel/2014-
> April/244779.html)
> > which aims to make it possible to flag the kernel's code section as
> read-only,
> > rendering kexec inoperable.
> >
> > The first patch does the actual functional changes while the 2nd one
> is
> > optional and is only to be used in case you have already applied the
> aforementioned
> > CONFIG_DEBUG_RODATA patch
> >
> > Patch 3 is also optional and tries to make the assembly code a bit
> more "sane"
> > by introducing a .struct to describe the parameters for the kernel
> relocation
> > stub.
> >
> > Those patches have been tested on linux 3.14 on THUMB-2/non-THUMB-2
> host
> > kernels.
> >
> > Patch 1 and 3 apply cleanly on 3.15-rc3 and Patch 2 applies cleanly,
> provided
> > that Kees' patch is applied as well.
> >
> > Kees also managed to test that on a QEMU 2.0 based machine.
>
> Wouldn't it be far simpler to set_kernel_text_rw() on a kexec? At this
> point, we've committed to replacing our image, so whether we can write
> to the .text of the old image before we clobber it entirely seems somewhat
> moot to me.
>

We are poking one of the values in the prepare stage so doing set_kernel_text_rw() there
won't be very clean solution. However, I agree that if the values being poked into the prep
stage is stashed in a static variable and then in the kexec_execute function we make the kernel
RW since we are turning the MMO off and nuking the old kernel anyway.

> Will

Regards,
Nikolay

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

[PATCH 0/3] Make kexec work with CONFIG_DEBUG_RODATA enabled

[Kees Cook]

On Wed, Apr 30, 2014 at 6:19 AM, Nikolay Borisov
<Nikolay.Borisov@arm.com> wrote:
>> From: Will Deacon [mailto:will.deacon at arm.com]
>> Hi Nikolay,
>>
>> On Mon, Apr 28, 2014 at 10:31:44AM +0100, Nikolay Borisov wrote:
>> > This patch makes kexec work on a kernel that has its .text section
>> set to read
>> > only. The main reason of doing it is because there is a patch in the
>> making
>> > (http://lists.infradead.org/pipermail/linux-arm-kernel/2014-
>> April/244779.html)
>> > which aims to make it possible to flag the kernel's code section as
>> read-only,
>> > rendering kexec inoperable.
>> >
>> > The first patch does the actual functional changes while the 2nd one
>> is
>> > optional and is only to be used in case you have already applied the
>> aforementioned
>> > CONFIG_DEBUG_RODATA patch
>> >
>> > Patch 3 is also optional and tries to make the assembly code a bit
>> more "sane"
>> > by introducing a .struct to describe the parameters for the kernel
>> relocation
>> > stub.
>> >
>> > Those patches have been tested on linux 3.14 on THUMB-2/non-THUMB-2
>> host
>> > kernels.
>> >
>> > Patch 1 and 3 apply cleanly on 3.15-rc3 and Patch 2 applies cleanly,
>> provided
>> > that Kees' patch is applied as well.
>> >
>> > Kees also managed to test that on a QEMU 2.0 based machine.
>>
>> Wouldn't it be far simpler to set_kernel_text_rw() on a kexec? At this
>> point, we've committed to replacing our image, so whether we can write
>> to the .text of the old image before we clobber it entirely seems somewhat
>> moot to me.
>>
>
> We are poking one of the values in the prepare stage so doing set_kernel_text_rw() there
> won't be very clean solution. However, I agree that if the values being poked into the prep

set_kernel_text_rw() is also mm-specific, since the PMD sections are
copied per-mm. I'm not sure if that's as much of a problem here since
the prepare looks to be all in the same thread, but if we can avoid
set_kernel_text_rw() that'll probably make things less surprising if
changes are made later on.

-Kees

> stage is stashed in a static variable and then in the kexec_execute function we make the kernel
> RW since we are turning the MMO off and nuking the old kernel anyway.
>
>> Will
>
> Regards,
> Nikolay
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782
>



-- 
Kees Cook
Chrome OS Security