From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756555AbbCFTx1 (ORCPT ); Fri, 6 Mar 2015 14:53:27 -0500 Received: from mail-ig0-f180.google.com ([209.85.213.180]:36269 "EHLO mail-ig0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755288AbbCFTxX (ORCPT ); Fri, 6 Mar 2015 14:53:23 -0500 MIME-Version: 1.0 In-Reply-To: References: <1425456048-16236-1-git-send-email-yinghai@kernel.org> <1425456048-16236-5-git-send-email-yinghai@kernel.org> <20150304101649.GA3663@pd.tnic> <20150304200000.GB6276@gmail.com> <20150306133312.GC4259@pd.tnic> Date: Fri, 6 Mar 2015 11:53:22 -0800 X-Google-Sender-Auth: 76uRWML-jVsFRjFlTeunjuVTR2I Message-ID: Subject: Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly From: Yinghai Lu To: Borislav Petkov Cc: Ingo Molnar , Matt Fleming , "H. Peter Anvin" , Bjorn Helgaas , Thomas Gleixner , Ingo Molnar , Jiri Kosina , Baoquan He , Linux Kernel Mailing List , "linux-efi@vger.kernel.org" , "linux-pci@vger.kernel.org" , Kees Cook Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 6, 2015 at 11:50 AM, Yinghai Lu wrote: > On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov wrote: > >> >> "However, 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 proper, we're already running >> with paging enabled and therefore must access it through its virtual >> address." >> >> That's it, now how hard was to explain it that way? > > No, I don't think your change log is right. > > Actually the old code is using address as value. > > if the old code would be like: > > kaslr_enabled = (bool)(*(unsigned char *)(pa_data + sizeof(struct setup_data))); > > then your change log would be good, but the old code is > > kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); Please check if you are ok with this: Subject: [PATCH v4] x86, kaslr: Access the correct kaslr_enabled variable commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation") started passing KASLR status to kernel proper, but it uses address as the vaule. That will get wrong value back for kaslr_enabled in kernel stage. 1. When kaslr is not enabled at boot/choose_kernel_location, if kaslr_enabled get set wrongly in setup.c, late in module.c::get_module_load_offset will return not wanted random module load offset. That change behavior when HIBERNATION is defined or nokaslr is passed. 2. When kaslr is enabled at boot/choose_kernel_location, if kaslr_enabled get cleared wrongly in setup.c, late in module.c::get_module_load_offset will not return wanted random module load offset. 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 proper, 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, and will keep boot and kernel consistent with kaslr. -v3: add checking return from early_memmap according to Boris. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yinghai Lu Subject: Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly Date: Fri, 6 Mar 2015 11:53:22 -0800 Message-ID: References: <1425456048-16236-1-git-send-email-yinghai@kernel.org> <1425456048-16236-5-git-send-email-yinghai@kernel.org> <20150304101649.GA3663@pd.tnic> <20150304200000.GB6276@gmail.com> <20150306133312.GC4259@pd.tnic> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Borislav Petkov Cc: Ingo Molnar , Matt Fleming , "H. Peter Anvin" , Bjorn Helgaas , Thomas Gleixner , Ingo Molnar , Jiri Kosina , Baoquan He , Linux Kernel Mailing List , "linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Kees Cook List-Id: linux-efi@vger.kernel.org On Fri, Mar 6, 2015 at 11:50 AM, Yinghai Lu wrote: > On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov wrote: > >> >> "However, 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 proper, we're already running >> with paging enabled and therefore must access it through its virtual >> address." >> >> That's it, now how hard was to explain it that way? > > No, I don't think your change log is right. > > Actually the old code is using address as value. > > if the old code would be like: > > kaslr_enabled = (bool)(*(unsigned char *)(pa_data + sizeof(struct setup_data))); > > then your change log would be good, but the old code is > > kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); Please check if you are ok with this: Subject: [PATCH v4] x86, kaslr: Access the correct kaslr_enabled variable commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation") started passing KASLR status to kernel proper, but it uses address as the vaule. That will get wrong value back for kaslr_enabled in kernel stage. 1. When kaslr is not enabled at boot/choose_kernel_location, if kaslr_enabled get set wrongly in setup.c, late in module.c::get_module_load_offset will return not wanted random module load offset. That change behavior when HIBERNATION is defined or nokaslr is passed. 2. When kaslr is enabled at boot/choose_kernel_location, if kaslr_enabled get cleared wrongly in setup.c, late in module.c::get_module_load_offset will not return wanted random module load offset. 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 proper, 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, and will keep boot and kernel consistent with kaslr. -v3: add checking return from early_memmap according to Boris.