Messages in this thread Patch in this message | | | Date | Thu, 31 Mar 2022 12:04:40 +0300 | Subject | Re: [PATCH v8 3/4] efi: Load efi_secret module if EFI secret area is populated | From | Dov Murik <> |
| |
Hello Ard,
On 28/02/2022 15:15, Ard Biesheuvel wrote: > On Mon, 28 Feb 2022 at 14:07, Dov Murik <dovmurik@linux.ibm.com> wrote: >> >> >> >> On 28/02/2022 14:49, Ard Biesheuvel wrote: >>> On Mon, 28 Feb 2022 at 12:43, Dov Murik <dovmurik@linux.ibm.com> wrote: >>>> >>>> If the efi_secret module is built, register a late_initcall in the EFI >>>> driver which checks whether the EFI secret area is available and >>>> populated, and then requests to load the efi_secret module. >>>> >>>> This will cause the <securityfs>/secrets/coco directory to appear in >>>> guests into which secrets were injected; in other cases, the module is >>>> not loaded. >>>> >>>> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> >>>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> >>> >>> It would be better to simply expose a platform device and associated >>> driver, instead of hooking into the module machinery directly. >>> >>> We already do something similar for the EFI rtc and the efivars >>> subsystem, using platform_device_register_simple() >>> >> >> Thanks Ard, I'll look into this. >> >> I hope the mechanism you suggest allows me to perform complex check to >> see if the device is really there (in this case: check for an efi >> variable, map memory as encrypted, verify it starts with a specific GUID >> -- everything before request_module() in the code below). >> > > There is the device part and the driver part. Some of this belongs in > the core code that registers the platform device, and some of it > belongs in the driver. At this point, it probably does not matter that > much which side does what, as the platform driver simply probes and > can perform whatever check it needs, as long as it can back out > gracefully (although I understand that, in this particular case, there > are reasons why the driver may decide to wipe the secret)
I finally got to implement this, it seems like it makes the code simple. Thanks for the advice.
Just making sure I understand correctly: in this approach this we rely on udev to load the efi_secret module (aliased as "platform:efi_secret") and call its .probe() function? If there's no udev, the module will not be loaded automatically. Did I understand that correctly?
In such a case, the only thing needed to add in efi.c is:
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 378d044b2463..b92eabc554e6 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -425,6 +425,9 @@ static int __init efisubsys_init(void) if (efi_enabled(EFI_DBG) && efi_enabled(EFI_PRESERVE_BS_REGIONS)) efi_debugfs_init();
+ if (efi.coco_secret != EFI_INVALID_TABLE_ADDR) + platform_device_register_simple("efi_secret", 0, NULL, 0); + return 0;
err_remove_group:
Does this seem OK? (before I re-spin the whole series.)
Thanks, -Dov
| |