lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 10 Aug 2018 17:34:04 +0000 From: "Prakhya, Sai Praneeth" <sai.praneeth.prakhya@...el.com> To: Bhupesh Sharma <bhsharma@...hat.com> CC: "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>, "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>, "ricardo.ner@...el.com" <ricardo.ner@...el.com>, Matt Fleming <matt@...eblueprint.co.uk>, Lee Chun-Yi <jlee@...e.com>, Al Stone <astone@...hat.com>, Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...nel.org>, Andy Lutomirski <luto@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Ard Biesheuvel <ard.biesheuvel@...aro.org> Subject: RE: [PATCH V1 6/6] x86/efi: Introduce EFI_WARN_ON_ILLEGAL_ACCESSES > > +config EFI_WARN_ON_ILLEGAL_ACCESSES > > How about shortening the name to just 'EFI_WARN_ON_ILLEGAL_ACCESS'? > Makes sense.. I will shorten it. > > + bool "Warn about illegal memory accesses by firmware" > > + depends on EFI > > From a distribution p-o-v, I would suggest that we set this CONFIG option only if > we are in the EXPERT mode, as this need more thrashing with the behaviour we > see on old, buggy EFI firmwares available on very old x86 machines. So > something like: > bool "Warn about illegal memory accesses by firmware" if EXPERT > Agreed. Although the feature is intended to warn about all these buggy firmware's instead of silently working around (as we are doing presently), it needs more testing. Hence, as you said, I think it's safe to have it as an EXPERT config option. > > + help > > + Enable this debug feature so that the kernel can detect illegal > > + memory accesses by firmware and issue a warning. Also, > > + 1. If the illegally accessed region is EFI_BOOT_SERVICES_<CODE/DATA>, > > + the kernel fixes it up by mapping the requested region. [....] > > diff --git a/init/main.c b/init/main.c index > > 3b4ada11ed52..dce0520861a1 100644 > > --- a/init/main.c > > +++ b/init/main.c > > @@ -730,7 +730,8 @@ asmlinkage __visible void __init start_kernel(void) > > arch_post_acpi_subsys_init(); > > sfi_init_late(); > > > > - if (efi_enabled(EFI_RUNTIME_SERVICES)) { > > + if (efi_enabled(EFI_RUNTIME_SERVICES) && > > + !IS_ENABLED(CONFIG_EFI_WARN_ON_ILLEGAL_ACCESSES)) { > > Since this is an arch agnostic code leg, do we really want to introduce a generic > check for CONFIG_EFI_WARN_ON_ILLEGAL_ACCESSES > here, without checking for whether we are actually running this on a > x86 hardware, or alternatively we can consider moving > CONFIG_EFI_WARN_ON_ILLEGAL_ACCESSES out of 'arch/x86/Kconfig' to > 'arch/Kconfig' so that later it can be used on other archs like ARM64 as well. > > I think the later would be more useful.. Thanks for bringing this up. I see your point. I will refrain from polluting architecture agnostic code. As we don't need efi_free_boot_services() at all, if EFI_WARN_ON_ILLEGAL_ACCESSES is enabled, probably making changes to include/linux/efi.h would be better, I guess.. Moving this to arch/Kconfig could be too futuristic.. I guess.. because I am not sure if this problem exists on ARM machines, if so, probably Ard could suggest something here. Regards, Sai
Powered by blists - more mailing lists