Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-05-13 Thread Hans de Goede

Hi,

On 05/08/2018 06:12 PM, Luis R. Rodriguez wrote:

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 belong in a properly architected
OS<->firmware exchange.


OK, it sounds to me like we have room to then implement our own de-facto
standard for letting vendors stuff firmware into EFI as we in the Linux
community see fit.

We can start out by supporting existing drivers, but also consider customizing
this in the future for our own needs, so long as we document it and set
expectations well.

So we need to support what Hans is implementing for two reasons then:

a) The FV Protocol cannot be used to support the two drivers he's
trying to provide support for -- I 

Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-05-13 Thread Hans de Goede

Hi,

On 05/13/2018 12:43 PM, Ard Biesheuvel wrote:

On 13 May 2018 at 13:03, Hans de Goede  wrote:

Hi,


On 05/04/2018 06:56 AM, Ard Biesheuvel wrote:


Hi Hans,

One comment below, which I missed in review before.

On 29 April 2018 at 11:35, 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 
---


[...]


diff --git a/drivers/firmware/efi/embedded-firmware.c
b/drivers/firmware/efi/embedded-firmware.c
new file mode 100644
index ..22a0f598b53d
--- /dev/null
+++ b/drivers/firmware/efi/embedded-firmware.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for extracting embedded firmware for peripherals from EFI
code,
+ *
+ * Copyright (c) 2018 Hans de Goede 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct embedded_fw {
+   struct list_head list;
+   const char *name;
+   void *data;
+   size_t length;
+};
+
+static LIST_HEAD(found_fw_list);
+
+static const struct dmi_system_id * const embedded_fw_table[] = {
+   NULL
+};
+
+/*
+ * Note the efi_check_for_embedded_firmwares() code currently makes the
+ * following 2 assumptions. This may needs to be revisited if embedded
firmware
+ * is found where this is not true:
+ * 1) The firmware is only found in EFI_BOOT_SERVICES_CODE memory
segments
+ * 2) The firmware always starts at an offset which is a multiple of 8
bytes
+ */
+static int __init efi_check_md_for_embedded_firmware(
+   efi_memory_desc_t *md, const struct efi_embedded_fw_desc *desc)
+{
+   struct embedded_fw *fw;
+   u64 i, size;
+   u32 crc;
+   u8 *mem;
+
+   size = md->num_pages << EFI_PAGE_SHIFT;
+   mem = memremap(md->phys_addr, size, MEMREMAP_WB);
+   if (!mem) {
+   pr_err("Error mapping EFI mem at %#llx\n",
md->phys_addr);
+   return -ENOMEM;
+   }
+
+   size -= desc->length;
+   for (i = 0; i < size; i += 8) {
+   if (*((u64 *)(mem + i)) != *((u64 *)desc->prefix))
+   continue;
+



Please use the proper APIs here to cast u8* to u64*, i.e., either use
get_unaligned64() or use memcmp()



But we know the memory addresses are 64 bit aligned, so using
get_unaligned64 seems wrong, and I'm not sure if the compiler is
smart enough to optimize a memcmp to the single 64 bit integer comparison
we want done here.



Fair enough. The memory regions are indeed guaranteed to be 4k aligned.

So in that case, please make mem a u64* and cast the other way where needed.


Ok, I've reworked the code to get rid of the compares in the if condition.

Regards,

Hans








+   /* Seed with ~0, invert to match crc32 userspace utility
*/
+   crc = ~crc32(~0, mem + i, desc->length);
+   if (crc == desc->crc)
+   break;
+   }
+
+   memunmap(mem);
+
+   if (i >= size)
+   return -ENOENT;
+
+   pr_info("Found EFI embedded fw '%s' crc %08x\n", desc->name,
desc->crc);
+
+   fw = kmalloc(sizeof(*fw), 

Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-05-13 Thread Ard Biesheuvel
On 13 May 2018 at 13:03, Hans de Goede  wrote:
> Hi,
>
>
> On 05/04/2018 06:56 AM, Ard Biesheuvel wrote:
>>
>> Hi Hans,
>>
>> One comment below, which I missed in review before.
>>
>> On 29 April 2018 at 11:35, 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 
>>> ---
>>
>> [...]
>>>
>>> diff --git a/drivers/firmware/efi/embedded-firmware.c
>>> b/drivers/firmware/efi/embedded-firmware.c
>>> new file mode 100644
>>> index ..22a0f598b53d
>>> --- /dev/null
>>> +++ b/drivers/firmware/efi/embedded-firmware.c
>>> @@ -0,0 +1,149 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Support for extracting embedded firmware for peripherals from EFI
>>> code,
>>> + *
>>> + * Copyright (c) 2018 Hans de Goede 
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +struct embedded_fw {
>>> +   struct list_head list;
>>> +   const char *name;
>>> +   void *data;
>>> +   size_t length;
>>> +};
>>> +
>>> +static LIST_HEAD(found_fw_list);
>>> +
>>> +static const struct dmi_system_id * const embedded_fw_table[] = {
>>> +   NULL
>>> +};
>>> +
>>> +/*
>>> + * Note the efi_check_for_embedded_firmwares() code currently makes the
>>> + * following 2 assumptions. This may needs to be revisited if embedded
>>> firmware
>>> + * is found where this is not true:
>>> + * 1) The firmware is only found in EFI_BOOT_SERVICES_CODE memory
>>> segments
>>> + * 2) The firmware always starts at an offset which is a multiple of 8
>>> bytes
>>> + */
>>> +static int __init efi_check_md_for_embedded_firmware(
>>> +   efi_memory_desc_t *md, const struct efi_embedded_fw_desc *desc)
>>> +{
>>> +   struct embedded_fw *fw;
>>> +   u64 i, size;
>>> +   u32 crc;
>>> +   u8 *mem;
>>> +
>>> +   size = md->num_pages << EFI_PAGE_SHIFT;
>>> +   mem = memremap(md->phys_addr, size, MEMREMAP_WB);
>>> +   if (!mem) {
>>> +   pr_err("Error mapping EFI mem at %#llx\n",
>>> md->phys_addr);
>>> +   return -ENOMEM;
>>> +   }
>>> +
>>> +   size -= desc->length;
>>> +   for (i = 0; i < size; i += 8) {
>>> +   if (*((u64 *)(mem + i)) != *((u64 *)desc->prefix))
>>> +   continue;
>>> +
>>
>>
>> Please use the proper APIs here to cast u8* to u64*, i.e., either use
>> get_unaligned64() or use memcmp()
>
>
> But we know the memory addresses are 64 bit aligned, so using
> get_unaligned64 seems wrong, and I'm not sure if the compiler is
> smart enough to optimize a memcmp to the single 64 bit integer comparison
> we want done here.
>

Fair enough. The memory regions are indeed guaranteed to be 4k aligned.

So in that case, please make mem a u64* and cast the other way where needed.

>>
>>> +   /* Seed with ~0, invert to match crc32 userspace 

Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-05-13 Thread Hans de Goede

Hi,

On 05/03/2018 11:35 PM, Andy Lutomirski wrote:

On Thu, May 3, 2018 at 3:31 PM Luis R. Rodriguez  wrote:


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.


Nah, don't use the cryptoapi for this.  You'll probably regret it for any
number of reasons.  My code is here:

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=crypto/sha256_bpf=e9e12f056f2abed50a30b762db9185799f5864e6

and its two parents.  It needs a little bit of dusting and it needs
checking that all combinations of modular and non-modular builds work.  Ard
probably has further comments.


Looks good, I've cherry picked this into my personal tree and will make
the next version of the EFI embedded-firmware patches use SHA256.

As Luis already mentioned geting the EFI embedded-firmware patches
upstream is not something urgent, so it is probably best to just
wait for you to push these upstream I guess?

Regards,

Hans

--
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

2018-05-13 Thread Hans de Goede

Hi,

On 05/03/2018 11:31 PM, Luis R. Rodriguez wrote:

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.


I agree that there is no rush to get this in. I will rebase this on top
of the "[PATCH v7 00/14] firmware_loader changes for v4.18" series you recently
send as well as try to address all the remarks made sofar. I'm not entirely
sure when I will get around to this.

Regards,

Hans

--
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

2018-05-13 Thread Hans de Goede

Hi,

On 05/04/2018 06:56 AM, Ard Biesheuvel wrote:

Hi Hans,

One comment below, which I missed in review before.

On 29 April 2018 at 11:35, 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 
---

[...]

diff --git a/drivers/firmware/efi/embedded-firmware.c 
b/drivers/firmware/efi/embedded-firmware.c
new file mode 100644
index ..22a0f598b53d
--- /dev/null
+++ b/drivers/firmware/efi/embedded-firmware.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for extracting embedded firmware for peripherals from EFI code,
+ *
+ * Copyright (c) 2018 Hans de Goede 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct embedded_fw {
+   struct list_head list;
+   const char *name;
+   void *data;
+   size_t length;
+};
+
+static LIST_HEAD(found_fw_list);
+
+static const struct dmi_system_id * const embedded_fw_table[] = {
+   NULL
+};
+
+/*
+ * Note the efi_check_for_embedded_firmwares() code currently makes the
+ * following 2 assumptions. This may needs to be revisited if embedded firmware
+ * is found where this is not true:
+ * 1) The firmware is only found in EFI_BOOT_SERVICES_CODE memory segments
+ * 2) The firmware always starts at an offset which is a multiple of 8 bytes
+ */
+static int __init efi_check_md_for_embedded_firmware(
+   efi_memory_desc_t *md, const struct efi_embedded_fw_desc *desc)
+{
+   struct embedded_fw *fw;
+   u64 i, size;
+   u32 crc;
+   u8 *mem;
+
+   size = md->num_pages << EFI_PAGE_SHIFT;
+   mem = memremap(md->phys_addr, size, MEMREMAP_WB);
+   if (!mem) {
+   pr_err("Error mapping EFI mem at %#llx\n", md->phys_addr);
+   return -ENOMEM;
+   }
+
+   size -= desc->length;
+   for (i = 0; i < size; i += 8) {
+   if (*((u64 *)(mem + i)) != *((u64 *)desc->prefix))
+   continue;
+


Please use the proper APIs here to cast u8* to u64*, i.e., either use
get_unaligned64() or use memcmp()


But we know the memory addresses are 64 bit aligned, so using
get_unaligned64 seems wrong, and I'm not sure if the compiler is
smart enough to optimize a memcmp to the single 64 bit integer comparison
we want done here.

Regards,

Hans




+   /* Seed with ~0, invert to match crc32 userspace utility */
+   crc = ~crc32(~0, mem + i, desc->length);
+   if (crc == desc->crc)
+   break;
+   }
+
+   memunmap(mem);
+
+   if (i >= size)
+   return -ENOENT;
+
+   pr_info("Found EFI embedded fw '%s' crc %08x\n", desc->name, desc->crc);
+
+   fw = kmalloc(sizeof(*fw), GFP_KERNEL);
+   if (!fw)
+   return -ENOMEM;
+
+   mem = memremap(md->phys_addr + i, desc->length, MEMREMAP_WB);
+   if (!mem) {
+   pr_err("Error mapping embedded firmware\n");
+   goto error_free_fw;
+   }
+   fw->data = kmemdup(mem, desc->length, GFP_KERNEL);
+   memunmap(mem);
+   if 

Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-05-08 Thread Luis R. Rodriguez
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 belong in a properly architected
> OS<->firmware exchange.

OK, it sounds to me like we have room to then implement our own de-facto
standard for letting vendors stuff firmware into EFI as we in the Linux
community see fit.

We can start out by supporting existing drivers, but also consider customizing
this in the future for our own needs, so long as we document it and set
expectations well.

So we need to support what Hans is 

Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-05-03 Thread Ard Biesheuvel
Hi Hans,

One comment below, which I missed in review before.

On 29 April 2018 at 11:35, 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 
> ---
[...]
> diff --git a/drivers/firmware/efi/embedded-firmware.c 
> b/drivers/firmware/efi/embedded-firmware.c
> new file mode 100644
> index ..22a0f598b53d
> --- /dev/null
> +++ b/drivers/firmware/efi/embedded-firmware.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Support for extracting embedded firmware for peripherals from EFI code,
> + *
> + * Copyright (c) 2018 Hans de Goede 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct embedded_fw {
> +   struct list_head list;
> +   const char *name;
> +   void *data;
> +   size_t length;
> +};
> +
> +static LIST_HEAD(found_fw_list);
> +
> +static const struct dmi_system_id * const embedded_fw_table[] = {
> +   NULL
> +};
> +
> +/*
> + * Note the efi_check_for_embedded_firmwares() code currently makes the
> + * following 2 assumptions. This may needs to be revisited if embedded 
> firmware
> + * is found where this is not true:
> + * 1) The firmware is only found in EFI_BOOT_SERVICES_CODE memory segments
> + * 2) The firmware always starts at an offset which is a multiple of 8 bytes
> + */
> +static int __init efi_check_md_for_embedded_firmware(
> +   efi_memory_desc_t *md, const struct efi_embedded_fw_desc *desc)
> +{
> +   struct embedded_fw *fw;
> +   u64 i, size;
> +   u32 crc;
> +   u8 *mem;
> +
> +   size = md->num_pages << EFI_PAGE_SHIFT;
> +   mem = memremap(md->phys_addr, size, MEMREMAP_WB);
> +   if (!mem) {
> +   pr_err("Error mapping EFI mem at %#llx\n", md->phys_addr);
> +   return -ENOMEM;
> +   }
> +
> +   size -= desc->length;
> +   for (i = 0; i < size; i += 8) {
> +   if (*((u64 *)(mem + i)) != *((u64 *)desc->prefix))
> +   continue;
> +

Please use the proper APIs here to cast u8* to u64*, i.e., either use
get_unaligned64() or use memcmp()

> +   /* Seed with ~0, invert to match crc32 userspace utility */
> +   crc = ~crc32(~0, mem + i, desc->length);
> +   if (crc == desc->crc)
> +   break;
> +   }
> +
> +   memunmap(mem);
> +
> +   if (i >= size)
> +   return -ENOENT;
> +
> +   pr_info("Found EFI embedded fw '%s' crc %08x\n", desc->name, 
> desc->crc);
> +
> +   fw = kmalloc(sizeof(*fw), GFP_KERNEL);
> +   if (!fw)
> +   return -ENOMEM;
> +
> +   mem = memremap(md->phys_addr + i, desc->length, MEMREMAP_WB);
> +   if (!mem) {
> +   pr_err("Error mapping embedded firmware\n");
> +   goto error_free_fw;
> +   }
> +   fw->data = kmemdup(mem, desc->length, GFP_KERNEL);
> +   memunmap(mem);
> +   if (!fw->data)
> +   goto error_free_fw;
> +
> 

Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-05-03 Thread Ard Biesheuvel
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, and so it is likely that
the protocols are available. However, the PI spec does not cover
firmware blobs, 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.

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.

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. 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.

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, or we architect it properly, in which case we shouldn't
depend on PI because it does not belong in a properly architected
OS<->firmware exchange.

-- 
Ard.
--
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

2018-05-03 Thread Luis R. Rodriguez
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

Re: [PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-05-03 Thread Mimi Zohar
On Thu, 2018-05-03 at 22:23 +, Luis R. Rodriguez wrote:
> 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?

The direct fs lookup calls kernel_read_file_from_path(), which calls
the security_kernel_read_file() and security_kernel_post_read_file()
hooks.  So there is no need to add a direct call to either of these
security calls.

Mimi



  

--
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

2018-05-03 Thread Andy Lutomirski
On Thu, May 3, 2018 at 3:31 PM Luis R. Rodriguez  wrote:

> 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.

Nah, don't use the cryptoapi for this.  You'll probably regret it for any
number of reasons.  My code is here:

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=crypto/sha256_bpf=e9e12f056f2abed50a30b762db9185799f5864e6

and its two parents.  It needs a little bit of dusting and it needs
checking that all combinations of modular and non-modular builds work.  Ard
probably has further comments.


> 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.

Fair enough.
--
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

2018-05-03 Thread Luis R. Rodriguez
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

2018-05-03 Thread Luis R. Rodriguez
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 v5 2/5] efi: Add embedded peripheral firmware support

2018-05-02 Thread Hans de Goede

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.


(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
--
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

2018-05-01 Thread Andy Lutomirski
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?  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.

> +   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.

(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.)
--
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

2018-05-01 Thread Mimi Zohar
On Tue, 2018-05-01 at 21:11 +0200, Hans de Goede wrote:
> Hi,
> 
> On 01-05-18 16:36, Mimi Zohar wrote:
> > [Cc'ing linux-security]
> > 
> > On Sun, 2018-04-29 at 11:35 +0200, Hans de Goede wrote:
> > [...]
> >> diff --git a/drivers/base/firmware_loader/fallback_efi.c 
> >> b/drivers/base/firmware_loader/fallback_efi.c
> >> new file mode 100644
> >> index ..82ba82f48a79
> >> --- /dev/null
> >> +++ b/drivers/base/firmware_loader/fallback_efi.c
> >> @@ -0,0 +1,51 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#include "fallback.h"
> >> +#include "firmware.h"
> >> +
> >> +int fw_get_efi_embedded_fw(struct device *dev, struct fw_priv *fw_priv,
> >> + enum fw_opt *opt_flags, int ret)
> >> +{
> >> +  enum kernel_read_file_id id = READING_FIRMWARE;
> > 
> > Please define a new kernel_read_file_id for this (eg.
> > READING_FIRMWARE_EFI_EMBEDDED).
> 
> Are you sure, I wonder how useful it is to add a new
> kernel_read_file_id every time a new way to get firmware
> comes up?
> 
> I especially wonder about the sense in adding a new id
> given that the quite old FIRMWARE_PREALLOC_BUFFER is
> still not supported / checked properly by the security code.

I posted patches earlier today[1], which address this.  Patch 5/6 just
makes it equivalent to READING_FIRMWARE.  Patch 6/6 questions whether
the device has access to the pre-allocated buffer *before* the
signature has been verified.

[1] kernsec.org/pipermail/linux-security-module-archive/2018-May/006639.html

> 
> Anyways I can add a new id if you want me to, what about
> when fw_get_efi_embedded_fw is reading into a driver allocated
> buffer, do you want a separate EADING_FIRMWARE_EFI_EMBEDDED_PREALLOC_BUFFER
> for that ?

Without the kernel being able to verify the firmware's signature, I'm
not sure it makes much of a difference.

> 
> > 
> >> +  size_t size, max = INT_MAX;
> >> +  int rc;
> >> +
> >> +  if (!dev)
> >> +  return ret;
> >> +
> >> +  if (!device_property_read_bool(dev, "efi-embedded-firmware"))
> >> +  return ret;
> > 
> > Instead of calling security_kernel_post_read_file(), either in
> > device_property_read_bool() or here call security_kernel_read_file().
> > 
> > The pre read call is for deciding whether to allow this call
> > independent of the firmware being loaded, whereas the post security
> > call is currently being used by IMA-appraisal for verifying a
> > signature.  There might be other LSMs using the post hook as well.  As
> > there is no kernel signature associated with this firmware, use the
> > security pre read_file hook.
> 
> 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

--
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

2018-05-01 Thread Hans de Goede

Hi,

On 01-05-18 16:36, Mimi Zohar wrote:

[Cc'ing linux-security]

On Sun, 2018-04-29 at 11:35 +0200, Hans de Goede wrote:
[...]

diff --git a/drivers/base/firmware_loader/fallback_efi.c 
b/drivers/base/firmware_loader/fallback_efi.c
new file mode 100644
index ..82ba82f48a79
--- /dev/null
+++ b/drivers/base/firmware_loader/fallback_efi.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+#include 
+#include 
+
+#include "fallback.h"
+#include "firmware.h"
+
+int fw_get_efi_embedded_fw(struct device *dev, struct fw_priv *fw_priv,
+  enum fw_opt *opt_flags, int ret)
+{
+   enum kernel_read_file_id id = READING_FIRMWARE;


Please define a new kernel_read_file_id for this (eg.
READING_FIRMWARE_EFI_EMBEDDED).


Are you sure, I wonder how useful it is to add a new
kernel_read_file_id every time a new way to get firmware
comes up?

I especially wonder about the sense in adding a new id
given that the quite old FIRMWARE_PREALLOC_BUFFER is
still not supported / checked properly by the security code.

Anyways I can add a new id if you want me to, what about
when fw_get_efi_embedded_fw is reading into a driver allocated
buffer, do you want a separate EADING_FIRMWARE_EFI_EMBEDDED_PREALLOC_BUFFER
for that ?







+   size_t size, max = INT_MAX;
+   int rc;
+
+   if (!dev)
+   return ret;
+
+   if (!device_property_read_bool(dev, "efi-embedded-firmware"))
+   return ret;


Instead of calling security_kernel_post_read_file(), either in
device_property_read_bool() or here call security_kernel_read_file().

The pre read call is for deciding whether to allow this call
independent of the firmware being loaded, whereas the post security
call is currently being used by IMA-appraisal for verifying a
signature.  There might be other LSMs using the post hook as well.  As
there is no kernel signature associated with this firmware, use the
security pre read_file hook.


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.

Regards,

Hans




thanks,

Mimi


+
+   *opt_flags |= FW_OPT_NO_WARN | FW_OPT_NOCACHE | FW_OPT_NOFALLBACK;
+
+   /* Already populated data member means we're loading into a buffer */
+   if (fw_priv->data) {
+   id = READING_FIRMWARE_PREALLOC_BUFFER;
+   max = fw_priv->allocated_size;
+   }
+
+   rc = efi_get_embedded_fw(fw_priv->fw_name, _priv->data, , max);
+   if (rc) {
+   dev_warn(dev, "Firmware %s not in EFI\n", fw_priv->fw_name);
+   return ret;
+   }
+
+   rc = security_kernel_post_read_file(NULL, fw_priv->data, size, id);
+   if (rc) {
+   if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
+   vfree(fw_priv->data);
+   fw_priv->data = NULL;
+   }
+   return rc;
+   }
+
+   dev_dbg(dev, "using efi-embedded fw %s\n", fw_priv->fw_name);
+   fw_priv->size = size;
+   fw_state_done(fw_priv);
+   return 0;
+}



--
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

2018-05-01 Thread Mimi Zohar
[Cc'ing linux-security]

On Sun, 2018-04-29 at 11:35 +0200, Hans de Goede wrote:
[...]
> diff --git a/drivers/base/firmware_loader/fallback_efi.c 
> b/drivers/base/firmware_loader/fallback_efi.c
> new file mode 100644
> index ..82ba82f48a79
> --- /dev/null
> +++ b/drivers/base/firmware_loader/fallback_efi.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "fallback.h"
> +#include "firmware.h"
> +
> +int fw_get_efi_embedded_fw(struct device *dev, struct fw_priv *fw_priv,
> +enum fw_opt *opt_flags, int ret)
> +{
> + enum kernel_read_file_id id = READING_FIRMWARE;

Please define a new kernel_read_file_id for this (eg.
READING_FIRMWARE_EFI_EMBEDDED).

> + size_t size, max = INT_MAX;
> + int rc;
> +
> + if (!dev)
> + return ret;
> +
> + if (!device_property_read_bool(dev, "efi-embedded-firmware"))
> + return ret;

Instead of calling security_kernel_post_read_file(), either in
device_property_read_bool() or here call security_kernel_read_file().

The pre read call is for deciding whether to allow this call
independent of the firmware being loaded, whereas the post security
call is currently being used by IMA-appraisal for verifying a
signature.  There might be other LSMs using the post hook as well.  As
there is no kernel signature associated with this firmware, use the
security pre read_file hook.

thanks,

Mimi

> +
> + *opt_flags |= FW_OPT_NO_WARN | FW_OPT_NOCACHE | FW_OPT_NOFALLBACK;
> +
> + /* Already populated data member means we're loading into a buffer */
> + if (fw_priv->data) {
> + id = READING_FIRMWARE_PREALLOC_BUFFER;
> + max = fw_priv->allocated_size;
> + }
> +
> + rc = efi_get_embedded_fw(fw_priv->fw_name, _priv->data, , max);
> + if (rc) {
> + dev_warn(dev, "Firmware %s not in EFI\n", fw_priv->fw_name);
> + return ret;
> + }
> +
> + rc = security_kernel_post_read_file(NULL, fw_priv->data, size, id);
> + if (rc) {
> + if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
> + vfree(fw_priv->data);
> + fw_priv->data = NULL;
> + }
> + return rc;
> + }
> +
> + dev_dbg(dev, "using efi-embedded fw %s\n", fw_priv->fw_name);
> + fw_priv->size = size;
> + fw_state_done(fw_priv);
> + return 0;
> +}

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 2/5] efi: Add embedded peripheral firmware support

2018-04-29 Thread Hans de Goede
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 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  |  66 
 drivers/base/firmware_loader/Makefile |   1 +
 drivers/base/firmware_loader/fallback.h   |  12 ++
 drivers/base/firmware_loader/fallback_efi.c   |  51 ++
 drivers/base/firmware_loader/main.c   |   2 +
 drivers/firmware/efi/Kconfig  |   3 +
 drivers/firmware/efi/Makefile |   1 +
 drivers/firmware/efi/embedded-firmware.c  | 149 ++
 include/linux/efi.h   |   6 +
 include/linux/efi_embedded_fw.h   |  25 +++
 init/main.c   |   3 +
 11 files changed, 319 insertions(+)
 create mode 100644 drivers/base/firmware_loader/fallback_efi.c
 create mode 100644 drivers/firmware/efi/embedded-firmware.c
 create mode 100644 include/linux/efi_embedded_fw.h

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
@@