From 2bc1cd39fa9f659956b25e500422e700a6cd4ec3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 5 May 2016 15:13:46 -0700 Subject: [PATCH] x86/boot: Clean up pointer casting Currently extract_kernel() defines the input and output buffer pointers as "unsigned char *" since that's effectively what they are. It passes these to the decompressor routine and to the ELF parser, which both logically deal with buffer pointers too. There is some casting ("unsigned long") done to validate the numerical value of the pointers, but it is relatively limited. However, choose_random_location() operates almost exclusively on the numerical representation of these pointers, so it ended up carrying a lot of "unsigned long" casts. With the future physical/virtual split these casts were going to multiply, so this attempts to solve the problem by doing all the casting in choose_random_location()'s entry and return instead of through-out the code. Adjusts argument names to be more meaningful, and changes one us of "choice" to "output" to make the future physical/virtual split more clear (i.e. "choice" should be strictly a function return value and not used as an intermediate). Suggested-by: Ingo Molnar Signed-off-by: Kees Cook Cc: Andrew Morton Cc: Andy Lutomirski Cc: Andy Lutomirski Cc: Baoquan He Cc: Borislav Petkov Cc: Brian Gerst Cc: Dave Young Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Vivek Goyal Cc: Yinghai Lu Cc: kernel-hardening@lists.openwall.com Cc: lasse.collin@tukaani.org Link: http://lkml.kernel.org/r/1462486436-3707-2-git-send-email-keescook@chromium.org Signed-off-by: Ingo Molnar --- arch/x86/boot/compressed/kaslr.c | 20 ++++++++++++++------ arch/x86/boot/compressed/misc.h | 10 +++++----- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index f1818d95d726..2072d82c1911 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -305,12 +305,21 @@ static unsigned long find_random_addr(unsigned long minimum, return slots_fetch_random(); } -unsigned char *choose_random_location(unsigned char *input, +unsigned char *choose_random_location(unsigned char *input_ptr, unsigned long input_size, - unsigned char *output, + unsigned char *output_ptr, unsigned long output_size) { - unsigned long choice = (unsigned long)output; + /* + * The caller of choose_random_location() uses unsigned char * for + * buffer pointers since it performs decompression, elf parsing, etc. + * Since this code examines addresses much more numerically, + * unsigned long is used internally here. Instead of sprinkling + * more casts into extract_kernel, do them here and at return. + */ + unsigned long input = (unsigned long)input_ptr; + unsigned long output = (unsigned long)output_ptr; + unsigned long choice = output; unsigned long random_addr; #ifdef CONFIG_HIBERNATION @@ -328,11 +337,10 @@ unsigned char *choose_random_location(unsigned char *input, boot_params->hdr.loadflags |= KASLR_FLAG; /* Record the various known unsafe memory ranges. */ - mem_avoid_init((unsigned long)input, input_size, - (unsigned long)output, output_size); + mem_avoid_init(input, input_size, output, output_size); /* Walk e820 and find a random address. */ - random_addr = find_random_addr(choice, output_size); + random_addr = find_random_addr(output, output_size); if (!random_addr) { warn("KASLR disabled: could not find suitable E820 region!"); goto out; diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index 9887e0d4aaeb..1f23d022d241 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -67,20 +67,20 @@ int cmdline_find_option_bool(const char *option); #if CONFIG_RANDOMIZE_BASE /* kaslr.c */ -unsigned char *choose_random_location(unsigned char *input, +unsigned char *choose_random_location(unsigned char *input_ptr, unsigned long input_size, - unsigned char *output, + unsigned char *output_ptr, unsigned long output_size); /* cpuflags.c */ bool has_cpuflag(int flag); #else static inline -unsigned char *choose_random_location(unsigned char *input, +unsigned char *choose_random_location(unsigned char *input_ptr, unsigned long input_size, - unsigned char *output, + unsigned char *output_ptr, unsigned long output_size) { - return output; + return output_ptr; } #endif -- 2.20.1