From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH 3/5] efi arm64: do not enter virtual mode in case booting with efi=noruntime or noefi Date: Tue, 12 Aug 2014 11:46:27 +0100 Message-ID: <20140812104627.GI29013@arm.com> References: <1407823822-23829-1-git-send-email-dyoung@redhat.com> <1407823822-23829-3-git-send-email-dyoung@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1407823822-23829-3-git-send-email-dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Dave Young Cc: Matt Fleming , Catalin Marinas , Thomas Gleixner , Ingo Molnar , "hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org" , Alessandro Zummo , Leif Lindholm , Ard Biesheuvel , "msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" , Randy Dunlap , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org" List-Id: linux-efi@vger.kernel.org On Tue, Aug 12, 2014 at 07:10:20AM +0100, Dave Young wrote: > In case efi runtime disabled via noefi kernel cmdline arm64_enter_virtual_mode > should error out. > > At the same time move early_memunmap(memmap.map, mapsize) to the beginning of > the function or it will leak early mem. > > Signed-off-by: Dave Young > --- > arch/arm64/kernel/efi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > index e72f310..324cdd1 100644 > --- a/arch/arm64/kernel/efi.c > +++ b/arch/arm64/kernel/efi.c > @@ -386,7 +386,10 @@ static int __init arm64_enter_virtual_mode(void) > int count = 0; > unsigned long flags; > > - if (!efi_enabled(EFI_BOOT)) { > + mapsize = memmap.map_end - memmap.map; > + early_memunmap(memmap.map, mapsize); > + > + if (!efi_enabled(EFI_BOOT) || efi_runtime_disabled()) { Hmm, is this right? We only set EFI_BOOT if we find EFI parameters in the DT (see efi_init -> uefi_init), so you run the risk of unmapping something we never mapped here. Furthermore, there seems to be a leak in uefi_init anyway, as we return -EINVAL if the signature of the EFI table doesn't match without unmapping it first. Will From mboxrd@z Thu Jan 1 00:00:00 1970 From: will.deacon@arm.com (Will Deacon) Date: Tue, 12 Aug 2014 11:46:27 +0100 Subject: [PATCH 3/5] efi arm64: do not enter virtual mode in case booting with efi=noruntime or noefi In-Reply-To: <1407823822-23829-3-git-send-email-dyoung@redhat.com> References: <1407823822-23829-1-git-send-email-dyoung@redhat.com> <1407823822-23829-3-git-send-email-dyoung@redhat.com> Message-ID: <20140812104627.GI29013@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Aug 12, 2014 at 07:10:20AM +0100, Dave Young wrote: > In case efi runtime disabled via noefi kernel cmdline arm64_enter_virtual_mode > should error out. > > At the same time move early_memunmap(memmap.map, mapsize) to the beginning of > the function or it will leak early mem. > > Signed-off-by: Dave Young > --- > arch/arm64/kernel/efi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > index e72f310..324cdd1 100644 > --- a/arch/arm64/kernel/efi.c > +++ b/arch/arm64/kernel/efi.c > @@ -386,7 +386,10 @@ static int __init arm64_enter_virtual_mode(void) > int count = 0; > unsigned long flags; > > - if (!efi_enabled(EFI_BOOT)) { > + mapsize = memmap.map_end - memmap.map; > + early_memunmap(memmap.map, mapsize); > + > + if (!efi_enabled(EFI_BOOT) || efi_runtime_disabled()) { Hmm, is this right? We only set EFI_BOOT if we find EFI parameters in the DT (see efi_init -> uefi_init), so you run the risk of unmapping something we never mapped here. Furthermore, there seems to be a leak in uefi_init anyway, as we return -EINVAL if the signature of the EFI table doesn't match without unmapping it first. Will From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751941AbaHLKrY (ORCPT ); Tue, 12 Aug 2014 06:47:24 -0400 Received: from cam-admin0.cambridge.arm.com ([217.140.96.50]:48078 "EHLO cam-admin0.cambridge.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750939AbaHLKrW (ORCPT ); Tue, 12 Aug 2014 06:47:22 -0400 Date: Tue, 12 Aug 2014 11:46:27 +0100 From: Will Deacon To: Dave Young Cc: Matt Fleming , Catalin Marinas , Thomas Gleixner , Ingo Molnar , "hpa@zytor.com" , Alessandro Zummo , Leif Lindholm , Ard Biesheuvel , "msalter@redhat.com" , Randy Dunlap , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-efi@vger.kernel.org" , "rtc-linux@googlegroups.com" Subject: Re: [PATCH 3/5] efi arm64: do not enter virtual mode in case booting with efi=noruntime or noefi Message-ID: <20140812104627.GI29013@arm.com> References: <1407823822-23829-1-git-send-email-dyoung@redhat.com> <1407823822-23829-3-git-send-email-dyoung@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1407823822-23829-3-git-send-email-dyoung@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 12, 2014 at 07:10:20AM +0100, Dave Young wrote: > In case efi runtime disabled via noefi kernel cmdline arm64_enter_virtual_mode > should error out. > > At the same time move early_memunmap(memmap.map, mapsize) to the beginning of > the function or it will leak early mem. > > Signed-off-by: Dave Young > --- > arch/arm64/kernel/efi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > index e72f310..324cdd1 100644 > --- a/arch/arm64/kernel/efi.c > +++ b/arch/arm64/kernel/efi.c > @@ -386,7 +386,10 @@ static int __init arm64_enter_virtual_mode(void) > int count = 0; > unsigned long flags; > > - if (!efi_enabled(EFI_BOOT)) { > + mapsize = memmap.map_end - memmap.map; > + early_memunmap(memmap.map, mapsize); > + > + if (!efi_enabled(EFI_BOOT) || efi_runtime_disabled()) { Hmm, is this right? We only set EFI_BOOT if we find EFI parameters in the DT (see efi_init -> uefi_init), so you run the risk of unmapping something we never mapped here. Furthermore, there seems to be a leak in uefi_init anyway, as we return -EINVAL if the signature of the EFI table doesn't match without unmapping it first. Will