From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752694AbbCOMRI (ORCPT ); Sun, 15 Mar 2015 08:17:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38491 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751019AbbCOMRE (ORCPT ); Sun, 15 Mar 2015 08:17:04 -0400 Date: Sun, 15 Mar 2015 20:18:45 +0800 From: Minfei Huang To: Yinghai Lu Cc: "H. Peter Anvin" , Ingo Molnar , Kees Cook , Borislav Petkov , Baoquan He , Thomas Gleixner , Jiri Kosina , Andrew Morton , Linus Torvalds , linux-kernel@vger.kernel.org, Matt Fleming Subject: Re: [PATCH v4] x86, kaslr: Access the correct kaslr_enabled variable Message-ID: <20150315121845.GA22920@dhcp-128-1.nay.redhat.com> References: <1426405767-21100-1-git-send-email-yinghai@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426405767-21100-1-git-send-email-yinghai@kernel.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/15/15 at 12:49am, Yinghai Lu wrote: > While trying to optimize setup_data handling code, found commit f47233c2d34f > ("x86/mm/ASLR: Propagate base load address calculation"), uses a physical > address as the value. > > It leads to wrong kaslr status. That kaslr status is passed for controlling > module load offset. So when kernel has kasl support compiled in, and user > use nokaslr to disable kaslr, wrong value cause unwanted offset for module > loading. > Hi, Yinghai! It confuses me with the virtual address in function parse_kaslr_setup. When we are in here(parse_kaslr_setup), we already use the virtual address, instead of physical address. Is it all right? In the other words, using physical address in parse_kaslr_setup is always a mistake, whatever the kaslr is on or off. Thanks Minfei > We should keep kaslr status consistent between aslr randrom kernel base, > and late module random offset. That is old behavior before that commit. > > The setup_data linked list and thus the element which contains > kaslr_enabled is chained together using physical addresses. At the time > when we access it in the kernel, we're already running with paging enabled > and therefore must access it through its virtual address. > > This patch changes the code to use early_memmap() and access the value, > as it is still in kernel early stage, we don't have kernel mapping setup > yet. > > Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation") > Cc: Matt Fleming > Cc: Borislav Petkov > Cc: Kees Cook > Cc: Jiri Kosina > Acked-by: Jiri Kosina > Signed-off-by: Yinghai Lu > > --- > -v3: add checking return from early_memmap according to Boris. > -v4: add description about setup_data accessing from Boris to change log. > > --- > arch/x86/kernel/setup.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > Index: linux-2.6/arch/x86/kernel/setup.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/setup.c > +++ linux-2.6/arch/x86/kernel/setup.c > @@ -429,7 +429,18 @@ static void __init reserve_initrd(void) > > static void __init parse_kaslr_setup(u64 pa_data, u32 data_len) > { > - kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); > + /* kaslr_setup_data is defined in aslr.c */ > + unsigned char *data; > + unsigned long offset = sizeof(struct setup_data); > + > + data = early_memremap(pa_data, offset + 1); > + if (!data) { > + kaslr_enabled = true; > + return; > + } > + > + kaslr_enabled = *(data + offset); > + early_memunmap(data, offset + 1); > } > > static void __init parse_setup_data(void) > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/