Re: [RFT PATCH] efi/x86: move x86 back to libstub
On Thu, Oct 16, 2014 at 10:42 AM, Matt Fleming m...@console-pimps.org wrote: On Mon, 06 Oct, at 01:06:43PM, Ard Biesheuvel wrote: This reverts commit 84be880560fb, which itself reverted my original attempt to move x86 from #include'ing .c files from across the tree to using the EFI stub built as a static library. The issue that affected the original approach was that splitting the implementation into several .o files resulted in the variable 'efi_early' becoming a global with external linkage, which under -fPIC implies that references to it must go through the GOT. However, dealing with this additional GOT entry turned out to be troublesome on some EFI implementations. (GCC's visibility=hidden attribute is supposed to lift this requirement, but it turned out not to work on the 32-bit build.) Instead, use a pure getter function to get a reference to efi_early. This approach results in no additional GOT entries being generated, so there is no need for any changes in the early GOT handling. Cc: Maarten Lankhorst maarten.lankho...@canonical.com Cc: Josh Boyer jwbo...@fedoraproject.org Cc: Linus Torvalds torva...@linux-foundation.org Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org --- Gents, This is a request for testing: I would like to find out if this patch fixes Maarten's issue without breaking anything like it did for Josh and Linus the first time around. arch/x86/boot/compressed/Makefile | 3 ++- arch/x86/boot/compressed/eboot.c | 8 arch/x86/boot/compressed/eboot.h | 16 arch/x86/include/asm/efi.h| 24 drivers/firmware/efi/Makefile | 2 +- 5 files changed, 31 insertions(+), 22 deletions(-) Ard, this is a very neat trick. Thanks for taking the time to reimplement this, it's much appreciated. Things work fine on my end, and I do have a Fedora 20/grub box in my testing farm now, so I'm confident this is a good patch. But if other people want to test it out, I'd be grateful of some more Tested-by tags (I've already picked up Maarten's). I applied this (with minor massaging) on top if Linus' tree as of commit 0429fbc0bdc297d64188483ba029a23773ae07b0 this morning. It built and booted on the macbook machine I originally reported the problem with, so it does appear to be fixed in that case. I'll try it on some other machines likely tomorrow. Sorry for the delay on this. The merge window is particularly busy for me. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trusted kernel patchset for Secure Boot lockdown
On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett matthew.garr...@nebula.com wrote: The conclusion we came to at Plumbers was that this patchset was basically fine but that Linus hated the name securelevel more than I hate pickled herring, so after thinking about this for a few months I've come up with Trusted Kernel. This flag indicates that the kernel is, via some external mechanism, trusted and should behave that way. If firmware has some way to verify the kernel, it can pass that information on. If userspace has some way to verify the kernel, it can set the flag itself. However, userspace should not attempt to use the flag as a means to verify that the kernel was trusted - untrusted userspace could have set it on an untrusted kernel, but by the same metric an untrusted kernel could just set it itself. FWIW, I've been running a kernel using this patchset in place of the patchset Fedora typically carries for this purpose for a bit. Things appear to be working as expected and the protections remain the same. It would be really nice to get this set of patches in so some of the other patches that depend on them can start being pushed as well. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trusted kernel patchset for Secure Boot lockdown
On Thu, Feb 27, 2014 at 2:07 PM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 27, 2014 at 01:04:34PM -0500, Josh Boyer wrote: On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett matthew.garr...@nebula.com wrote: The conclusion we came to at Plumbers was that this patchset was basically fine but that Linus hated the name securelevel more than I hate pickled herring, so after thinking about this for a few months I've come up with Trusted Kernel. This flag indicates that the kernel is, via some external mechanism, trusted and should behave that way. If firmware has some way to verify the kernel, it can pass that information on. If userspace has some way to verify the kernel, it can set the flag itself. However, userspace should not attempt to use the flag as a means to verify that the kernel was trusted - untrusted userspace could have set it on an untrusted kernel, but by the same metric an untrusted kernel could just set it itself. FWIW, I've been running a kernel using this patchset in place of the patchset Fedora typically carries for this purpose for a bit. Things appear to be working as expected and the protections remain the same. It would be really nice to get this set of patches in so some of the other patches that depend on them can start being pushed as well. What other patches depend on this series? Why aren't they also in this series? The patches we have to import certificates from the UEFI db and dbx vars, and MokListRT and apply them to signed module verification. Looking at them closely, there are pieces that could be sent now as they are slightly orthogonal to what this patchset is doing, which is probably why they aren't in this patchset to begin with. I'll have to figure out which of those actually depend on anything in Matthew's series. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/12] acpi: Ignore acpi_rsdp kernel parameter when securelevel is set
On Mon, Sep 9, 2013 at 11:49 AM, Matthew Garrett matthew.garr...@nebula.com wrote: From: Josh Boyer jwbo...@redhat.com This option allows userspace to pass the RSDP address to the kernel, which makes it possible for a user to execute arbitrary code in the kernel. Disable this when securelevel is set. Signed-off-by: Josh Boyer jwbo...@redhat.com Dredging up an old thread in the hopes that Matthew runs sed and resubmits... Also, FWIW, I didn't write this. It was derived from previous versions of something I did write, but there's really no evidence of anything I wrote left, so it should probably be From: you. --- drivers/acpi/osl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index e5f416c..f6d8977 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -45,6 +45,7 @@ #include linux/list.h #include linux/jiffies.h #include linux/semaphore.h +#include linux/security.h #include asm/io.h #include asm/uaccess.h @@ -249,7 +250,7 @@ early_param(acpi_rsdp, setup_acpi_rsdp); acpi_physical_address __init acpi_os_get_root_pointer(void) { #ifdef CONFIG_KEXEC - if (acpi_rsdp) + if (acpi_rsdp (get_securelevel = 0)) This is missing some ( ). That means you're comparing the get_securelevel function pointer to 0. Pretty sure bad things will happen. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/12] One more attempt at useful kernel lockdown
On Mon, Sep 9, 2013 at 4:10 PM, David Lang da...@lang.hm wrote: On Mon, 9 Sep 2013, Josh Boyer wrote: On Mon, Sep 9, 2013 at 3:41 PM, H. Peter Anvin h...@zytor.com wrote: On 09/09/2013 12:01 PM, valdis.kletni...@vt.edu wrote: On Mon, 09 Sep 2013 11:25:38 -0700, David Lang said: Given that we know that people want signed binaries without blocking kexec, you should have '1' just enforce module signing and '2' (or higher) implement a full lockdown including kexec. Or, eliminate the -1 permanently insecure option and make this a bitmask, if someone wants to enable every possible lockdown, have them set it to all 1's, define the bits only as you need them. This strikes me as much more workable than one big sledgehammer. I.e. capabilities ;) Circles. All I see here are circles. the thing is that these are not circles. they are separate orthoginal things that you may or may not want to allow. If this was a simple set of circles, then this could be defined as a vector instead of bitmap, the further you go the more secure you are. I didn't mean your recommendation of using a bitmask. I understood your proposal and I don't even disagree with it really. I was replying to something else. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 10/10] Add option to automatically enforce module signatures when in Secure Boot mode
On Wed, Sep 4, 2013 at 6:51 AM, joeyli j...@suse.com wrote: 於 五,2013-08-30 於 19:41 -0400,Josh Boyer 提到: On Fri, Aug 30, 2013 at 01:46:30PM -0700, H. Peter Anvin wrote: On 08/29/2013 11:37 AM, Josh Boyer wrote: setup_efi_pci(boot_params); diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index c15ddaf..d35da96 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -131,7 +131,8 @@ struct boot_params { __u8 eddbuf_entries; /* 0x1e9 */ __u8 edd_mbr_sig_buf_entries; /* 0x1ea */ __u8 kbd_status; /* 0x1eb */ -__u8 _pad5[3]; /* 0x1ec */ +__u8 secure_boot; /* 0x1ec */ +__u8 _pad5[2]; /* 0x1ec */ /* * The sentinel is set to a nonzero value (0xff) in header.S. * You need to include the following chunk of code with this, otherwise the secure_boot variable gets cleared. Not really. There are three cases: 1. Boot stub only. Here we do the right thing with the bootparams. 2. Boot loader bypasses the boot stub completely. Here we MUST NOT do what you suggest above. 3. Boot stub with a boot_params structure passed in. Here we should run sanitize_boot_params() (an inline for a reason) in the boot stub, before we set the secure_boot field. Once that is done, we again don't need that modification. OK. If 3 works, then great. All I know is that Fedora has been carrying the above hunk for months and it was missing in this patch set. So when I went to test it, the patches didn't do anything because the secure_boot field was getting cleared. I'm more than happy to try option 3, and I'll poke at it next week unless someone beats me to it. josh The secure_boot field cleaned by sanitize_boot_params() when using grub2 linuxefi to load efi stub kernel. I printed the boot_params-sentinel value, confirm this value is NOT 0 when running grub2 linuxefi path, the entry point is efi_stub_entry. On the other hand, the sentinel value is 0 when direct run efi stub kernel in UEFI shell, the secure_boot field can keep. Does that mean grub2 should clean the sentinel value? or we move the get secure_boot value to efi_init()? See V3 of this patch that Matthew sent yesterday. It calls sanitize_boot_params in efi_main before calling get_secure_boot. I tested that yesterday and it worked fine. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Sep 4, 2013 at 11:53 AM, Kees Cook keesc...@chromium.org wrote: On Tue, Sep 3, 2013 at 4:50 PM, Matthew Garrett matthew.garr...@nebula.com wrote: We have two in-kernel mechanisms for restricting module loading - disabling it entirely, or limiting it to the loading of modules signed with a trusted key. These can both be configured in such a way that even root is unable to relax the restrictions. However, right now, there's several other straightforward ways for root to modify running kernel code. At the most basic level these allow root to reset the configuration such that modules can be loaded again, rendering the existing restrictions useless. This patchset adds additional restrictions to various kernel entry points that would otherwise make it straightforward for root to disable enforcement of module loading restrictions. It also provides a patch that allows the kernel to be configured such that module signing will be automatically enabled when the system is booting via UEFI Secure Boot, allowing a stronger guarantee of kernel integrity. V3 addresses some review feedback and also locks down uswsusp. Looks good to me. Consider the entire series: Acked-by: Kees Cook keesc...@chromium.org I spent yesterday rebasing and testing Fedora 20 secure boot support to this series, and things have tested out fine on both SB and non-SB enabled machines. For the series: Reviewed-by: Josh Boyer jwbo...@fedoraproject.org Tested-by: Josh Boyer jwbo...@fedoraproject.org josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 08/11] kexec: Disable at runtime if the kernel enforces module loading restrictions
On Wed, Sep 4, 2013 at 4:09 PM, jerry.hoem...@hp.com wrote: On Tue, Sep 03, 2013 at 07:50:15PM -0400, Matthew Garrett wrote: kexec permits the loading and execution of arbitrary code in ring 0, which is something that module signing enforcement is meant to prevent. It makes sense to disable kexec in this situation. Signed-off-by: Matthew Garrett matthew.garr...@nebula.com Matthew, Disabling kexec will disable kdump, correct? Yes. Are there plans to enable kdump on a system where secure boot is enabled? Vivek Goyal has been working on this. I've not seen the code yet, but I believe it should be posted somewhere relatively soon. We're also planning on talking about it at the Secure Boot microconference at Linux Plumbers in two weeks. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] Add secure_modules() call
On Mon, Aug 19, 2013 at 01:26:02PM -0400, Matthew Garrett wrote: Provide a single call to allow kernel code to determine whether the system has been configured to either disable module loading entirely or to load only modules signed with a trusted key. Signed-off-by: Matthew Garrett matthew.garr...@nebula.com --- include/linux/module.h | 7 +++ kernel/module.c| 9 + 2 files changed, 16 insertions(+) diff --git a/include/linux/module.h b/include/linux/module.h index 46f1ea0..0c266b2 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -509,6 +509,8 @@ int unregister_module_notifier(struct notifier_block * nb); extern void print_modules(void); +extern bool secure_modules(void); + #else /* !CONFIG_MODULES... */ /* Given an address, look for it in the exception tables. */ @@ -619,6 +621,11 @@ static inline int unregister_module_notifier(struct notifier_block * nb) static inline void print_modules(void) { } + +static inline bool secure_modules(void) +{ + return false; +} #endif /* CONFIG_MODULES */ #ifdef CONFIG_SYSFS diff --git a/kernel/module.c b/kernel/module.c index 2069158..801021e 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -3852,3 +3852,12 @@ void module_layout(struct module *mod, } EXPORT_SYMBOL(module_layout); #endif + +bool secure_modules(void) +{ +#ifdef CONFIG_MODULE_SIG + return (sig_enforce || modules_disabled); +#else + return modules_disabled; +#endif +} Needs an EXPORT_SYMBOL/EXPORT_SYMBOL_GPL here, or the patches to drivers later in the series will fail to link. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/10] asus-wmi: Restrict debugfs interface when module loading is restricted
On Thu, Aug 29, 2013 at 12:05:47PM -0700, H. Peter Anvin wrote: On 08/29/2013 11:49 AM, Matthew Garrett wrote: No, you mixed and matched in a single patch... Right, but I'd fixed that in V2 (which I see I *did* send correctly, and you're just replying to the old one :)) Well, I'm responding to the one that was sent 31 minutes ago. No patches were sent 31 minutes ago. And this is definitely in the first series thread in my inbox. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] efi: Check EFI revision in setup_efi_vars
We need to check the runtime sys_table for the EFI version the firmware specifies instead of just checking for a NULL QueryVariableInfo. Older implementations of EFI don't have QueryVariableInfo but the runtime is a smaller structure, so the pointer to it may be pointing off into garbage. This is apparently the case with several Apple firmwares that support EFI 1.10, and the current check causes them to no longer boot. Fix based on a suggestion from Matthew Garrett. Signed-off-by: Josh Boyer jwbo...@redhat.com --- arch/x86/boot/compressed/eboot.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 8615f75..b46efbf 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -258,7 +258,9 @@ static efi_status_t setup_efi_vars(struct boot_params *params) u64 store_size, remaining_size, var_size; efi_status_t status; - if (!sys_table-runtime-query_variable_info) + if (sys_table-runtime-hdr.revision EFI_2_00_SYSTEM_TABLE_REVISION) + return EFI_UNSUPPORTED; + else if(!sys_table-runtime-query_variable_info) return EFI_UNSUPPORTED; data = (struct setup_data *)(unsigned long)params-hdr.setup_data; -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi: Check EFI revision in setup_efi_vars
On Wed, Apr 24, 2013 at 03:44:30PM +0100, Matt Fleming wrote: On 24/04/13 15:37, Josh Boyer wrote: We need to check the runtime sys_table for the EFI version the firmware specifies instead of just checking for a NULL QueryVariableInfo. Older implementations of EFI don't have QueryVariableInfo but the runtime is a smaller structure, so the pointer to it may be pointing off into garbage. This is apparently the case with several Apple firmwares that support EFI 1.10, and the current check causes them to no longer boot. Fix based on a suggestion from Matthew Garrett. Signed-off-by: Josh Boyer jwbo...@redhat.com --- arch/x86/boot/compressed/eboot.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 8615f75..b46efbf 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -258,7 +258,9 @@ static efi_status_t setup_efi_vars(struct boot_params *params) u64 store_size, remaining_size, var_size; efi_status_t status; - if (!sys_table-runtime-query_variable_info) + if (sys_table-runtime-hdr.revision EFI_2_00_SYSTEM_TABLE_REVISION) + return EFI_UNSUPPORTED; + else if(!sys_table-runtime-query_variable_info) return EFI_UNSUPPORTED; data = (struct setup_data *)(unsigned long)params-hdr.setup_data; Thanks Josh, that looks correct. It's a small point, but does the check against NULL actually make sense? I don't think we ever check other system table pointers against NULL. That I'm not sure of. I was going off of the assumption that Matthew put it there because someone's EFI 2.0 implementation was crappy and didn't actually implement it. So I left that check in place for now. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Fri, Apr 19, 2013 at 08:50:14AM +0100, Matt Fleming wrote: On 04/19/2013 01:18 AM, Darren Hart wrote: On 04/18/2013 09:19 AM, Matt Fleming wrote: Could you give it a spin on your MinnowBoard? I've removed the patch I reference above and applied your patch to my 3.8.4 MinnowBoard dev tree. It panics with: D'oh. OK, at this point I'm inclined to apply Josh Boyer's patch on top of my urgent branch which will address the WARNING people are hitting on i386. I updated the commit message a little. Josh (Boyer), are you guys still carrying this patch and have you seen any fallout? I notice your SoB isn't on the patch that Darren posted, am I OK to add it? Yeah, we're still carrying it. We've had it around since August of 2011 to cover the bug mentioned. I'm not aware of any further fallout it might have caused, but again we don't support 32-bit EFI in Fedora. That's not to say Fedora won't work, we just don't focus on it. I don't actually remember authoring the patch, but it's been a while so: Signed-off-by: Josh Boyer jwbo...@redhat.com josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, Apr 18, 2013 at 12:00:26PM +0100, Matt Fleming wrote: On 17/04/13 23:00, Bryan O'Donoghue wrote: In my mind the only memory that is relevant to efi_enter_virtual_mode is memory that the BIOS has marked as EFI_RUNTIME_SERVICE md-attribute 0x8000_ I couldn't quite understand why the code in arch/x86/platform/efi/efi.c:enter_virtual_mode() looks like this for (p = memmap.map; p memmap.map_end; p += memmap.desc_size) { md = p; if (!(md-attribute EFI_MEMORY_RUNTIME) md-type != EFI_BOOT_SERVICES_CODE md-type != EFI_BOOT_SERVICES_DATA) continue; While the code in arch/ia64/kernel/efi.c:enter_virtual_mode for (p = efi_map_start; p efi_map_end; p += efi_desc_size) { md = p; if (md-attribute EFI_MEMORY_RUNTIME) { The ia64 version is consistent with the standard - but obviously isn't accounting for the work-around implemented to retrieve the BGRT on ia32. Looking at the commit message associated with arch/x86/platform/efi/efi-bgrt.c It's pretty obvious the mapping of boot code/data was done to facilitate BGRT. No, that's incorrect. The patch that introduced the mapping of EFI_BOOT_SERVICES_{CODE,DATA} was committed before support for bgrt existed. git blame is a good tool to use when doing one of these historical digs, and in this case it shows that the above lines from efi_enter_virtual_mode() were introduced in the following commit, commit 916f676f8dc016103f983c7ec54c18ecdbb6e349 Author: Matthew Garrett m...@redhat.com Date: Wed May 25 09:53:13 2011 -0400 x86, efi: Retain boot service code until after switching to virtual mode UEFI stands for Unified Extensible Firmware Interface, where Firmware is an ancient African word meaning Why do something right when you can do it so wrong that children will weep and brave adults will cower before you, and UEI is Celtic for We missed DOS so we burned it into your ROMs. The UEFI specification provides for runtime services (ie, another way for the operating system to be forced to depend on the firmware) and we rely on these for certain trivial tasks such as setting up the bootloader. But some hardware fails to work if we attempt to use these runtime services from physical mode, and so we have to switch into virtual mode. So far so dreadful. The specification makes it clear that the operating system is free to do whatever it wants with boot services code after ExitBootServices() has been called. SetVirtualAddressMap() can't be called until ExitBootServices() has been. So, obviously, a whole bunch of EFI implementations call into boot services code when we do that. Since we've been charmingly naive and trusted that the specification may be somehow relevant to the real world, we've already stuffed a picture of a penguin or something in that address space. And just to make things more entertaining, we've also marked it non-executable. This patch allocates the boot services regions during EFI init and makes sure that they're executable. Then, after SetVirtualAddressMap(), it discards them and everyone lives happily ever after. Except for the ones who have to work on EFI, who live sad lives haunted by the knowledge that someone's eventually going to write yet another firmware specification. [ hpa: adding this to urgent with a stable tag since it fixes currently-broken hardware. However, I do not know what the dependencies are and so I do not know which -stable versions this may be a candidate for. ] Signed-off-by: Matthew Garrett m...@redhat.com Link: http://lkml.kernel.org/r/1306331593-28715-1-git-send-email-...@redhat.com Signed-off-by: H. Peter Anvin h...@linux.intel.com Cc: Tony Luck tony.l...@intel.com Cc: sta...@kernel.org Yes the bgrt code accesses the Boot Service mappings, but that isn't the only reason we want to map those regions. That's one solution - you'd need to make the i386::efi_ioremap() aware of the BGRT work-around. Presumably this work-around is also required on ia64 too. No, we've never seen an ia64 firmware implementation with the access EFI Boot Services Code/Data after ExitBootServices() bug, and it doesn't suffer from the same virtual address space limitations that i386 does. No, no - we *don't* have a BGRT object at all. We have a completely clean memory map - but the BGRT code is causing the is_ram() failure. You assume that mapping of the Boot Services regions is done purely for the benefit of pulling out the bgrt image - it's not, see the above commit log - and I assumed that you had an ACPI bgrt pointer in your memory map, but you don't. Darren, Josh, have you ever seen an i386 machine with
Re: [PATCH 05/12] PCI: Require CAP_COMPROMISE_KERNEL for PCI BAR access
On Wed, Mar 27, 2013 at 11:08 AM, Kyle McMartin kmcma...@redhat.com wrote: On Wed, Mar 27, 2013 at 11:03:26AM -0400, Josh Boyer wrote: On Mon, Mar 18, 2013 at 5:32 PM, Matthew Garrett matthew.garr...@nebula.com wrote: Any hardware that can potentially generate DMA has to be locked down from userspace in order to avoid it being possible for an attacker to cause arbitrary kernel behaviour. Default to paranoid - in future we can potentially relax this for sufficiently IOMMU-isolated devices. Signed-off-by: Matthew Garrett matthew.garr...@nebula.com As noted here: https://bugzilla.redhat.com/show_bug.cgi?id=90 this breaks pci passthru with QEMU. The suggestion in the bug is to move the check from read/write to open, but sysfs makes that somewhat difficult. The open code is part of the core sysfs functionality shared with the majority of sysfs files, so adding a check there would restrict things that clearly don't need to be restricted. Kyle had the idea to add a cap field to the attribute structure, and do a capable check if that is set. That would allow for a more generic usage of capabilities in sysfs code, at the cost of slightly increasing the structure size and open path. That seems somewhat promising if we stick with capabilities. I would love to just squarely blame capabilities for causing this, but we can't just replace it with an efi_enabled(EFI_SECURE_BOOT) check because of the sysfs open case. I'm not sure there are great answers here. Yeah, that was something like this (I don't even remember which Fedora kernel version this was against.) Mostly an FYI for the peanut gallery, but we noticed moving the cap check to open breaks lspci being run by an unprivileged user. It also doesn't fix pci passthrough because QEMU opens the PCI resource files by itself after it's already dropped all caps. More thinking required. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] PCI: Require CAP_COMPROMISE_KERNEL for PCI BAR access
On Mon, Mar 18, 2013 at 5:32 PM, Matthew Garrett matthew.garr...@nebula.com wrote: Any hardware that can potentially generate DMA has to be locked down from userspace in order to avoid it being possible for an attacker to cause arbitrary kernel behaviour. Default to paranoid - in future we can potentially relax this for sufficiently IOMMU-isolated devices. Signed-off-by: Matthew Garrett matthew.garr...@nebula.com As noted here: https://bugzilla.redhat.com/show_bug.cgi?id=90 this breaks pci passthru with QEMU. The suggestion in the bug is to move the check from read/write to open, but sysfs makes that somewhat difficult. The open code is part of the core sysfs functionality shared with the majority of sysfs files, so adding a check there would restrict things that clearly don't need to be restricted. Kyle had the idea to add a cap field to the attribute structure, and do a capable check if that is set. That would allow for a more generic usage of capabilities in sysfs code, at the cost of slightly increasing the structure size and open path. That seems somewhat promising if we stick with capabilities. I would love to just squarely blame capabilities for causing this, but we can't just replace it with an efi_enabled(EFI_SECURE_BOOT) check because of the sysfs open case. I'm not sure there are great answers here. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: Lock down MSR writing in secure boot
On Fri, Feb 8, 2013 at 4:07 PM, Matthew Garrett matthew.garr...@nebula.com wrote: On Fri, 2013-02-08 at 13:02 -0800, Kees Cook wrote: I don't find it unreasonable to drop all caps and lose access to sensitive things. :) That's sort of the point, really. I think a cap is the best match. It seems like it should either be a cap or a namespace flag, but the latter seems messy. Yeah, I think it's an expected outcome, but it means that if (say) qemu drops privileges, qemu can no longer access PCI resources - even on non-secure boot systems. That breaks existing userspace. Right. We've had a few reports in Fedora of things breaking on non-SB systems because of this. The qemu one is the latest, but the general problem is people think dropping all caps blindly is making their apps safer. Then they find they can't do things they could do before the new cap was added. It's messy. I've thought of treating CAP_COMPROMISE_KERNEL as a hidden cap, where only the kernel can grant or drop it. Peter Jones suggested it might work to allow it to be dropped iff it is the only cap being changed. Either way, it's a special cap and I have no idea how acceptable something like that would be. Really though, the main issue is that you cannot introduce new caps to enforce finer grained access without breaking something. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/4] Add firmware signature file check
On Mon, Nov 5, 2012 at 12:18 PM, Takashi Iwai ti...@suse.de wrote: Hi, this is a patch series to add the support for firmware signature check. At this time, the kernel checks extra signature file (*.sig) for each firmware, instead of embedded signature. It's just a quick hack using the existing module signing mechanism, thus provided only as a proof of concept for now. To be noted, it doesn't support the firmwares via udev but only the direct loading, and the check for built-in firmware is missing, too. Just to make sure I'm reading this correctly, it will sign any of the firwmare files installed directly from the kernel tree if the option is set. So for the firmware in the linux-firmware tree we'd need to either copy that into the kernel tree during build time, or duplicate the signing bits in the linux-firmware tree itself. However if we do the latter, we'd probably need to use the same keys as the per-build kernel key which means copying keys (ew) or tell the kernel to include a separate firmware key in the extra list. I feel like I'm rambling a bit, but I'm trying to work out how signed firmware would look from a distro perspective. A significant amount of work has been done to decouple linux-firmware from the kernel tree and if signed firmware is used it seems to couple them together one way or another. At the moment, using generated per-build keys to come up with the firmware signatures seems a bit suboptimal in that regard. josh -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html