From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752827AbbCPDaX (ORCPT ); Sun, 15 Mar 2015 23:30:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59550 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750937AbbCPDaW (ORCPT ); Sun, 15 Mar 2015 23:30:22 -0400 Date: Mon, 16 Mar 2015 11:28:10 +0800 From: Baoquan He To: Yinghai Lu Cc: "H. Peter Anvin" , Ingo Molnar , Kees Cook , Borislav Petkov , 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: <20150316032810.GD26587@dhcp-16-105.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: > 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) { It's good to check the ret value as Boris suggested. However it could fail since early_memremap self fail, e.g slot not found. In this case making kaslr_enabled true may not be good. As Minfei talked with you kaslr_setup_data is a global variable inside kernel code, it has been ident mapped. Just derefencing the physical address which is virtual address too and getting the real stored value may be safer. And also parse_kaslr_setup is a function specified to handle kaslr, it doesn't make me uncomfortable to implement with a specific knowledge which here means the setup_data is a global varialbe in kernel code and no need to do early_memremap since mapping has been built . Thanks Baoquan > + kaslr_enabled = true; > + return; > + } > + > + kaslr_enabled = *(data + offset); > + early_memunmap(data, offset + 1); > } > > static void __init parse_setup_data(void)