x86, boot: skip relocs when load address unchanged

This is a reimplementation of Baoquan's "kaslr: check if kernel location is changed", which performs the check without needing to change the function declaration. This should have exactly the same effect, but I dropped Vivek's Ack and Thomas's Test, since it's technically a different patch.

On 64-bit, relocation is not required unless the load address gets changed. Without this, relocations do unexpected things when the kernel is above 4G.

Reported-by Baoquan He <[email protected]>

Signed-off-by Kees Cook <[email protected]>

Cc [email protected]

---
This is a reimplementation of Baoquan's "kaslr: check if kernel location is changed", which performs the check without needing to change the function declaration. This should have exactly the same effect, but I dropped Vivek's Ack and Thomas's Test, since it's technically a different patch.

---
 arch/x86/boot/compressed/misc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

arch/x86/boot/compressed/misc.c +8 -1

index dcc1c536cc21..a950864a64da 100644
@@ -373,6 +373,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 				  unsigned long output_len,
 				  unsigned long run_size)
 {
+	unsigned char *output_orig = output;
+
 	real_mode = rmode;
 
 	sanitize_boot_params(real_mode);

@@ -421,7 +423,12 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 	debug_putstr("\nDecompressing Linux... ");
 	decompress(input_data, input_len, NULL, NULL, output, NULL, error);
 	parse_elf(output);
-	handle_relocations(output, output_len);
+	/*
+	 * 32-bit always performs relocations. 64-bit relocations are only
+	 * needed if kASLR has chosen a different load address.
+	 */
+	if (!IS_ENABLED(CONFIG_X86_64) || output != output_orig)
+		handle_relocations(output, output_len);
 	debug_putstr("done.\nBooting the kernel.\n");
 	return output;
 }

Signature · 6 lines

1.9.1


-- 
Kees Cook
Chrome OS Security

This patch works for me. And good to see it's being merged. About the patch log, I would say relocations do unexpected things when the kernel is above 1G since randomization is done from 16M to 1G, namely CONFIG_RANDOMIZE_BASE_MAX_OFFSET. So above 1G kernel text mapping will step into kernel module mapping region.

Quoted 3 lines

On 64-bit, relocation is not required unless the load address gets changed. Without this, relocations do unexpected things when the kernel is above 4G.

This patch works for me. And good to see it's being merged. About the patch log, I would say relocations do unexpected things when the kernel is above 1G since randomization is done from 16M to 1G, namely CONFIG_RANDOMIZE_BASE_MAX_OFFSET. So above 1G kernel text mapping will step into kernel module mapping region.

BTW, I am working on separate randomization of kernel physical and virtual address , will post it. But it won't conflict with this because I don't think it can be accepted in a short time. Before that this patch truly fix the kexec/kdump bug when kaslr is compiled in.

Thanks
Baoquan

Quoted 44 lines

Reported-by Baoquan He <[email protected]>

Signed-off-by Kees Cook <[email protected]>

Cc [email protected]

---
This is a reimplementation of Baoquan's "kaslr: check if kernel location is changed", which performs the check without needing to change the function declaration. This should have exactly the same effect, but I dropped Vivek's Ack and Thomas's Test, since it's technically a different patch.

---
 arch/x86/boot/compressed/misc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

arch/x86/boot/compressed/misc.c +8 -2

index dcc1c536cc21..a950864a64da 100644
@@ -373,6 +373,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
                                unsigned long output_len,
                                unsigned long run_size)
 {
+     unsigned char *output_orig = output;
+
      real_mode = rmode;
 
      sanitize_boot_params(real_mode);

@@ -421,7 +423,12 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
      debug_putstr("\nDecompressing Linux... ");
      decompress(input_data, input_len, NULL, NULL, output, NULL, error);
      parse_elf(output);
-     handle_relocations(output, output_len);
+     /*
+      * 32-bit always performs relocations. 64-bit relocations are only
+      * needed if kASLR has chosen a different load address.
+      */
+     if (!IS_ENABLED(CONFIG_X86_64) || output != output_orig)
+             handle_relocations(output, output_len);
      debug_putstr("done.\nBooting the kernel.\n");
      return output;
 }
--

1.9.1

Quoted 3 lines

--
Kees Cook
Chrome OS Security

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Thanks for this patch, and good to see it in mainline!

This actually fixes the problem I reported here:
https://lkml.org/lkml/2014/12/1/15

I wish it to be backported into the Ubuntu Utopic kernel asap.

Quoted 5 lines

This patch works for me. And good to see it's being merged. About the patch log, I would say relocations do unexpected things when the kernel is above 1G since randomization is done from 16M to 1G, namely CONFIG_RANDOMIZE_BASE_MAX_OFFSET. So above 1G kernel text mapping will step into kernel module mapping region.

I'm just speculating, but is it the reason why I only get problems when I boot with kexec? Maybe it's only then when the kernel gets above 1G. Otherwise, the kernels boot properly when they are booted from GRUB.

Thanks for this patch, and good to see it in mainline!

This actually fixes the problem I reported here:
https://lkml.org/lkml/2014/12/1/15

I wish it to be backported into the Ubuntu Utopic kernel asap.