From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752531AbcEFHqz (ORCPT ); Fri, 6 May 2016 03:46:55 -0400 Received: from terminus.zytor.com ([198.137.202.10]:44340 "EHLO terminus.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751287AbcEFHqx (ORCPT ); Fri, 6 May 2016 03:46:53 -0400 Date: Fri, 6 May 2016 00:45:41 -0700 From: tip-bot for Kees Cook Message-ID: Cc: akpm@linux-foundation.org, luto@amacapital.net, bhe@redhat.com, mingo@kernel.org, vgoyal@redhat.com, yinghai@kernel.org, dvlasenk@redhat.com, peterz@infradead.org, linux-kernel@vger.kernel.org, brgerst@gmail.com, tglx@linutronix.de, keescook@chromium.org, luto@kernel.org, hpa@zytor.com, torvalds@linux-foundation.org, dyoung@redhat.com, bp@alien8.de Reply-To: torvalds@linux-foundation.org, hpa@zytor.com, dyoung@redhat.com, bp@alien8.de, tglx@linutronix.de, keescook@chromium.org, luto@kernel.org, peterz@infradead.org, dvlasenk@redhat.com, linux-kernel@vger.kernel.org, brgerst@gmail.com, luto@amacapital.net, akpm@linux-foundation.org, bhe@redhat.com, mingo@kernel.org, yinghai@kernel.org, vgoyal@redhat.com In-Reply-To: <1462486436-3707-2-git-send-email-keescook@chromium.org> References: <1462486436-3707-2-git-send-email-keescook@chromium.org> To: linux-tip-commits@vger.kernel.org Subject: [tip:x86/boot] x86/boot: Clean up pointer casting Git-Commit-ID: 2bc1cd39fa9f659956b25e500422e700a6cd4ec3 X-Mailer: tip-git-log-daemon Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit-ID: 2bc1cd39fa9f659956b25e500422e700a6cd4ec3 Gitweb: http://git.kernel.org/tip/2bc1cd39fa9f659956b25e500422e700a6cd4ec3 Author: Kees Cook AuthorDate: Thu, 5 May 2016 15:13:46 -0700 Committer: Ingo Molnar CommitDate: Fri, 6 May 2016 09:00:59 +0200 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 f1818d9..2072d82 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 9887e0d..1f23d02 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