From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751486AbdFXOXt (ORCPT ); Sat, 24 Jun 2017 10:23:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49362 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751055AbdFXOXs (ORCPT ); Sat, 24 Jun 2017 10:23:48 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5713C13A60 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=bhe@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 5713C13A60 Date: Sat, 24 Jun 2017 22:23:44 +0800 From: Baoquan He To: linux-kernel@vger.kernel.org Subject: Re: [PATCH] x86/boot/KASLR: Skip relocation handling in no kaslr case Message-ID: <20170624142344.GB2412@x1> References: <1498313793-18278-1-git-send-email-bhe@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1498313793-18278-1-git-send-email-bhe@redhat.com> User-Agent: Mutt/1.7.0 (2016-08-17) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Sat, 24 Jun 2017 14:23:48 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sorry, forgot adding Cc: before maintainers contact, NACK this patch and will repost. On 06/24/17 at 10:16pm, Baoquan He wrote: > Kdump kernel will reset to firmware after crash is trigered when > crashkernel=xxM,high is added to kernel command line. Kexec has the > same phenomenon. This only happened on system with kaslr code > compiled in and kernel option 'nokaslr'is added. Both of them works > well when kaslr is enabled. > > When crashkernel high is set or kexec case, kexec/kdump kernel will be > put above 4G. Since we assign the original loading address of kernel to > virt_addr as initial value, the virt_addr will be larger than 1G if kaslr > is disabled, it exceeds the kernel mapping size which is only 1G. Then > it will cause relocation handling error in handle_relocations(). > > In fact this is a known issue and fixed in commit: > > ... f285f4a ("x86, boot: Skip relocs when load address unchanged") > > But above fix was lost carelessly in later commit: > > ... 8391c73 ("x86/KASLR: Randomize virtual address separately") > > To fix it, just assign LOAD_PHYSICAL_ADDR to virt_addr as initial value. > If no kaslr is taken, it will skip the relocation handling and jump out. > > Fixes: 8391c73 ("x86/KASLR: Randomize virtual address separately") > Tested-by: Dave Young > Signed-off-by: Baoquan He > "H. Peter Anvin" > Thomas Gleixner > Ingo Molnar > x86@kernel.org > Kees Cook Baoquan He > Dave Jiang > Yinghai Lu > Arnd Bergmann > Thomas Garnier > --- > arch/x86/boot/compressed/kaslr.c | 3 --- > arch/x86/boot/compressed/misc.c | 4 ++-- > arch/x86/boot/compressed/misc.h | 2 -- > 3 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c > index fe318b4..91f27ab 100644 > --- a/arch/x86/boot/compressed/kaslr.c > +++ b/arch/x86/boot/compressed/kaslr.c > @@ -625,9 +625,6 @@ void choose_random_location(unsigned long input, > { > unsigned long random_addr, min_addr; > > - /* By default, keep output position unchanged. */ > - *virt_addr = *output; > - > if (cmdline_find_option_bool("nokaslr")) { > warn("KASLR disabled: 'nokaslr' on cmdline."); > return; > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c > index b3c5a5f0..c945acd 100644 > --- a/arch/x86/boot/compressed/misc.c > +++ b/arch/x86/boot/compressed/misc.c > @@ -338,7 +338,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap, > unsigned long output_len) > { > const unsigned long kernel_total_size = VO__end - VO__text; > - unsigned long virt_addr = (unsigned long)output; > + unsigned long virt_addr = LOAD_PHYSICAL_ADDR; > > /* Retain x86 boot parameters pointer passed from startup_32/64. */ > boot_params = rmode; > @@ -397,7 +397,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap, > #ifndef CONFIG_RELOCATABLE > if ((unsigned long)output != LOAD_PHYSICAL_ADDR) > error("Destination address does not match LOAD_PHYSICAL_ADDR"); > - if ((unsigned long)output != virt_addr) > + if (virt_addr != LOAD_PHYSICAL_ADDR) > error("Destination virtual address changed when not relocatable"); > #endif > > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h > index 1c8355e..766a521 100644 > --- a/arch/x86/boot/compressed/misc.h > +++ b/arch/x86/boot/compressed/misc.h > @@ -81,8 +81,6 @@ static inline void choose_random_location(unsigned long input, > unsigned long output_size, > unsigned long *virt_addr) > { > - /* No change from existing output location. */ > - *virt_addr = *output; > } > #endif > > -- > 2.5.5 >