Re: [PATCH v3 2/5] efi: Add embedded peripheral firmware support
On Thu, Jun 07, 2018 at 09:49:50AM -0700, Bjorn Andersson wrote: > On Tue 08 May 09:10 PDT 2018, Luis R. Rodriguez wrote: > > > On Tue, May 08, 2018 at 03:38:05PM +0000, Luis R. Rodriguez wrote: > > > On Fri, May 04, 2018 at 12:44:37PM -0700, Martijn Coenen wrote: > > > > On Wed, Apr 25, 2018 at 10:55 AM, Luis R. Rodriguez > > > > wrote: > [..] > > > > 2) Most of those paths are not mounted by the time the corresponding > > > > drivers are loaded, because pretty much all Android kernels today are > > > > built without module support, and therefore drivers are loaded well > > > > before the firmware partition is mounted > > > > I've given this some more thought and you can address this with initramfs, > > this is how other Linux distributions are addressing this. One way to > > address this automatically is to scrape the drivers built-in or needed > > early on > > boot in initamfs and if the driver has a MODULE_FIRMWARE() its respective > > firmware is added to initramfs as well. > > > > That could be done, but it would not change the fact that the > /sys/class/firmware is ABI and you may not break it. Right, this is now well documented and also the latest changes to the firmware API have made the sysfs fallback loader an option through a sysctl knob. The code should be much easier to follow and test now. > And it doesn't change the fact that the ramdisk would have to be over > 100mb to facilitate this. Indeed, this is now acknowledged in the latest Kconfig for the firmware loader. > > If you *don't* use initramfs, then yes you can obviously run into issues > > where your firmware may not be accessible if the driver is somehow loaded > > early. > > > > This is still a problem that lacks a solution. The firmwared solution capturing uevents and using the sysfs fallback mechanism should resolve this. Its also now properly documented on the firmware loader Kconfig. > > > > 3) I think we use _FALLBACK because doing this with uevents is just > > > > the easiest thing to do; our init code has a firmware helper that > > > > deals with this and searches the paths that we care about > > > > > > > > 2) will change at some point, because Android is moving towards a > > > > model where device-specific peripheral drivers will be loaded as > > > > modules, and since those modules would likely come from the same > > > > partition as the firmware, it's possible that the direct load would > > > > succeed (depending on whether the custom path is configured there or > > > > not). But I don't think we can rely on the direct loader even in those > > > > cases, unless we could configure it with multiple custom paths. > > > > Using initramfs will help, but because of the custom path needs -- you're > > right, we don't have anything for that yet, its also a bit unclear if > > something nice and clean can be drawn up for it. So perhaps dealing with > > the fallback mechanism is the way to go for this for sure, since we already > > have support for it. > > > > Just keep in mind that the fallback mechanism costs you about ~13436 bytes. > > > > Remember that putting the firmware in the ramdisk would cost about > 1x (yes, ten thousand times) more ram. Indeed, this is now documented on the Kconfig too. > > So, if someone comes up with a clean interface for custom paths I'd love > > to consider it to avoid those 13436 bytes. > > > > Combined with a way of synchronizing this with the availability of the > firmware, this would be a nice thing! The firmwared solution seems to be the way to go for now. Luis -- 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 v6 2/5] efi: Add embedded peripheral firmware support
On Thu, Jun 07, 2018 at 03:46:11PM +0200, Hans de Goede wrote: > On 06-06-18 23:42, Luis R. Rodriguez wrote: > > On Wed, Jun 06, 2018 at 08:39:15PM +0200, Hans de Goede wrote: > > > On 05-06-18 23:07, Luis R. Rodriguez wrote: > > > 2) Should this flag then be checked inside _request_firmware() before it > > > calls fw_get_efi_embedded_fw() (which may be an empty stub), > > > > You are the architect behind this call, so its up to you. > > > > To answer this you have to review the other flags and see if other users of > > the > > other flags may want your functionality. For instance the Android folks for > > instance rely on the FW_OPT_NOFALLBACK - the sysfs fallback mechanism to > > account for odd partition layouts. Could they ever want to use your fallback > > mechanism? Granted your mechanism is for x86, but they could eventually add > > support for it on ARM. > > > > Checking if the firmware is on the EFI platform firmware list is much faster > > than the fallback mechanism, that would be one gain for them, as such it may > > make sense to check for firmware_request_platform() before using the sysfs > > fallback mechanism. However if Android folks want to always override the > > platform firmware with the sysfs fallback interface we'd need another flag > > added and call to then change the order later if we checked for for the > > platform firmware first. > > I believe we agreed a while back that the platform fallback would > replace the sysfs one when requested. I believe that still makes > sense. If a driver wants both it can simply call request_firmware_foo > itself twice and determine the order itself. Fine by me, so in your case the syfs fallback will be ignored. Which gets me thinking that perhaps we should have a separate syfs fallback call. There are only two drivers that use it explicitly: o CONFIG_LEDS_LP55XX_COMMON request_firmware_nowait(THIS_MODULE, false, ...); o CONFIG_DELL_RBU request_firmware_nowait(THIS_MODULE, FW_ACTION_NOHOTPLUG, ...) And FW_ACTION_NOHOTPLUG is 0. The above revelation of the async call being data driven is a good opportunity to break that tradition to the more preferred functional one. And so we'd also get rid of the FW_ACTION_NOHOTPLUG option all together. So a new firmware_request_sysfs_async() I suppose. Luis -- 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 v6 2/5] efi: Add embedded peripheral firmware support
On Wed, Jun 06, 2018 at 08:39:15PM +0200, Hans de Goede wrote: > On 05-06-18 23:07, Luis R. Rodriguez wrote: > > > +To make request_firmware() fallback to trying EFI embedded firmwares > > > after this, > > > +the driver must set a boolean "efi-embedded-firmware" device-property on > > > the > > > +device before passing it to request_firmware(). > > > > No, as I have requested before I don't want this, it is silly to have such > > functionality always be considered as a fallback if we only have 2 drivers > > which need this. > Please add a call which only if used would then > > *evaluate* > > such fallback prospect. > > Ok, so I've a few questions about this: > > 1) You want me to add a: > > int firmware_request_with_system_firmware_fallback(struct device *device, > const char *name); > > function? The idea is correct, the name however is obviously terrible. This is functionality that is very specialized and only *two* device drivers need it that we are aware of which would be upstream. Experience has shown fallback mechanisms *can* be a pain, and if we add this we will be supporting this for *life*, as such I'd very much prefer to: a) *Clearly* reduce the scope of functionality clearly *beyond* what you have done. b) Have access to one simple call which folks can use to *clearly* and quickly grep for those oddball drivers using this new interface. We can do this by using a separate function for it. Before you claim something seems unreasonable from the above logic, please read the latest state of affairs with respect to data driven Vs functional API evolution discussion over the firmware API [0] as well as my latests recommendation for what to do for the async firmware API [1]. The skinny of it is that long ago I actually proposed having only *two* firmware API calls, an async and a sync call and having all functionality fleshed out through a structure of parameters. The issue with that strategy was it was *too* data driven, and as per Greg's request we'll instead be exposing new symbols per functionality for the firmware API with his justification that this is just what is traditionally done on Linux. Hence we have firmware_request_nowarn() now for just a slight variation for a sync call. Despite Greg's recommendation -- for the respective async functionality I suggested this is not going to scale well -- it is also just dumb to follow the same approach there for a few reasons. 1) We have only *one* async call and had decided to *not* provide a structure for that call since its inception 2) Over time have evolved this single async call each time we have a new feature, causing a slew of collateral evolutions. So, while we like it or not, it turns out the async call to the firmware API is already completely data driven. Extending it with just another argument would just be silly now. So refer to my recommendations to Andres for how to evolve the async API if you need it, however from a quick review you don't need async calls, so you won't have to address any of that. [0] https://lkml.kernel.org/r/20180421173650.gw14...@wotan.suse.de [1] https://lkml.kernel.org/r/20180422202609.gx14...@wotan.suse.de > Note I've deliberately named it with_system_firmware_fallback > and not with_efi_fallback to have the name be platform agnostic in > case we want something similar on other platforms in the future. firmware_request_platform() ? > And then have this pass a new FW_OPT_SYS_FW_FALLBACK flag to > _request_firmware(), right ? Yeap. > 2) Should this flag then be checked inside _request_firmware() before it > calls fw_get_efi_embedded_fw() (which may be an empty stub), You are the architect behind this call, so its up to you. To answer this you have to review the other flags and see if other users of the other flags may want your functionality. For instance the Android folks for instance rely on the FW_OPT_NOFALLBACK - the sysfs fallback mechanism to account for odd partition layouts. Could they ever want to use your fallback mechanism? Granted your mechanism is for x86, but they could eventually add support for it on ARM. Checking if the firmware is on the EFI platform firmware list is much faster than the fallback mechanism, that would be one gain for them, as such it may make sense to check for firmware_request_platform() before using the sysfs fallback mechanism. However if Android folks want to always override the platform firmware with the sysfs fallback interface we'd need another flag added and call to then change the order later if we checked for for the platform firmware first. If you however are 100% sure they won't need it, than checking firmware_request_platform() first would make sense now if you are certain these same devices won't need the s
Do Qualcomm drivers use DMA buffers for request_firmware_into_buf()?
On Fri, Jun 01, 2018 at 09:23:46PM +0200, Luis R. Rodriguez wrote: > On Tue, May 08, 2018 at 03:38:05PM +0000, Luis R. Rodriguez wrote: > > On Fri, May 04, 2018 at 12:44:37PM -0700, Martijn Coenen wrote: > > > > > > I think the Qualcomm folks owning this (Andy, David, Bjorn, already > > > cc'd here) are better suited to answer that question. > > > > Andy, David, Bjorn? > > Andy, David, Bjorn? A month now with no answer... Perhaps someone who has this hardware can find out empirically for us, as follows (mm folks is this right?): page = virt_to_page(address); if (!page) fail closed... if (page_zone(page) == ZONE_DMA || page_zone(page) == ZONE_DMA32) this is a DMA buffer else not DMA! Note that when request_firmware_into_buf() was being reviewed Mimi had asked back in 2016 [0] that if a DMA buffer was going to be used READING_FIRMWARE_DMA should be used otherwise READING_FIRMWARE_PREALLOC_BUFFER was fine. If it is a DMA buffer *now*, why / how did this change? [0] https://patchwork.kernel.org/patch/9074611/ Luis -- 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 v6 0/5] efi/firmware/platform-x86: Add EFI embedded fw support
On Wed, Jun 06, 2018 at 08:17:26PM +0200, Hans de Goede wrote: > But yes this means that these probably won't go in for another > cycle or 2, that is fine. > > > > -Add new READING_FIRMWARE_EFI_EMBEDDED read_file_id and use it > > > -Call security_kernel_read_file(NULL, READING_FIRMWARE_EFI_EMBEDDED) > > > to check if this is allowed before looking at EFI embedded fw > > > > There's a discussion over having security_kernel_read_file(NULL, > > READING_WHATEVER) become another LSM hook. So your series would conflict > > with > > that at the moment. > > > > So yet another piece of code which this series depends on. > > Ah well, I'm in no big hurry to get this merged. OTOH if this is > ready and that discussion is not yet finished it might be better > to merge this as is and then have the security_kernel_read_file / LSM > hook series fix this up as necessary when it is merged. True, there is also value in getting this series reviewed so that all that is needed is to consider merging it, so if you address the new call as I requested in a next series I'll review the series then. Luis -- 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 v6 2/5] efi: Add embedded peripheral firmware support
On Fri, Jun 01, 2018 at 02:53:27PM +0200, Hans de Goede wrote: > Just like with PCI options ROMs, which we save in the setup_efi_pci* > functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself > sometimes may contain data which is useful/necessary for peripheral drivers > to have access to. > > Specifically the EFI code may contain an embedded copy of firmware which > needs to be (re)loaded into the peripheral. Normally such firmware would be > part of linux-firmware, but in some cases this is not feasible, for 2 > reasons: > > 1) The firmware is customized for a specific use-case of the chipset / use > with a specific hardware model, so we cannot have a single firmware file > for the chipset. E.g. touchscreen controller firmwares are compiled > specifically for the hardware model they are used with, as they are > calibrated for a specific model digitizer. > > 2) Despite repeated attempts we have failed to get permission to > redistribute the firmware. This is especially a problem with customized > firmwares, these get created by the chip vendor for a specific ODM and the > copyright may partially belong with the ODM, so the chip vendor cannot > give a blanket permission to distribute these. > > This commit adds support for finding peripheral firmware embedded in the > EFI code and making this available to peripheral drivers through the > standard firmware loading mechanism. > > Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware near the end > of start_kernel(), just before calling rest_init(), this is on purpose > because the typical EFI_BOOT_SERVICES_CODE memory-segment is too large for > early_memremap(), so the check must be done after mm_init(). This relies > on EFI_BOOT_SERVICES_CODE not being free-ed until efi_free_boot_services() > is called, which means that this will only work on x86 for now. > > Reported-by: Dave Olsthoorn > Suggested-by: Peter Jones > Acked-by: Ard Biesheuvel > Signed-off-by: Hans de Goede > --- > Changes in v6: > -Rework code to remove casts from if (prefix == mem) comparison > -Use SHA256 hashes instead of crc32 sums > -Add new READING_FIRMWARE_EFI_EMBEDDED read_file_id and use it > -Call security_kernel_read_file(NULL, READING_FIRMWARE_EFI_EMBEDDED) > to check if this is allowed before looking at EFI embedded fw > -Document why we are not using the PI Firmware Volume protocol > > Changes in v5: > -Rename the EFI_BOOT_SERVICES flag to EFI_PRESERVE_BS_REGIONS > > Changes in v4: > -Drop note in docs about EFI_FIRMWARE_VOLUME_PROTOCOL, it is not part of > UEFI proper, so the EFI maintainers don't want us referring people to it > -Use new EFI_BOOT_SERVICES flag > -Put the new fw_get_efi_embedded_fw() function in its own fallback_efi.c > file which only gets built when EFI_EMBEDDED_FIRMWARE is selected > -Define an empty stub for fw_get_efi_embedded_fw() in fallback.h hwen > EFI_EMBEDDED_FIRMWARE is not selected, to avoid the need for #ifdefs > in firmware_loader/main.c > -Properly call security_kernel_post_read_file() on the firmware returned > by efi_get_embedded_fw() to make sure that we are allowed to use it > > Changes in v3: > -Fix the docs using "efi-embedded-fw" as property name instead of > "efi-embedded-firmware" > > Changes in v2: > -Rebased on driver-core/driver-core-next > -Add documentation describing the EFI embedded firmware mechanism to: > Documentation/driver-api/firmware/request_firmware.rst > -Add a new EFI_EMBEDDED_FIRMWARE Kconfig bool and only build the embedded > fw support if this is set. This is an invisible option which should be > selected by drivers which need this > -Remove the efi_embedded_fw_desc and dmi_system_id-s for known devices > from the efi-embedded-fw code, instead drivers using this are expected to > export a dmi_system_id array, with each entries' driver_data pointing to a > efi_embedded_fw_desc struct and register this with the efi-embedded-fw code > -Use kmemdup to make a copy instead of efi_mem_reserve()-ing the firmware, > this avoids us messing with the EFI memmap and avoids the need to make > changes to efi_mem_desc_lookup() > -Make the firmware-loader code only fallback to efi_get_embedded_fw() if the > passed in device has the "efi-embedded-firmware" device-property bool set > -Skip usermodehelper fallback when "efi-embedded-firmware" device-property > is set > --- > .../driver-api/firmware/request_firmware.rst | 76 + > drivers/base/firmware_loader/Makefile | 1 + > drivers/base/firmware_loader/fallback.h | 12 ++ > drivers/base/firmware_loader/fallback_efi.c | 56 +++ > drivers/base/firmware_loader/main.c | 2 + > drivers/firmware/efi/Kconfig | 3 + > drivers/firmware/efi/Makefile | 1 + > drivers/firmware/efi/embedded-firmware.c | 148 ++ > include/linux/efi.h | 6 + > include/linux/efi_embedded_fw.h | 25 +++ > include/li
Re: [PATCH v6 0/5] efi/firmware/platform-x86: Add EFI embedded fw support
On Fri, Jun 01, 2018 at 02:53:25PM +0200, Hans de Goede wrote: > Hi All, > > Here is v6 of my patch-set to add support for EFI embedded fw to the kernel. > > This patch-set applies on top of the "[PATCH v7 00/14] firmware_loader > changes for v4.18" series from mcgrof. > > It now also depends on the series from Andy Lutomirski which allow using the > sha256 code in a standalone manner. Andy what is the status of those? > > Changes since v5: > -Rework code to remove casts from if (prefix == mem) comparison > -Use SHA256 hashes instead of crc32 sums Nice! I see no updates on this progress, but it would seem this may then mean this cannot be merged until the release after? > -Add new READING_FIRMWARE_EFI_EMBEDDED read_file_id and use it > -Call security_kernel_read_file(NULL, READING_FIRMWARE_EFI_EMBEDDED) > to check if this is allowed before looking at EFI embedded fw There's a discussion over having security_kernel_read_file(NULL, READING_WHATEVER) become another LSM hook. So your series would conflict with that at the moment. So yet another piece of code which this series depends on. > -Document why we are not using the PI Firmware Volume protocol > > For reference I've included the coverletter from v4 (which includes > previous covverletters) below. > > Regards, > > Hans > > > Previous coverletter: > > Here is v5 of my patch-set to add support for EFI embedded fw to the kernel. > > Changes since v4: > -Rename the EFI_BOOT_SERVICES flag to EFI_PRESERVE_BS_REGIONS > > So I think this patch-set is getting close to ready for merging, which > brings us to the question of how to merge this, I think that patches 1 > and 2 should probably both be merged through the same tree. Then an > unmutable branch should be created on that tree, merged into the > platform/x86 tree and then the last 3 patches can be merged through > that tree. > > Ard has already indicated he is fine with the EFI bits going upstream > through another tree, so perhaps patches 1-2 can be merged through the > firmware-loader-tree and then do an unmutable branch on the > firmware-loader-tree for the platform/x86 tree to merge? > > I don't think taking all 5 through 1 tree is a good idea because of > the file rename under platform/x86. > > > For the record here are the cover letters of the previous versions: > > Changes since v3: > -Drop note in docs about EFI_FIRMWARE_VOLUME_PROTOCOL, it is not part of > UEFI proper, so the EFI maintainers don't want us referring people to it > -Use new EFI_BOOT_SERVICES flag > -Put the new fw_get_efi_embedded_fw() function in its own fallback_efi.c > file which only gets built when EFI_EMBEDDED_FIRMWARE is selected > -Define an empty stub for fw_get_efi_embedded_fw() in fallback.h hwen > EFI_EMBEDDED_FIRMWARE is not selected, to avoid the need for #ifdefs > in firmware_loader/main.c > -Properly call security_kernel_post_read_file() on the firmware returned > by efi_get_embedded_fw() to make sure that we are allowed to use it > > The 3 most prominent changes in v2 are: > > 1) Add documentation describing the EFI embedded firmware mechanism to: >Documentation/driver-api/firmware/request_firmware.rst > > 2) Instead of having a single dmi_system_id array with its driver_data >members pointing to efi_embedded_fw_desc structs, have the drivers which >need EFI embedded-fw support export a dmi_system_id array and register >that with the EFI embedded-fw code > >This series also includes the first driver to use this, in the form of >the touchscreen_dmi code (formerly silead_dmi) from drivers/platfrom/x86 > > 3) As discussed during the review of v1 we want to make the firmware_loader >code fallback to EFI embedded-fw optional. Rather the adding yet another >firmware_request_foo variant for this, with the risk of later also needing >firmware_request_foo_nowait, etc. variants I've decided to make the code >check if the device has a "efi-embedded-firmware" device-property bool set. > >This also seemed better because the same driver may want to use the >fallback on some systems, but not on others since e.g. not all (x86) >systems with a silead touchscreen have their touchscreen firmware embedded >in their EFI. > >Note that (as discussed) when the EFI fallback path is requested, the >usermodehelper fallback path is skipped. > > Here is the full changelog of patch 2/5 which is where most of the changes > are: > > Changes in v2: > -Rebased on driver-core/driver-core-next > -Add documentation describing the EFI embedded firmware mechanism to: > Documentation/driver-api/firmware/request_firmware.rst > -Add a new EFI_EMBEDDED_FIRMWARE Kconfig bool and only build the embedded > fw support if this is set. This is an invisible option which should be > selected by drivers which need this > -Remove the efi_embedded_fw_desc and dmi_system_id-s for known devices > from the efi-embedded-fw code, instead drivers using this are expected
Re: [PATCH v3 2/5] efi: Add embedded peripheral firmware support
On Tue, May 08, 2018 at 03:38:05PM +, Luis R. Rodriguez wrote: > On Fri, May 04, 2018 at 12:44:37PM -0700, Martijn Coenen wrote: > > On Wed, Apr 25, 2018 at 10:55 AM, Luis R. Rodriguez > > wrote: > > > Is ptr below > > > > > > ret = request_firmware_into_buf(&seg_fw, fw_name, dev, > > > ptr, phdr->p_filesz); > > > > > > Also part of the DMA buffer allocated earlier via: > > > > > > ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size); > > > > > > Android folks? > > > > I think the Qualcomm folks owning this (Andy, David, Bjorn, already > > cc'd here) are better suited to answer that question. > > Andy, David, Bjorn? Andy, David, Bjorn? Note: as-is we have no option but to assume this is DMA memory for now. We cannot keep IMA's guarantees with the current prealloc firmware API buffer, so I've suggested: a) The prealloc buffer API be expanded to enable the caller to descrbe it b) Have the qcom driver say this is DMA c) IMA would reject it to ensure it stays true to what it needs to gaurantee d) Future platforms which want to use IMA but want to trust DMA buffers would need to devise a way to describe IMA can trust some of these calls. I'll leave it up to you guys (Andy, David, Bjorn) to come up with the code for d) once and if you guys want to use IMA later. But since what is pressing here is to stay to true to IMA, with a-c IMA would reject such calls for now. Luis -- 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 v5 2/5] efi: Add embedded peripheral firmware support
On Fri, May 04, 2018 at 07:54:28AM +0200, Ard Biesheuvel wrote:
> On 4 May 2018 at 01:29, Luis R. Rodriguez wrote:
> > On Sun, Apr 29, 2018 at 11:35:55AM +0200, Hans de Goede wrote:
> [...]
> >> diff --git a/Documentation/driver-api/firmware/request_firmware.rst
> >> b/Documentation/driver-api/firmware/request_firmware.rst
> >> index c8bddbdcfd10..560dfed76e38 100644
> >> --- a/Documentation/driver-api/firmware/request_firmware.rst
> >> +++ b/Documentation/driver-api/firmware/request_firmware.rst
> >> @@ -73,3 +73,69 @@ If something went wrong firmware_request() returns
> >> non-zero and fw_entry
> >> is set to NULL. Once your driver is done with processing the firmware it
> >> can call call firmware_release(fw_entry) to release the firmware image
> >> and any related resource.
> >> +
> >> +EFI embedded firmware support
> >> +=
> >
> > This is a new fallback mechanism, please see:
> >
> > Documentation/driver-api/firmware/fallback-mechanisms.rst
> >
> > Refer to the section "Types of fallback mechanisms", augument the list there
> > and then move the section "Firmware sysfs loading facility" to a new file,
> > and
> > then add a new file for your own.
> >
> >> +
> >> +On some devices the system's EFI code / ROM may contain an embedded copy
> >> +of firmware for some of the system's integrated peripheral devices and
> >> +the peripheral's Linux device-driver needs to access this firmware.
> >
> > You in no way indicate this is a just an invented scheme, a custom solution
> > and
> > nothing standard. I realize Ard criticized that the EFI Firmware Volume
> > Protocol
> > is not part of the UEFI spec -- however it is a bit more widely used right?
> > Why can't Linux support it instead?
> >
>
> Most implementations of UEFI are based on PI,
That seems to be the UEFI Platform Initialization specification:
http://www.uefi.org/sites/default/files/resources/PI_Spec_1_6.pdf
> and so it is likely that
> the protocols are available. However, the PI spec does not cover
> firmware blobs,
Indeed, I cannot find anything about it on the PI Spec, but I *can* easily
find a few documents referring to the Firmware Volume Protocol:
http://wiki.phoenix.com/wiki/index.php/EFI_FIRMWARE_VOLUME_PROTOCOL
But this has no references at all...
I see stupid patents over some of this and authentication mechanisms for it:
https://patents.google.com/patent/US20170098084
> and so it is undefined whether such blobs are self
> contained (i.e., in separate files in the firmware volume), statically
> linked into the driver or maybe even encrypted or otherwise
> encapsulated, and the actual loadable image only lives in memory.
Got it, thanks this helps! There are two things then:
1) The "EFI Firmware Volume Protocol" ("FV" for short in your descriptions
below), and whether to support it or not in the future and recommend it
for future use cases.
b) Han's EFI scraper to help support 2 drivers, and whether or not to
recommend it for future use cases.
> Hans's case is the second one, i.e., the firmware is at an arbitrary
> offset in the driver image. Using the FV protocol in this case would
> result in a mix of both approaches: look up the driver file by GUID
> [which could change btw between different versions of the system
> firmware, although this is unlikely] and then still use the prefix/crc
> based approach to sift through the image itself.
Got it. And to be clear its a reversed engineered solution to what
two vendors decided to do.
> But my main objection is simply that from the UEFI forum point of
> view, there is a clear distinction between the OS visible interfaces
> in the UEFI spec and the internal interfaces in the PI spec (which for
> instance are not subject to the same rules when it comes to backward
> compatibility), and so I think we should not depend on PI at all.
Ah I see.
> This
> is all the more important considering that we are trying to encourage
> the creation of other implementations of UEFI that are not based on PI
> (e.g., uboot for arm64 implements the required UEFI interfaces for
> booting the kernel via GRUB), and adding dependencies on PI protocols
> makes that a moving target.
Got it!
> So in my view, we either take a ad-hoc approach which works for the
> few platforms we expect to support, in which case Hans's approach is
> sufficient,
Modulo it needs some work for ARM as it only works for x86 right now ;)
> or we architect it properly, in which case we shouldn't
> depend on PI because it does not belon
Re: [PATCH v3 2/5] efi: Add embedded peripheral firmware support
On Tue, May 08, 2018 at 03:38:05PM +, Luis R. Rodriguez wrote: > On Fri, May 04, 2018 at 12:44:37PM -0700, Martijn Coenen wrote: > > On Wed, Apr 25, 2018 at 10:55 AM, Luis R. Rodriguez > > wrote: > > > Android became the primary user of CONFIG_FW_LOADER_USER_HELPER_FALLBACK. > > > > > > It would be good for us to hear from Android folks if their current use of > > > request_firmware_into_buf() is designed in practice to *never* use the > > > direct > > > filesystem firmware loading interface, and always rely instead on the > > > fallback mechanism. > > > > It's hard to answer this question for Android in general. As far as I > > can tell the reasons we use CONFIG_FW_LOADER_USER_HELPER(_FALLBACK) > > are: > > 1) We have multiple different paths on our devices where firmware can > > be located, and the direct loader only supports one custom path FWIW I'd love to consider patches to address this, if this is something you may find a need for in the future to *avoid* the fallback, however would like a clean solution. > > 2) Most of those paths are not mounted by the time the corresponding > > drivers are loaded, because pretty much all Android kernels today are > > built without module support, and therefore drivers are loaded well > > before the firmware partition is mounted I've given this some more thought and you can address this with initramfs, this is how other Linux distributions are addressing this. One way to address this automatically is to scrape the drivers built-in or needed early on boot in initamfs and if the driver has a MODULE_FIRMWARE() its respective firmware is added to initramfs as well. If you *don't* use initramfs, then yes you can obviously run into issues where your firmware may not be accessible if the driver is somehow loaded early. > > 3) I think we use _FALLBACK because doing this with uevents is just > > the easiest thing to do; our init code has a firmware helper that > > deals with this and searches the paths that we care about > > > > 2) will change at some point, because Android is moving towards a > > model where device-specific peripheral drivers will be loaded as > > modules, and since those modules would likely come from the same > > partition as the firmware, it's possible that the direct load would > > succeed (depending on whether the custom path is configured there or > > not). But I don't think we can rely on the direct loader even in those > > cases, unless we could configure it with multiple custom paths. Using initramfs will help, but because of the custom path needs -- you're right, we don't have anything for that yet, its also a bit unclear if something nice and clean can be drawn up for it. So perhaps dealing with the fallback mechanism is the way to go for this for sure, since we already have support for it. Just keep in mind that the fallback mechanism costs you about ~13436 bytes. So, if someone comes up with a clean interface for custom paths I'd love to consider it to avoid those 13436 bytes. Luis -- 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 2/5] efi: Add embedded peripheral firmware support
On Fri, May 04, 2018 at 12:44:37PM -0700, Martijn Coenen wrote: > On Wed, Apr 25, 2018 at 10:55 AM, Luis R. Rodriguez wrote: > > Android became the primary user of CONFIG_FW_LOADER_USER_HELPER_FALLBACK. > > > > It would be good for us to hear from Android folks if their current use of > > request_firmware_into_buf() is designed in practice to *never* use the > > direct > > filesystem firmware loading interface, and always rely instead on the > > fallback mechanism. > > It's hard to answer this question for Android in general. As far as I > can tell the reasons we use CONFIG_FW_LOADER_USER_HELPER(_FALLBACK) > are: > 1) We have multiple different paths on our devices where firmware can > be located, and the direct loader only supports one custom path > 2) Most of those paths are not mounted by the time the corresponding > drivers are loaded, because pretty much all Android kernels today are > built without module support, and therefore drivers are loaded well > before the firmware partition is mounted > 3) I think we use _FALLBACK because doing this with uevents is just > the easiest thing to do; our init code has a firmware helper that > deals with this and searches the paths that we care about > > 2) will change at some point, because Android is moving towards a > model where device-specific peripheral drivers will be loaded as > modules, and since those modules would likely come from the same > partition as the firmware, it's possible that the direct load would > succeed (depending on whether the custom path is configured there or > not). But I don't think we can rely on the direct loader even in those > cases, unless we could configure it with multiple custom paths. > > I have no reason to believe request_firmware_into_buf() is special in > this regard; drivers that depend on it may have their corresponding > firmware in different locations, so just depending on the direct > loader would not be good enough. Thanks! This is very useful! This provides yet-another justification and use case to document for the fallback mechanism. I'll go and extend it. > > > > Is ptr below > > > > ret = request_firmware_into_buf(&seg_fw, fw_name, dev, > > ptr, phdr->p_filesz); > > > > Also part of the DMA buffer allocated earlier via: > > > > ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size); > > > > Android folks? > > I think the Qualcomm folks owning this (Andy, David, Bjorn, already > cc'd here) are better suited to answer that question. Andy, David, Bjorn? Luis -- 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 2/5] efi: Add embedded peripheral firmware support
Android folks, poke below. otherwise we'll have no option but to seriously consider Mimi's patch to prevent these calls when IMA appraisal is enforced: http://lkml.kernel.org/r/1525182503-13849-7-git-send-email-zo...@linux.vnet.ibm.com Please read below On Wed, Apr 25, 2018 at 05:55:57PM +0000, Luis R. Rodriguez wrote: > On Wed, Apr 25, 2018 at 01:00:09AM -0400, Mimi Zohar wrote: > > On Tue, 2018-04-24 at 23:42 +, Luis R. Rodriguez wrote: > > > On Tue, Apr 24, 2018 at 12:07:01PM -0400, Mimi Zohar wrote: > > > > On Tue, 2018-04-24 at 17:09 +0200, Hans de Goede wrote: > > > If its of any help -- > > > > > > drivers/soc/qcom/mdt_loader.c is the only driver currently using > > > request_firmware_into_buf() however I'll note qcom_mdt_load() is used in > > > many > > > other drivers so they are wrappers around request_firmware_into_buf(): > > > > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c: * adreno_request_fw() handles > > > this, but qcom_mdt_load() does > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c: ret = qcom_mdt_load(dev, > > > fw, fwname, GPU_PAS_ID, > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c: ret = qcom_mdt_load(dev, > > > fw, newname, GPU_PAS_ID, > > > drivers/media/platform/qcom/venus/firmware.c: ret = qcom_mdt_load(dev, > > > mdt, fwname, VENUS_PAS_ID, mem_va, mem_phys, > > > drivers/remoteproc/qcom_adsp_pil.c: return qcom_mdt_load(adsp->dev, > > > fw, rproc->firmware, adsp->pas_id, > > > drivers/remoteproc/qcom_wcnss.c:return qcom_mdt_load(wcnss->dev, > > > fw, rproc->firmware, WCNSS_PAS_ID, > > > > > > > > As such the current IMA code (from v4.17-rc2) actually does > > > > > not handle READING_FIRMWARE_PREALLOC_BUFFER at all, > > > > > > > > Right, it doesn't yet address READING_FIRMWARE_PREALLOC_BUFFER, but > > > > should. > > > > > > > > Depending on whether the device requesting the firmware has access to > > > > the DMA memory, before the signature verification, > > > > > > It would seem from the original patch review about > > > READING_FIRMWARE_PREALLOC_BUFFER > > > that this is not a DMA buffer. To be very clear I believe Stephen implied this was not DMA buffer. Mimi asked for READING_FIRMWARE_DMA if it was: https://patchwork.kernel.org/patch/9074611/ > > The call sequence: > > qcom_mdt_load() -> qcom_scm_pas_init_image() -> dma_alloc_coherent() > > > > If dma_alloc_coherent() isn't allocating a DMA buffer, then the > > function name is misleading/confusing. > > Hah, by *definition* the device *and* processor has immediate access > to data written *immediately* when dma_alloc_coherent() is used. From > Documentation/DMA-API.txt: > > --- > Part Ia - Using large DMA-coherent buffers > > -- > > > > :: > > > > void * > > dma_alloc_coherent(struct device *dev, size_t size, > >dma_addr_t *dma_handle, gfp_t flag) > > > > Consistent memory is memory for which a write by either the device or > > the processor can immediately be read by the processor or device > > without having to worry about caching effects. (You may however need > > to make sure to flush the processor's write buffers before telling > > devices to read that memory.) > > > Is ptr below > > ret = request_firmware_into_buf(&seg_fw, fw_name, dev, > ptr, phdr->p_filesz); > > Also part of the DMA buffer allocated earlier via: > > ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size); > > > Android folks? Android folks? > > > The device driver should have access to the buffer pointer with write > > > given > > > that wi
Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support
Please Cc andre...@gmail.com on future patches. On Sun, Apr 29, 2018 at 11:35:55AM +0200, Hans de Goede wrote: > Just like with PCI options ROMs, which we save in the setup_efi_pci* > functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself > sometimes may contain data which is useful/necessary for peripheral drivers > to have access to. Interesting. Mimi you may want to look into that for appraisal gap coverage. > Specifically the EFI code may contain an embedded copy of firmware which > needs to be (re)loaded into the peripheral. Normally such firmware would be > part of linux-firmware, but in some cases this is not feasible, for 2 > reasons: This and other reasons have been scattered through mailing lists, and documentation, our Kconfig does not reflect which of the reasons are currently addressed as how. Our goal for your commit log would be to document the precise reason you can't use any of the existing interfaces exposed. Let's see if some of the reasons you state cannot be used with the existing APIs. > 1) The firmware is customized for a specific use-case of the chipset / use > with a specific hardware model, so we cannot have a single firmware file > for the chipset. E.g. touchscreen controller firmwares are compiled > specifically for the hardware model they are used with, as they are > calibrated for a specific model digitizer. In such cases the firmware may be stashed in a separate partition of a custom device and firmwared could be used with a uevent and the fallback mechanism, enabling userspace to do whatever it needs. However in your case the firmware has been found on EFI. > 2) Despite repeated attempts we have failed to get permission to > redistribute the firmware. This is especially a problem with customized > firmwares, these get created by the chip vendor for a specific ODM and the > copyright may partially belong with the ODM, so the chip vendor cannot > give a blanket permission to distribute these. Indeed, we don't have a good way to deal with this now except the above noted firmwared use case, and that still ponies up the redistribution problem up to a system integrator somehow. But also -- your solution is super custom to only two drivers, does not follow a standard of any sort either, and I fear of setting any wrong precedents which will be hard to later not support. Just look at the fallback mechanism and how hairy it got, I've put some love into it recently but it was a pain to even comprehend. Once we add this interface we are going to have to support it for a while so I want to be sure we get the architecture right. Specially if were going to enable *other* vendors to start using such interface. Is your goal to enable or encourage other vendors in similar predicament to use the same strategy? There is nothing wrong in setting de-facto standards for Linux, we do this all the time, but if we are going to do it I want to be sure we document this well and provide proper justifications in the commit log and documentation. > This commit adds support for finding peripheral firmware embedded in the > EFI code Please specify this is through a custom scheme as it reads now it seems to read this is a sort of standard. > diff --git a/Documentation/driver-api/firmware/request_firmware.rst > b/Documentation/driver-api/firmware/request_firmware.rst > index c8bddbdcfd10..560dfed76e38 100644 > --- a/Documentation/driver-api/firmware/request_firmware.rst > +++ b/Documentation/driver-api/firmware/request_firmware.rst > @@ -73,3 +73,69 @@ If something went wrong firmware_request() returns > non-zero and fw_entry > is set to NULL. Once your driver is done with processing the firmware it > can call call firmware_release(fw_entry) to release the firmware image > and any related resource. > + > +EFI embedded firmware support > += This is a new fallback mechanism, please see: Documentation/driver-api/firmware/fallback-mechanisms.rst Refer to the section "Types of fallback mechanisms", augument the list there and then move the section "Firmware sysfs loading facility" to a new file, and then add a new file for your own. > + > +On some devices the system's EFI code / ROM may contain an embedded copy > +of firmware for some of the system's integrated peripheral devices and > +the peripheral's Linux device-driver needs to access this firmware. You in no way indicate this is a just an invented scheme, a custom solution and nothing standard. I realize Ard criticized that the EFI Firmware Volume Protocol is not part of the UEFI spec -- however it is a bit more widely used right? Why can't Linux support it instead? Could your new scheme grow to support the EFI Firmware Volume Protocol rather easily if someone wants to wrap up their sleeves? If so what likely would change? Ensure that the documentation answers the question then, when would folks use the EFI Firmware Volume Protocol? Why can't Linux support it? Why and would u
Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support
On Wed, May 02, 2018 at 04:49:53PM +0200, Hans de Goede wrote:
> Hi,
>
> On 05/01/2018 09:29 PM, Andy Lutomirski wrote:
> > On Sun, Apr 29, 2018 at 2:36 AM Hans de Goede wrote:
> > > +The EFI embedded-fw code works by scanning all EFI_BOOT_SERVICES_CODE
> > memory
> > > +segments for an eight byte sequence matching prefix, if the prefix is
> > found it
> > > +then does a crc32 over length bytes and if that matches makes a copy of
> > length
> > > +bytes and adds that to its list with found firmwares.
> > > +
> >
> > Eww, gross. Is there really no better way to do this?
>
> I'm afraid not.
>
> > Is the issue that
> > the EFI code does not intend to pass the firmware to the OS but that it has
> > a copy for its own purposes and that Linux is just going to hijack EFI's
> > copy? If so, that's brilliant and terrible at the same time.
>
> Yes that is exactly the issue / what it happening here.
>
> >
> > > + for (i = 0; i < size; i += 8) {
> > > + if (*((u64 *)(mem + i)) != *((u64 *)desc->prefix))
> > > + continue;
> > > +
> > > + /* Seed with ~0, invert to match crc32 userspace utility
> > */
> > > + crc = ~crc32(~0, mem + i, desc->length);
> > > + if (crc == desc->crc)
> > > + break;
> > > + }
> >
> > I hate to play the security card, but this stinks a bit. The kernel
> > obviously needs to trust the EFI boot services code since the EFI boot
> > services code is free to modify the kernel image. But your patch is not
> > actually getting this firmware blob from the boot services code via any
> > defined interface -- you're literally snarfing up the blob from a range of
> > memory. I fully expect there to be any number of ways for untrustworthy
> > entities to inject malicious blobs into this memory range on quite a few
> > implementations. For example, there are probably unauthenticated EFI
> > variables and even parts of USB sticks and such that get read into boot
> > services memory, and I see no reason at all to expect that nothing in the
> > so-called "boot services code" range is actually just plain old boot
> > services *heap*.
> >
> > Fortunately, given your design, this is very easy to fix. Just replace
> > CRC32 with SHA-256 or similar. If you find the crypto api too ugly for
> > this purpose, I have patches that only need a small amount of dusting off
> > to give an entirely reasonable SHA-256 API in the kernel.
>
> My main reason for going with crc32 is that the scanning happens before
> the kernel is fully up and running (it happens just before the rest_init()
> call in start_kernel() (from init/main.c) I'm open to using the
> crypto api, but I was not sure if that is ready for use at that time.
Not being sure is different than being certain. As Andy noted, if that does
not work please poke Andy about the SHA-256 API he has which would enable
its use in kernel.
Right now this is just a crazy hack for *2* drivers. Its a lot of hacks for
just that, so no need to rush this in just yet. It seems unclear if we're
all happy with this yet as well.
Luis
> > (To be clear, I don't love my own suggestion here. What I'd *really* like
> > to see is a better interface and no attempt to match the data to some
> > built-in hash at all. In particular, there are plenty of devices for which
> > the driver wants access to a genuinely device-specific blob. For example,
> > I'm typing this email while connected to a router that is running ath10k
> > and is using a calibration blob awkwardly snarfed out of flash somewhere.
> > It would be really nice if there was a way to pull a blob out of EFI space
> > that is marked, by EFI, as belonging to a particular device. Then the
> > firmware could just pass it over without any particular verification. But
> > since your code is literally scanning a wide swath of physical memory for
> > something that looks like a valid blob, I think you need to use a
> > cryptographically strong concept of validity.)
>
> Yes ideally this would not be needed at all and/or use a well defined
> interface, but alas we don't live in an ideal world :)
>
> Regards,
>
> Hans
>
--
Do not panic
--
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 v5 2/5] efi: Add embedded peripheral firmware support
On Tue, May 01, 2018 at 03:27:27PM -0400, Mimi Zohar wrote: > On Tue, 2018-05-01 at 21:11 +0200, Hans de Goede wrote: > > Only the pre hook? I believe the post-hook should still be called too, > > right? So that we've hashes of all loaded firmwares in the IMA core. > > Good catch! Right, if IMA-measurement is enabled, then we would want > to add the measurement. Mimi, just a heads up, we only use the post hook for the syfs fallback mechanism, ie, we don't even use the post hook for direct fs lookup. Do we want that there? Luis -- 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 2/5] efi: Add embedded peripheral firmware support
On Wed, Apr 25, 2018 at 01:00:09AM -0400, Mimi Zohar wrote:
> On Tue, 2018-04-24 at 23:42 +0000, Luis R. Rodriguez wrote:
> > On Tue, Apr 24, 2018 at 12:07:01PM -0400, Mimi Zohar wrote:
> > > On Tue, 2018-04-24 at 17:09 +0200, Hans de Goede wrote:
> > > > On 23-04-18 23:11, Luis R. Rodriguez wrote:
> > > > > Hans, please see use of READING_FIRMWARE_PREALLOC_BUFFER, we'll need
> > > > > a new ID
> > > > > and security for this type of request so IMA can reject it if the
> > > > > policy is
> > > > > configured for it.
> > > >
> > > > Hmm, interesting, actually it seems like the whole existence
> > > > of READING_FIRMWARE_PREALLOC_BUFFER is a mistake,
> >
> > request_firmware_into_buf() was merged without my own review, however,
> > the ID thing did get review from Mimi:
> >
> > https://patchwork.kernel.org/patch/9074611/
> >
> > The ID is not for IMA alone, its for any LSM to decide what to do.
> > Note Mimi asked for READING_FIRMWARE_DMA if such buffer was in DMA,
> > otherise READING_FIRMWARE_PREALLOC_BUFFER was suggested.
> >
> > Mimi why did you want a separate ID for it back before?
>
> The point of commit a098ecd2fa7d ("firmware: support loading into a
> pre-allocated buffer") is to avoid reading the firmware into kernel
> memory and then copying it "to it's final resting place". My concern
> is that if the device driver has access to the buffer, it could access
> the buffer prior to the firmware's signature having been verified by
> the kernel.
If request_firmware_into_buf() is used and the firmware was found in
/lib/firmware/ paths then the driver will *not* use the firmware prior
to any LSM doing any firmware signature verification because
kernel_read_file_from_path() and in turn security_kernel_read_file().
The firmware API has a fallback mechanism [0] though, and if that is used then
security_kernel_post_read_file() is used once the firmware is loaded through
the sysfs interface *prior* to handing the firmware data to the driver. As
Hans noted though security_kernel_post_read_file() currently *only* uses
READING_FIRMWARE, so this needs to be fixed. Also note though that LSMs
get a hint of what is going to happen *soon* prior to the fallback
mechanism kicking on as we travere the /lib/firmware/ paths for direct
filesystem loading.
If this is not sufficient to cover LSM appraisals *one* option could be to
have security_kernel_read_file() return a special error of some sort
for READING_FIRMWARE_PREALLOC_BUFFER so that kernel_read_file_from_path()
users could *know* to fatally give up.
Currently the device drivers using request_firmware_into_buf() can end up
getting the buffer with firmware stashed in it without having the kernel do any
firmware signature verification at all through its LSMs. The LSM hooks added to
the firmware loader long ago by Kees via commit 6593d9245bc66 ("firmware_class:
perform new LSM checks") on v3.17 added an LSM for direct filesystem lookups,
but on the fallback mechanism seems to have only added a post LSM hook
security_kernel_fw_from_file().
There is also a custom fallback mechanism [1] which can be used if the path to
the firmware may be out of the /lib/firmware/ paths or perhaps the firmware
requires some very custom fetching of some sort. The only thing this does
though is just *not* issue a uevent when we don't find the firmware and also
sets the timeout to a practically never-ending value. The custom fallback
mechanism is only usable for request_firmware_nowait() though. In retrospect
the custom fallback mechanism is pure crap and these days we've acknowledged
that even in crazy custom firmware fetching cases folks should be able to
accomplish this by relying on uevents and using the firmwared [2] or forking
it, or a different similar proprietary similar solution, which would just
monitor for uevents for firmware and just Do The Right Thing (TM).
Consider some mobile devices which may want to fetch it from some custom
partition which only it can know how to get.
There is a kernel config option which enables the fallback mechanism always,
This is now easily readable as follows:
drivers/base/firmware_loader/fallback_table.c
struct firmware_fallback_config fw_fallback_config = {
.force_sysfs_fallback =
IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
.loading_timeout = 60,
.old_timeout = 60,
};
Even if this is used we always do direct fs lookups first.
Android became the primary user of CONFIG_FW_LOADER_USER_HELPER_FALLBACK.
It would be good for us to hear from Android folks if their current use of
request_firmware_into_buf() is designed in practice to *never* use the direct
filesystem firmware loading interface, and always rely instead on th
Re: [PATCH v3 2/5] efi: Add embedded peripheral firmware support
On Tue, Apr 24, 2018 at 12:07:01PM -0400, Mimi Zohar wrote: > On Tue, 2018-04-24 at 17:09 +0200, Hans de Goede wrote: > > Hi, > > > > On 23-04-18 23:11, Luis R. Rodriguez wrote: > > > Hans, please see use of READING_FIRMWARE_PREALLOC_BUFFER, we'll need a > > > new ID > > > and security for this type of request so IMA can reject it if the policy > > > is > > > configured for it. > > > > Hmm, interesting, actually it seems like the whole existence > > of READING_FIRMWARE_PREALLOC_BUFFER is a mistake, request_firmware_into_buf() was merged without my own review, however, the ID thing did get review from Mimi: https://patchwork.kernel.org/patch/9074611/ The ID is not for IMA alone, its for any LSM to decide what to do. Note Mimi asked for READING_FIRMWARE_DMA if such buffer was in DMA, otherise READING_FIRMWARE_PREALLOC_BUFFER was suggested. > > the IMA > > framework really does not care if we are loading the firmware > > into memory allocated by the firmware-loader code, or into > > memory allocated by the device-driver requesting the firmware. That's up to LSM folks to decide. We have these so far: #define __kernel_read_file_id(id) \ id(UNKNOWN, unknown)\ id(FIRMWARE, firmware) \ id(FIRMWARE_PREALLOC_BUFFER, firmware) \ id(MODULE, kernel-module) \ id(KEXEC_IMAGE, kexec-image)\ id(KEXEC_INITRAMFS, kexec-initramfs)\ id(POLICY, security-policy) \ id(X509_CERTIFICATE, x509-certificate) \ id(MAX_ID, ) The first type of IDs added was about type of files the kernel LSMs may want to do different things for. Mimi why did you want a separate ID for it back before? I should note now that request_firmware_into_buf() and its READING_FIRMWARE_PREALLOC_BUFFER was to enable a driver on memory constrained devices. The files are large (commit says 16 MiB). I've heard of larger possible files with remoteproc and with Android using the custom fallback mechanism -- which could mean a proprietary tool fetching firmware from a random special place on a device. I could perhaps imagine an LSM which may be aware of such type of arrangement may want to do its own vetting of some sort, but this would not be specific to READING_FIRMWARE_PREALLOC_BUFFER, but rather the custom fallback mechaism. Whether or not the buffer was preallocated by the driver seems a little odd for security folks to do something different with it. Security LSM folks please chime in. I could see a bit more of a use case for an ID for firmware scraped from EFI, which Hans' patch will provide. But that *also* should get good review from other LSM folks. One of the issues with accepting more IDs loosely is where do we stop though? If no one really is using READING_FIRMWARE_PREALLOC_BUFFER I'd say lets remove it. Likewise, for this EFI thing I'd like an idea if we really are going to have users for it. If its of any help -- drivers/soc/qcom/mdt_loader.c is the only driver currently using request_firmware_into_buf() however I'll note qcom_mdt_load() is used in many other drivers so they are wrappers around request_firmware_into_buf(): drivers/gpu/drm/msm/adreno/a5xx_gpu.c: * adreno_request_fw() handles this, but qcom_mdt_load() does drivers/gpu/drm/msm/adreno/a5xx_gpu.c: ret = qcom_mdt_load(dev, fw, fwname, GPU_PAS_ID, drivers/gpu/drm/msm/adreno/a5xx_gpu.c: ret = qcom_mdt_load(dev, fw, newname, GPU_PAS_ID, drivers/media/platform/qcom/venus/firmware.c: ret = qcom_mdt_load(dev, mdt, fwname, VENUS_PAS_ID, mem_va, mem_phys, drivers/remoteproc/qcom_adsp_pil.c: return qcom_mdt_load(adsp->dev, fw, rproc->firmware, adsp->pas_id, drivers/remoteproc/qcom_wcnss.c:return qcom_mdt_load(wcnss->dev, fw, rproc->firmware, WCNSS_PAS_ID, Are we going to add more IDs for more types of firmware? What type of *different* decisions could LSMs take if the firmware was being written to a buffer? Or in this new case that is coming up, if the file came scraping EFI, would having that information be useful? > > As such the current IMA code (from v4.17-rc2) actually does > > not handle READING_FIRMWARE_PREALLOC_BUFFER at all, > > Right, it doesn't yet address READING_FIRMWARE_PREALLOC_BUFFER, but > should. > > Depending on whether the device requesting the firmware has access to > the DMA memory, before the signature verification, It would seem from the original patch review about READING_FIRMWARE_
Re: [PATCH v3 2/5] efi: Add embedded peripheral firmware support
Hans, please see use of READING_FIRMWARE_PREALLOC_BUFFER, we'll need a new ID and security for this type of request so IMA can reject it if the policy is configured for it. Please Cc Kees in future patches. Luis -- 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 2/5] efi: Add embedded peripheral firmware support
On Sun, Apr 08, 2018 at 07:40:11PM +0200, Hans de Goede wrote:
> static void firmware_free_data(const struct firmware *fw)
> {
> @@ -576,6 +600,15 @@ _request_firmware(const struct firmware **firmware_p,
> const char *name,
> goto out;
>
> ret = fw_get_filesystem_firmware(device, fw->priv);
> +#ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
> + if (ret && device &&
> + device_property_read_bool(device, "efi-embedded-firmware")) {
> + ret = fw_get_efi_embedded_fw(device, fw->priv, ret);
> + if (ret == 0)
> + ret = assign_fw(fw, device, opt_flags | FW_OPT_NOCACHE);
> + goto out;
> + }
> +#endif
You mussed what I asked for in terms of adding a new flag, (please work on top
of Andre's patches as those likely will be merged first, and also have kdocs
for the flags) and then a new firmware API to wrap the above into a function
which would only do something if the driver *asked* for it on their firmware
API call. Ie, please add a new firmware_request_efi_fw(). Also if you see the
work I've done to remove the ifdefs over fallback mechanism you'll see it helps
split code and make it easier to read. We should strive to not add any more
ifdefery and instead make tehis code read easily.
Luis
--
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 2/2] efi: Add embedded peripheral firmware support
On Thu, Apr 05, 2018 at 07:43:49AM +0200, Lukas Wunner wrote: > On Wed, Apr 04, 2018 at 01:18:36PM -0400, Peter Jones wrote: > > > On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote: > > > > * Add the EFI Firmware Volume Protocol to include/linux/efi.h: > > > > > > > > https://www.intel.com/content/dam/doc/reference-guide/efi-firmware-file-volume-specification.pdf > > > > > > > > * Amend arch/x86/boot/compressed/eboot.c to read the files with the > > > > GUIDs you're interested in into memory and pass the files to the > > > > kernel as setup_data payloads. > > > > To be honest, I'm a bit skeptical about the firmware volume approach. > > Tools like UEFITool[0] and uefi-firmware-parser[1] have existed for > > years, still don't seem to reliably parse firmware images I see in the > > wild, and have a fairly regular need for fixes. These are tools > > maintained by smart people who are making a real effort, and it still > > looks pretty hard to do a good job that applies across a lot of > > platforms. > > > > So I'd rather use Hans's existing patches, at least for now, and if > > someone is interested in hacking on making an efi firmware volume parser > > for the kernel, switch them to that when such a thing is ready. > > Hello? As I've written in the above-quoted e-mail the kernel should > read the files using EFI_FIRMWARE_VOLUME_PROTOCOL.ReadFile(). > > *Not* by parsing the firmware volume! > > Parsing the firmware volume is only necessary to find out the GUIDs > of the files you're looking for. You only do that *once*. How do you get the GUIDs for each driver BTW? Hans, I do believe we should *try* this approach at the very least. Why not? Otherwise it would be wise to provide a technical reason for why we'd choose one custom mechanism which would only serve a few tablets over a mechanism which could serve more devices. Luis -- 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 2/2] efi: Add embedded peripheral firmware support
On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote: > On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: > > I asked Peter Jones for suggestions how to extract this during boot and > > he suggested seeing if there was a copy of the firmware in the > > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. > > > > My patch to add support for this contains a table of device-model (dmi > > strings), firmware header (first 64 bits), length and crc32 and then if > > we boot on a device-model which is in the table the code scans the > > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and > > caches the firmware for later use by request-firmware. > > > > So I just do a brute-force search for the firmware, this really is hack, > > nothing standard about it I'm afraid. But it works on 4 different x86 > > tablets I have and makes the touchscreen work OOTB on them, so I believe > > it is a worthwhile hack to have. > > The EFI Firmware Volume contains a kind of filesystem with files > identified by GUIDs. Those files include EFI drivers, ACPI tables, > DMI data and so on. It is actually quite common for vendors to > also include device firmware on the Firmware Volume. Apple is doing > this to ship firmware updates e.g. for the GMUX controller found on > dual GPU MacBook Pros. If they want to update the controller's > firmware, they include it in a BIOS update, and an EFI driver checks > on boot if the firmware update for the controller is necessary and > if so, flashes it. > > The firmware files you're looking for are almost certainly included > on the Firmware Volume as individual files. What Hans implemented seems to have been for a specific x86 hack, best if we confirm if indeed they are present on the Firmware Volume. > Rather than scraping > the EFI memory for firmware, I think it would be cleaner and more > elegant if you just retrieve the files you're interested in from > the Firmware Volume. > > We're doing something similar with Apple EFI properties, see > 58c5475aba67 and c9cc3aaa0281. > > Basically what you need to do to implement this approach is: > > * Determine the GUIDs used by vendors for the files you're interested > in. Either dump the Firmware Volume or take an EFI update as > shipped by the vendor, then feed it to UEFIExtract: > https://github.com/LongSoft/UEFITool > > * Add the EFI Firmware Volume Protocol to include/linux/efi.h: > > https://www.intel.com/content/dam/doc/reference-guide/efi-firmware-file-volume-specification.pdf > > * Amend arch/x86/boot/compressed/eboot.c to read the files with the > GUIDs you're interested in into memory and pass the files to the > kernel as setup_data payloads. > > * Once the kernel has booted, make the files you've retrieved > available to device drivers as firmware blobs. Happen to know if devices using Firmware Volumes also sign their firmware and if hw checks the firmware at load time? Luis > The end result is mostly the same as what you're doing here, > and you'll also have a similar array of structs, but instead > of hardcoding 8-byte signatures of firmware files, you'll be > using GUIDs. I envision lots of interesting use cases for > a generic Firmware Volume file retrieval mechanism. What you > need to keep in mind though is that this approach only works > if the kernel is booted via the EFI stub. > > Thanks, > > Lukas > -- Do not panic -- 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 2/2] efi: Add embedded peripheral firmware support
In your next patches please Cc the folks I added for future review as well. We don't have a mailing list for the firmware API so I just tend to Cc who I think should help review when needed. On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: > Hi Luis, > > Thank you for the review. > > On 03-04-18 01:23, Luis R. Rodriguez wrote: > > On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote: > > > Just like with PCI options ROMs, which we save in the setup_efi_pci* > > > functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself > > > sometimes may contain data which is useful/necessary for peripheral > > > drivers > > > to have access to. > > > > > > Specifically the EFI code may contain an embedded copy of firmware which > > > needs to be (re)loaded into the peripheral. Normally such firmware would > > > be > > > part of linux-firmware, but in some cases this is not feasible, for 2 > > > reasons: > > > > > > 1) The firmware is customized for a specific use-case of the chipset / use > > > with a specific hardware model, so we cannot have a single firmware file > > > for the chipset. E.g. touchscreen controller firmwares are compiled > > > specifically for the hardware model they are used with, as they are > > > calibrated for a specific model digitizer. > > > > Some devices have OTP and use this sort of calibration data, > > Right, I'm not sure it really is OTP and not flash, but many touchscreen > controllers do come with their firmware embedded into the controller, It varies, sometimes firmware has default fluff calibration data as well which can be used if the device does not have specific calibration data. > but not all unfortunately. Indeed. > > I was unaware of > > the use of EFI to stash firmware. Good to know, but can you also provide > > references to what part of what standard should be followed for it in > > documentation? > > This is not part of the standard. There has been a long(ish) standing issue > with us not being able to get re-distribute permission for the firmware for > some touchscreen controllers found on cheap x86 devices. Which means that > we cannot put it in Linux firmware. BTW do these cheap x86 devices have hw signing support? Just curious thinking long term here. Because of it is not-standard then perhaps wen can partner later up with a vendor to this properly and actually support hw firmware singing. > Dave Olsthoorn (in the Cc) noticed that the touchscreen did work in the > refind bootload UI, so the EFI code must have a copy of the firmware. :) > I asked Peter Jones for suggestions how to extract this during boot and > he suggested seeing if there was a copy of the firmware in the > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. Sneaky Pete, nice. So essentially we're reverse engineering support for this. Anyway please mention that this is not part of standard in the documentation, and we've just found out in practice some vendors are doing this. That would avoid having people ask later. > My patch to add support for this contains a table of device-model (dmi > strings), firmware header (first 64 bits), length and crc32 and then if > we boot on a device-model which is in the table the code scans the > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and > caches the firmware for later use by request-firmware. Neat, best to add proper docs for this. > So I just do a brute-force search for the firmware, this really is hack, > nothing standard about it I'm afraid. But it works on 4 different x86 > tablets I have and makes the touchscreen work OOTB on them, so I believe > it is a worthwhile hack to have. Absolutely, just not to shove an entire fallback firmware path to all users. > > > 2) Despite repeated attempts we have failed to get permission to > > > redistribute the firmware. This is especially a problem with customized > > > firmwares, these get created by the chip vendor for a specific ODM and the > > > copyright may partially belong with the ODM, so the chip vendor cannot > > > give a blanket permission to distribute these. > > > > > > This commit adds support for finding peripheral firmware embedded in the > > > EFI code and making this available to peripheral drivers through the > > > standard firmware loading mechanism. > > > > Neat. > > > > > Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware pretty > > > late in the init sequence, > > > > This also creates a technical limitation on use for the API that users > > should be
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote:
> Just like with PCI options ROMs, which we save in the setup_efi_pci*
> functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself
> sometimes may contain data which is useful/necessary for peripheral drivers
> to have access to.
>
> Specifically the EFI code may contain an embedded copy of firmware which
> needs to be (re)loaded into the peripheral. Normally such firmware would be
> part of linux-firmware, but in some cases this is not feasible, for 2
> reasons:
>
> 1) The firmware is customized for a specific use-case of the chipset / use
> with a specific hardware model, so we cannot have a single firmware file
> for the chipset. E.g. touchscreen controller firmwares are compiled
> specifically for the hardware model they are used with, as they are
> calibrated for a specific model digitizer.
Some devices have OTP and use this sort of calibration data, I was unaware of
the use of EFI to stash firmware. Good to know, but can you also provide
references to what part of what standard should be followed for it in
documentation?
> 2) Despite repeated attempts we have failed to get permission to
> redistribute the firmware. This is especially a problem with customized
> firmwares, these get created by the chip vendor for a specific ODM and the
> copyright may partially belong with the ODM, so the chip vendor cannot
> give a blanket permission to distribute these.
>
> This commit adds support for finding peripheral firmware embedded in the
> EFI code and making this available to peripheral drivers through the
> standard firmware loading mechanism.
Neat.
> Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware pretty
> late in the init sequence,
This also creates a technical limitation on use for the API that users
should be aware of. Its important to document such limitation.
Also if we can address the limitation that would be even better.
For instance, on what part of the driver is the call to request firmware
being made? Note that we support async probe now, so if the call was done
on probe, it may be wise to use async probe, however, can we be *certain*
that the EFI firmware would have been parsed and ready by then? Please
check. It just may be the case.
Or, if we use late_initcall() would that suffice on the driver, if they
used a request firmware call on init or probe?
> this is on purpose because the typical
> EFI_BOOT_SERVICES_CODE memory-segment is too large for early_memremap().
To be clear you neede to use memremap()
What mechanism would have in place to ensure that a driver which expects
firmware to be on EFI data to be already available prior to its driver's
call to initialize?
You seem to say its this consumes about about 25 MiB now, and for now you
have made this a debug thing only? How have these size requirements changed
over time? Has EFI_BOOT_SERVICES_CODE grown over time? How much? Do we
expect it will blow up later?
> This means we rely on the EFI_BOOT_SERVICES_CODE not being free-ed until
> efi_free_boot_services() is called, which means that this will only work
> on x86, if we ever want this on ARM we should make ARM delay the freeing
> of the EFI_BOOT_SERVICES_* memory-segments too.
Why not do that as well with your patch?
> Note this commit also modifies efi_mem_desc_lookup() to not skip
> EFI_BOOT_SERVICES_CODE memory-segments, so that efi_mem_reserve() works
> on such segments.
>
> Reported-by: Dave Olsthoorn
> Suggested-by: Peter Jones
> Signed-off-by: Hans de Goede
> ---
> drivers/base/firmware_class.c| 29 +++
> drivers/firmware/efi/Makefile| 1 +
> drivers/firmware/efi/efi.c | 1 +
> drivers/firmware/efi/embedded-firmware.c | 232 +++
> include/linux/efi.h | 2 +
> init/main.c | 1 +
> 6 files changed, 266 insertions(+)
> create mode 100644 drivers/firmware/efi/embedded-firmware.c
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 7dd36ace6152..b1e7b3de1975 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -12,6 +12,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -1207,6 +1208,32 @@ static inline void unregister_sysfs_loader(void)
>
> #endif /* CONFIG_FW_LOADER_USER_HELPER */
>
> +#ifdef CONFIG_EFI
> +static int
> +fw_get_efi_embedded_fw(struct device *dev, struct fw_priv *fw_priv, int ret)
> +{
> + size_t size;
> + int rc;
> +
> + rc = efi_get_embedded_fw(fw_priv->fw_name, &fw_priv->data, &size,
> + fw_priv->data ? fw_priv->allocated_size : 0);
> + if (rc == 0) {
> + dev_dbg(dev, "using efi-embedded fw %s\n", fw_priv->fw_name);
> + fw_priv->size = size;
> + fw_state_done(fw_priv);
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +#else
Re: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Tue, Dec 05, 2017 at 11:27:58AM +0100, Pavel Machek wrote: > Hi! > > > > Our ability to determine that userland hasn't been tampered with > > > depends on the kernel being trustworthy. If userland can upload > > > arbitrary firmware to DMA-capable devices then we can no longer trust > > > the kernel. So yes, firmware is special. > > > > You're ignoring the whole "firmware is already signed by the hardware > > manufacturer and we don't even have access to it" part. > > Well... I guess we'd prefer the firmware _not_ be signed, so we can > fix security holes in that after the vendor lost interest... Bugs in > the wifi stacks seemed patcheable that way. > > There is GPLed firmware available for some USB wifi's. We really > should make sure firmware signing is not mandatory/encouraged for the hw > vendors. I share this concern and interest, specially as more device drivers get purposely stupid and most of the technology and fun things shift to firmware. Its precisely for this same reason why most manufacturers won't be opening up more firmware, as they could argue tons of value-add is in firmware now. One has to be realistic though, as one of the few folks who pushed to open source (and GPL) a few of the open source firmwares we have for WiFi, I realize we have no option but to accept the fate that most manufacturers *will* use and require signed firmware in the future. If you're concerned about this you have only a few options. One is to get more and more folks reverse engineer firmware. This won't help if you can't deploy unsigned firmware though. But you have to also look at it from a practical point of view, in order to do development you *have* to have some sort of knobs to turn off fw signing verification, so it should be a matter of looking hard. And for devices where this is not obvious or almost impossible, hope for an exploit. Another option is to argue for the engineering gain for having open firmware, and a viable sensible and responsible option to continue to do R&D and innovation with open firmware. We did this with WiFi long ago, see CFG80211_CERTIFICATION_ONUS and the slew of open firmware released. The fact that we have open firmware repositories and also a respective proprietary firmwares for the same device could be used to metrically show the gains that such development had over time, in a quantifiable form (features, bug fixes, security issues fixed). There's tons of data available which could enable a researcher to have a field with this. And the last option is to just argue for a sensible opt-out knob to be documented as part of our rights or due to general long term community security interests. Just as with UEFI one can say one does not trust any key present already on the platform, one should also be able to do the same for peripheral devices and their own corresponding firmware -- a knob to disable firmware signing. Sadly *all* of these are lofty pie in the sky objectives. What the likely outcome will be is we sit and wait for the worst of the possible security shit issues to hit the fan for the large IoT universe, and then hope we can use this as leverage to require documenting such opt-out knobs just as with UEFI's opt-out mechanism. Although one would expect that such exploits already have happened, in practice not at the interface we're talking about here, which is for /lib/firmware/. For instance the Intel Management Engine does not use this interface, and neither do some of the storage driver and arrays I've been bluntly told have had tons of backdoors for years. Its precisely because of this lack of known issues with security for /lib/firmware exploits that I'm willing to put signing of /lib/firmware on the side for now. We just haven't had many known attacks come in through here. Luis -- 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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Mon, Nov 13, 2017 at 09:08:48PM +, Alan Cox wrote: > On Mon, 13 Nov 2017 18:42:50 +0100 > "Luis R. Rodriguez" wrote: > > > On Sat, Nov 11, 2017 at 02:32:40AM +, Alan Cox wrote: > > > > My assumption here is: > > > > 1) there are some less important and so security-insensitive firmwares, > > > >by which I mean that such firmwares won't be expected to be signed in > > > >terms of vulnerability or integrity. > > > >(I can't give you examples though.) > > > > 2) firmware's signature will be presented separately from the firmware > > > >blob itself. Say, "firmware.bin.p7s" for "firmware.bin" > > > > > > For x86 at least any firmware on any system modern enough to support > > > 'secure' boot should already be signed. The only major exception is > > > likely to be for things like random USB widgets. > > > > Alan, the firmware being considered here is /lib/firmware firmware, which > > is not signed today. Its unclear to me how you mean that /lib/firmware files > > are already signed or verified today > > By the hardware they get loaded onto. Pretty much all of the hardware on > PC class systems at least is signed and checked by the platform or the > device itself. For a 'secure' boot era PC that's pretty much everything > except USB toys and FPGA. > > So you don't actually need to sign a lot of PC class firmware because > it's already signed. It would seem that this logic can trump the need for /lib/firmware signing. And for users where this is *not done* they can open code it, just as is being done for cfg80211 to help a new kernel config replace CRDA. Additionally for non-UEFI platforms which *also* need firmware signing, one viable option is to rely on IMA. If we have enough open-coded uses similar to cfg80211 we can revisit this question later. I am curious though, is the above notion of having hardware require signed firmware an implication brought down by UEFI? If so do you have any pointers to where this is stipulated? Or is it just a best practice we assume some manufacturers are implementing? Luis -- 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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Wed, Nov 15, 2017 at 02:56:57PM -0500, Mimi Zohar wrote:
> On Wed, 2017-11-15 at 18:52 +0100, Luis R. Rodriguez wrote:
> > On Wed, Nov 15, 2017 at 06:49:57AM -0500, Mimi Zohar wrote:
> > > On Tue, 2017-11-14 at 21:50 +0100, Luis R. Rodriguez wrote:
> > >
> > > > Johannes made cfg80211 recently just use request_firmware() now via
> > > > commit on
> > > > linux-next 90a53e4432 ("cfg80211: implement regdb signature checking")
> > > > [0] as
> > > > he got tired of waiting firmware signing, but note he implemented a
> > > > signature
> > > > checking on its own so he open codes verify_pkcs7_signature() after the
> > > > request_firmware() call. If we are happy to live with this, then so be
> > > > it.
> > > >
> > > > [0]
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=90a53e4432b12288316efaa5f308adafb8d304b0
> > >
> > > Johannes was tired of waiting? Commit 5a9196d "ima: add support for
> > > measuring and appraising firmware" has been in the kernel since linux-
> > > 3.17.
> > >
> > > The original firmware hook for verifying firmware signatures were
> > > replaced with the common LSM pre and post kernel_read_file() hooks
> > > in linux-4.6.y.
> > >
> > > Even if you wanted to support firmware signature verification without
> > > IMA-appraisal, it should be using the LSM hooks.
> >
> > request_firmware() uses kernel_read_file_from_path() underneath the hood,
> > and so its used for both:
> >
> > /lib/firmware/regulatory.db
> > /lib/firmware/regulatory.db.p7s
>
> The firmware signature validation should occur as part of
> kernel_read_file_from_path(), not as a stand alone verification.
>
> Why not extend kernel_read_file_from_path() to pass the detached signature?
> Since the signature would only be used for the verification, there's no need
> to return the open file descriptor.
This goes along with the question if there were an other users who wanted it,
or more importantly -- if firmware signing was desirable for any reason, a
modified kernel_read_file_from_path_signed() could in turn be used, *or* an LSM
added to handle READING_FIRMWARE and READING_FIRMWARE_PREALLOC_BUFFER. The
above use case was one example outside of the typical firmware use. I've long
pointed out that we no longer use the firmware API for just firmware, and the
above is now a very good example of it. I've been suggesting uses of the
firmware API for non-firmware had already happened and that more uses were on
its way. Trusted boot has nothing to do with these uses as such the gains of
systems pegged with "trusted boot" have nothing to do validation of these files
through hardware.
In this particular case its simply easier to re-use the existing redistribution
mechanisms in place already so that users can get the regulatory.db and
regulatory.db.p7s.
So here is now an example use of request_firmware() API with its own key
validation / keyring solution completely open coded.
So, given the wireless regulatory use case *does* benefit from re-using
request_firmware() due to the file redistribution aspects, using a new
kernel_read_file_from_path_signed() for non-firmware with the same
/lib/firmware/ path specified would be rather sloppy and awkward. The right
way to not open code this is to consider adding firmware signing support
properly.
> > The later only if CONFIG_CFG80211_REQUIRE_SIGNED_REGDB, which defaults
> > to y anyway.
> >
> > What I meant was that net/wireless/reg.c now open codes firmware signature
> > validation on its own rather than using a helper. IMA appraisal will still
> > be used if enabled given kernel_read_file_from_path() is used.
> >
> > The open coding of the firmware signature check is what I wanted to
> > highlight.
>
> How are the keys in the CFG80211_EXTRA_REGDB_KEYDIR verified? The
> call to key_create_or_update() with the KEY_ALLOC_BYPASS_RESTRICTION
> option by-passes any requirement that the keys in this directory are
> signed. This by-passes the concept of extending the secure boot
> signature chain of trust. To safely validate the keys use the
> restrict_link_by_builtin_trusted option.
I'll let Johannes chime in, if he so wishes.
Luis
--
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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Wed, Nov 15, 2017 at 06:49:57AM -0500, Mimi Zohar wrote:
> On Tue, 2017-11-14 at 21:50 +0100, Luis R. Rodriguez wrote:
>
> > Johannes made cfg80211 recently just use request_firmware() now via commit
> > on
> > linux-next 90a53e4432 ("cfg80211: implement regdb signature checking") [0]
> > as
> > he got tired of waiting firmware signing, but note he implemented a
> > signature
> > checking on its own so he open codes verify_pkcs7_signature() after the
> > request_firmware() call. If we are happy to live with this, then so be it.
> >
> > [0]
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=90a53e4432b12288316efaa5f308adafb8d304b0
>
> Johannes was tired of waiting? Commit 5a9196d "ima: add support for
> measuring and appraising firmware" has been in the kernel since linux-
> 3.17.
>
> The original firmware hook for verifying firmware signatures were
> replaced with the common LSM pre and post kernel_read_file() hooks
> in linux-4.6.y.
>
> Even if you wanted to support firmware signature verification without
> IMA-appraisal, it should be using the LSM hooks.
request_firmware() uses kernel_read_file_from_path() underneath the hood,
and so its used for both:
/lib/firmware/regulatory.db
/lib/firmware/regulatory.db.p7s
The later only if CONFIG_CFG80211_REQUIRE_SIGNED_REGDB, which defaults
to y anyway.
What I meant was that net/wireless/reg.c now open codes firmware signature
validation on its own rather than using a helper. IMA appraisal will still
be used if enabled given kernel_read_file_from_path() is used.
The open coding of the firmware signature check is what I wanted to highlight.
Luis
--
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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Tue, Nov 14, 2017 at 12:18:54PM -0800, Linus Torvalds wrote:
> This is all theoretical security masturbation. The _real_ attacks have
> been elsewhere.
In my research on this front I'll have to agree with this, in terms of
justification and there are only *two* arguments which I've so far have found
to justify firmware signing:
a) If you want signed modules, you therefore should want signed firmware.
This however seems to be solved by using trusted boot thing, given it
seems trusted boot requires having firmware be signed as well. (Docs
would be useful to get about where in the specs this is mandated,
anyone?). Are there platforms that don't have trusted boot or for which
they don't enforce hardware checking for signed firmware for which
we still want to support firmware signing for? Are there platforms
that require and use module signing but don't and won't have a trusted
boot of some sort? Do we care?
b) Other kernel components have implemented some sort of "signing" a file
prior to sending it to the kernel. One is wifi for the regulatory.bin,
through CRDA, and this helped us persuade certain folks to support wifi
with open drivers. We can easily replace CRDA with kernel fetch for a
signed file given most of the APIs for this is available.
Originally effort a) was done for b), as it was believed we needed a).
If we are saying a) is pointless, what about b)? A simple new
kernel_read_file_from_path_pkcs7_signed() or some such would suffice.
Johannes made cfg80211 recently just use request_firmware() now via commit on
linux-next 90a53e4432 ("cfg80211: implement regdb signature checking") [0] as
he got tired of waiting firmware signing, but note he implemented a signature
checking on its own so he open codes verify_pkcs7_signature() after the
request_firmware() call. If we are happy to live with this, then so be it.
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=90a53e4432b12288316efaa5f308adafb8d304b0
Luis
--
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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Mon, Nov 13, 2017 at 07:50:35PM +0100, Luis R. Rodriguez wrote: > On Fri, Nov 10, 2017 at 08:45:06AM -0500, Mimi Zohar wrote: > It does not mean we don't have to support hashes from the start, we can, > however that could require a driver change where its hash is specified or > preferred, for instance. Actually the pseudo code I just demo'd on your RFC proposal shows how we could support the hashes for firmware an optional first policy and if that fails check the fw signature if present. So no driver changes would be needed other than key'ing a respective hash for the firmware, which can just be a macro driver addition, not an API call change. Luis -- 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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Fri, Nov 10, 2017 at 08:45:06AM -0500, Mimi Zohar wrote:
> On Fri, 2017-11-10 at 02:46 +0100, Luis R. Rodriguez wrote:
> > On Thu, Nov 09, 2017 at 10:48:43AM +0900, AKASHI, Takahiro wrote:
> > > On Wed, Nov 08, 2017 at 08:46:26PM +0100, Luis R. Rodriguez wrote:
> > > > But perhaps I'm not understanding the issue well, let me know.
> > >
> > > My point is quite simple:
> > > my_deviceA_init() {
> > > err = request_firmware(&fw, "deviceA"); <--- (a)
> > > if (err)
> > > goto err_request;
> > >
> > > err = verify_firmware(fw); <--- (b)
> > > if (err)
> > > goto err_verify;
> > >
> > > load_fw_to_deviceA(fw); <--- (c)
> > > ...
> > > }
> > >
> > > As legacy device drivers does not have (b), there is no chance to
> > > prevent loading a firmware at (c) for locked-down kernel.
> >
> > Ah, I think your example requires another piece of code to make it clearer.
> > Here is an example legacy driver:
> >
> > my_legacy_deviceB_init() {
> > err = request_firmware(&fw, "deviceB"); <--- (a)
> > if (err)
> > goto err_request;
> >
> > load_fw_to_deviceA(fw); <--- (c)
> > ...
> > }
> >
> > There is no verify_firmware() call here, and as such the approach Linus
> > suggested a while ago cannot possibly fail on a "locked down kernel", unless
> > *very* legacy API call gets a verify_firmware() sprinkled.
> >
> > One sensible thing to say here is then that all request_firmware() calls
> > should
> > just fail on a "locked down kernel", however if this were true then even
> > calls
> > which *did* issue a subsequent verify_firmware() would fail earlier
> > therefore
> > making verify_firmware() pointless on new drivers.
>
> As long as these "*very* legacy API calls", are calling
> kernel_read_file_from_path() to read the firmware, there shouldn't be
> a problem.
For the *default* validation approach, agreed that we can LSMify this now
through
kernel_read_file() by checking the enum kernel_read_file_id for
READING_FIRMWARE,
as you did with your proposed LSM hook fw_lockdown_read_file(), but *iff* we
agree on which mechanisms to support for the default validation approach.
I say support given LSM'ifying this means anyone can end up trying to scheme up
a kernel solution which is non-IMA appraisals. For instance, based on
discussions so far, perhaps a sensible initial *default* scheme to strive for
firmware which we may not get a vendor to sign for us, or for which we don't
have a key to trust for singing could be to use a simple hash of files. This
makes sense for firmware which is typically not updated regularly and for which
we lost a source of updates for.
There's still a similar catch-22 issue here though:
A few folks have argued that we should support custom key requirements
from the start, so we can support a vendor wishing to use their own keys
from the start. Using a specific key is much more efficient for device
drivers which for instance may have firmware updated much more regularly,
this way a hash update on the kernel is not needed per firmware update.
Even if we support both schemes from the start, supporting a "default"
scheme without having any drivers update implies *all* firmware API calls
would need to fit the default policy scheme initially supported. So if
hashes of files is what would be used to start off with we'd be forcing all
firmware API kernel calls to follow that scheme from the start and I don't
think that makes much sense.
Despite some folks not liking it, supporting an initial default scheme just be
to be firmware signed by the linux-firmware maintainer might be an easier first
optional policy to support.
It does not mean we don't have to support hashes from the start, we can,
however that could require a driver change where its hash is specified or
preferred, for instance.
Likewise, for alternative and custom signing requirements, we also can require
a different API call, but for this to work well, I think we'd need a different
firmware API call and an LSM hook other than kernel_read_file_from_path() which
can enable the custom requirements to be specified. Otherwise we run into
similar issue as Takahiro pointed out with verify_firmware(), where the default
policy (say a default key) would not have a match from the start so it would
fail from the start. There is also the technical limitation that
kernel_read_file_from_path() has no way of letting callers pass further
security criteria.
Luis
--
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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Sat, Nov 11, 2017 at 02:32:40AM +, Alan Cox wrote: > > My assumption here is: > > 1) there are some less important and so security-insensitive firmwares, > >by which I mean that such firmwares won't be expected to be signed in > >terms of vulnerability or integrity. > >(I can't give you examples though.) > > 2) firmware's signature will be presented separately from the firmware > >blob itself. Say, "firmware.bin.p7s" for "firmware.bin" > > For x86 at least any firmware on any system modern enough to support > 'secure' boot should already be signed. The only major exception is > likely to be for things like random USB widgets. Alan, the firmware being considered here is /lib/firmware firmware, which is not signed today. Its unclear to me how you mean that /lib/firmware files are already signed or verified today. > The other usual exception is FPGAs, but since the point of an FPGA is > usually the fact it *can* be reprogrammed it's not clear that signing > FPGA firmware makes sense unless it is designed to be fixed function. Agreed that we may end up with exemptions where we purposely cannot get signed firmware for, or where for certain drivers this makes no sense. Luis -- 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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Thu, Nov 09, 2017 at 10:48:43AM +0900, AKASHI, Takahiro wrote:
> On Wed, Nov 08, 2017 at 08:46:26PM +0100, Luis R. Rodriguez wrote:
> > But perhaps I'm not understanding the issue well, let me know.
>
> My point is quite simple:
> my_deviceA_init() {
> err = request_firmware(&fw, "deviceA"); <--- (a)
> if (err)
> goto err_request;
>
> err = verify_firmware(fw); <--- (b)
> if (err)
> goto err_verify;
>
> load_fw_to_deviceA(fw); <--- (c)
> ...
> }
>
> As legacy device drivers does not have (b), there is no chance to
> prevent loading a firmware at (c) for locked-down kernel.
Ah, I think your example requires another piece of code to make it clearer.
Here is an example legacy driver:
my_legacy_deviceB_init() {
err = request_firmware(&fw, "deviceB"); <--- (a)
if (err)
goto err_request;
load_fw_to_deviceA(fw); <--- (c)
...
}
There is no verify_firmware() call here, and as such the approach Linus
suggested a while ago cannot possibly fail on a "locked down kernel", unless
*very* legacy API call gets a verify_firmware() sprinkled.
One sensible thing to say here is then that all request_firmware() calls should
just fail on a "locked down kernel", however if this were true then even calls
which *did* issue a subsequent verify_firmware() would fail earlier therefore
making verify_firmware() pointless on new drivers.
Is that what you meant?
A long time ago we discussed similar default mechanisms, so its worth
re-iterating conclusions done before. The only difference with our current
discussion is that the latest proposed mechanism by Linus for firmware signing
was to consider a verify_firmware() so we can upkeep a functional API approach
to adding support for firmware signing.
What you are asking is what are default mechanism should be, and how do we
*ensure* this works using the functional API approach. We had ironed out
what the default mechanism should have been a while ago, not so much the
later as the firmware API has been in a state of flux. I'll provide pointers
to discussions around the default policy so that none of this is lost and
so that how we end up doing things is well understood later. We can try
to also hash out how to make this work then.
David Woodhouse has long recommended that a 'defult key' should only be last
resort [0], on that same email he also had suggested that since we could end up
supporting different signing schemes it may be best to support cryptographic
requirements (cert identifier or hash) from the start. James Bottomley has
recommended that the default behaviour can only be left up to "system policy".
A "system policy" is exactly what "kernel lockdown" is. David's email contained
a man page which stipulated that the policy that this type of system would
follow
*is* to reject unsigned firmware. He and others may want to review if they
indeed
wanted to follow this path in lieu of this email.
Even if one is not using a "kernel lockdown" system policy, we can consider the
old schemes discussed for firmware singing. In 2015 I first proposed a simple
signing scheme firmware signing which mimics the Linux module signing scheme
[2]. The issues with "purpose of a key" was well hashed out (a firmware key
should only work for firmware and only for the firmware it was designed for,
etc) and later iterations considered this thanks to Andy.
In this scheme, the default system policy depended on what kernel configuration
option your system had been built with, and matching the module signing scheme
we had:
o CONFIG_FIRMWARE_SIG - "Firmware signature verification"
o CONFIG_FIRMWARE_SIG_FORCE - "Require all firmware to be validly
signed"
Everything about signing was done automatically behind the schemes for us, just
as with module signing.
If CONFIG_FIRMWARE_SIG_FORCE was *not* enabled, we'd grant unsigned firmwares
to be loaded, we'd just pr_warn() about them. Note that contrary to module
signing, firmware signing could not taint for technical reasons I listed on my
follow up patches after New Mexico Linux Plumbers [3]. Its worth re-iterating
so its not lost.
add_taint_module() is currently not exported, that can be fixed easily, but
also, the old firmware API currently does not associate the caller module in the
code, so we have a few options as to how to taint a kernel with unsigned
firmware,
which I listed but I'll list here again:
1) Extend the firmware_class driver API to require the correct module to
always be set.
2) Use add_taint_module() with the firmware_class module for the two
synchrounous APIs, while using the right module for the async call.
3) Use the firmwa
Re: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Wed, Nov 08, 2017 at 03:01:09PM -0500, Mimi Zohar wrote: > > > > Or reflect that IMA-appraisal, if enabled, will enforce firmware being > > > validly signed. > > > > But FWICT lockdown is a built-in kernel thingy, unless lockdown implies IMA > > it would not be the place to refer to it. > > > > It seems the documentation was proposed to help users if an error was > > caught. > > That error should cover only what is being addressed in code on the kernel. > > Enabling "lockdown" needs to take into account IMA-appraisal to > prevent breaking systems with it enabled. > > An IMA builtin "secure_boot" policy was already upstreamed (commit > 503ceaef8e2e "ima: define a set of appraisal rules requiring file > signatures"). An additional patch, automatically enables the > "secure_boot" policy in "lockdown" mode. > > Refer to this discussion and patch: > http://kernsec.org/pipermail/linux-security-module-archive/2017-October/003913.html > http://kernsec.org/pipermail/linux-security-module-archive/2017-October/003910.html Ah then yeah this makes sense to mesh into the lock down documentation. Luis -- 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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Wed, Nov 08, 2017 at 03:15:54PM +0900, AKASHI, Takahiro wrote: > Luis, > > Thank you for this heads-up. > > On Wed, Nov 08, 2017 at 12:07:00AM +0100, Luis R. Rodriguez wrote: > > On Thu, Nov 02, 2017 at 06:10:41PM -0400, Mimi Zohar wrote: > > > On Thu, 2017-11-02 at 22:04 +, David Howells wrote: > > > > Mimi Zohar wrote: > > > > > > > > > > Only validly signed device firmware may be loaded. > > > > > > > > > > fw_get_filesystem_firmware() calls kernel_read_file_from_path() to > > > > > read the firmware, which calls into the security hooks. Is there > > > > > another place that validates the firmware signatures. I'm not seeing > > > > > which patch requires firmware to be signed? > > > > > > > > Luis has a set of patches for this. However, I'm not sure if that's > > > > going > > > > anywhere at the moment. Possibly I should remove this from the manpage > > > > for > > > > the moment. > > > > Remove it for now. The state of of affairs for firmware signing is complex > > given > > that we first wanted to address how to properly grow the API without making > > the API worse. This in and of itself was an effort, and that effort also > > evaluated two different development paradigms: > > > > o functional API > > o data driven API > > > > I only recently was convinced that functional API should be used, even for > > commonly used exported symbols, > > Are you? Yes, this stemmed from the fact that even system calls can be abused through data driven APIs, and that long term a functional API at least can make evolutions much easier to review and bisect. That said this is all based on *empirical hearsay*, and no formal observations. But the ease to more easily bisect long term is enough for me to consider this for the firmware API given subtle regressions have been a long standing pain on the firmware API and I would not want to make the process of bisecting any harder. If anyone *does* have actual efforts which compares and contrasts both, I'd love to get them, to further back my current position, but as-is I'm already sold on functional API driven interface. > I haven't answered Linus' question, but my concern about functional APIs, > as far as firmware signing goes, is that we have no way to _enforce_ > firmware signing to existing (i.e. verification-unaware) drivers if we need > an explicit call of a function, say, verify_firmware(). Your concern seems to be that a functional driven API for firmware signing would implicate having to support verify_firmware() for drivers which *cannot* get signed firmware verified, is that correct? IMHO that should just fail then, ie, a "locked down" kernel should not want to *pass* a firmware signature if such thing could not be done. Its no different than trying to verify a signed module on a "locked down" for which it has no signature. But perhaps I'm not understanding the issue well, let me know. > > and as such I've been going back and slowly > > grooming the firmware API with small atomic changes to first clean up the > > complex flag mess we have. > > > > Since I'm busy with that Takahiro AKASHI has taken up firmware singing > > effort > > but this will depend on the above small cleanup to be done first. I was busy > > with addressing existing bugs on the firmware API for a while, then company > > travel / conferences so was not able to address this, but I'm back now and > > I believe I should be able to tackle the cleanup now. > > Good to hear. > > > Only after this is merged can we expect a final respin of the firmware > > signing > > effort. > > > > > Or reflect that IMA-appraisal, if enabled, will enforce firmware being > > > validly signed. > > > > But FWICT lockdown is a built-in kernel thingy, unless lockdown implies IMA > > it would not be the place to refer to it. > > I think that the situation is the same as in module signing. But by definition a "locked down" kernel will enforce module signing, and it would seem logical that if modules signing is enforced, eventually a statement about firmware signing can be added, as it relates to a kernel enforcement effort, not a kernel-userpace framework such as IMA. Luis > -Takahiro AKASHI > > > It seems the documentation was proposed to help users if an error was > > caught. > > That error should cover only what is being addressed in code on the kernel. > > > > Luis > -- Luis Rodriguez, SUSE LINUX GmbH Maxfeldstrasse 5; D-90409 Nuernberg -- 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: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown
On Thu, Nov 02, 2017 at 06:10:41PM -0400, Mimi Zohar wrote: > On Thu, 2017-11-02 at 22:04 +, David Howells wrote: > > Mimi Zohar wrote: > > > > > > Only validly signed device firmware may be loaded. > > > > > > fw_get_filesystem_firmware() calls kernel_read_file_from_path() to > > > read the firmware, which calls into the security hooks. Is there > > > another place that validates the firmware signatures. I'm not seeing > > > which patch requires firmware to be signed? > > > > Luis has a set of patches for this. However, I'm not sure if that's going > > anywhere at the moment. Possibly I should remove this from the manpage for > > the moment. Remove it for now. The state of of affairs for firmware signing is complex given that we first wanted to address how to properly grow the API without making the API worse. This in and of itself was an effort, and that effort also evaluated two different development paradigms: o functional API o data driven API I only recently was convinced that functional API should be used, even for commonly used exported symbols, and as such I've been going back and slowly grooming the firmware API with small atomic changes to first clean up the complex flag mess we have. Since I'm busy with that Takahiro AKASHI has taken up firmware singing effort but this will depend on the above small cleanup to be done first. I was busy with addressing existing bugs on the firmware API for a while, then company travel / conferences so was not able to address this, but I'm back now and I believe I should be able to tackle the cleanup now. Only after this is merged can we expect a final respin of the firmware signing effort. > Or reflect that IMA-appraisal, if enabled, will enforce firmware being > validly signed. But FWICT lockdown is a built-in kernel thingy, unless lockdown implies IMA it would not be the place to refer to it. It seems the documentation was proposed to help users if an error was caught. That error should cover only what is being addressed in code on the kernel. Luis -- 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
