[efi] Allow for the existence of multiple shim lock protocols

When multiple shims are present in the system (e.g. in a boot chain
such as UEFI -> iPXE shim -> iPXE -> distro shim -> distro kernel),
there may be more than one installed shim lock protocol.

There is no sensible way to identify which shim lock protocol belongs
to which shim.  The shim lock protocol is installed on an anonymous
handle that has no device path, no other form of identifier, and no
connection to any other handle or protocol instance installed by the
shim.

The shim does include some extremely convoluted logic whereby a second
shim will attempt to uninstall a shim lock protocol installed by an
earlier shim.  However, this logic is broken: the second shim calls
UninstallProtocolInterface() with the wrong handle and the wrong
protocol interface pointer.  This logic error is silently ignored
since shim does not bother to check the return status.

Experience shows that there is unfortunately no point in trying to get
a fix for this upstreamed into shim, or even in raising the issue with
the shim project.  We therefore work around the shim bug by calling
all instances of the shim lock protocol, rather than relying on shim
itself to ensure that only one such instance exists.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/interface/efi/efi_shim.c b/src/interface/efi/efi_shim.c
index 553cb2721c64f118dfc2fb99574b9fb15becf0fc..36c459076d77a2213fd2bb59775493ddfbde9e19 100644 (file)
--- a/src/interface/efi/efi_shim.c
+++ b/src/interface/efi/efi_shim.c
@@ -144,19 +144,44 @@ static int efi_shim_is_sbatlevel ( const CHAR16 *name, const EFI_GUID *guid ) {
  */
 static void efi_shim_unlock ( void ) {
        EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+       EFI_GUID *protocol = &efi_shim_lock_protocol_guid;
+       EFI_SHIM_LOCK_PROTOCOL *lock;
        uint8_t empty[0];
-       union {
-               EFI_SHIM_LOCK_PROTOCOL *lock;
-               void *interface;
-       } u;
+       EFI_HANDLE *handles;
+       UINTN num_handles;
+       unsigned int i;
        EFI_STATUS efirc;
+       int rc;
 
-       /* Locate shim lock protocol */
-       if ( ( efirc = bs->LocateProtocol ( &efi_shim_lock_protocol_guid,
-                                           NULL, &u.interface ) ) == 0 ) {
-               u.lock->Verify ( empty, sizeof ( empty ) );
-               DBGC ( &efi_shim, "SHIM unlocked via %p\n", u.lock );
+       /* Locate shim lock protocol(s) */
+       if ( ( efirc = bs->LocateHandleBuffer ( ByProtocol, protocol,
+                                               NULL, &num_handles,
+                                               &handles ) ) != 0 ) {
+               rc = -EEFI ( efirc );
+               DBGC ( &efi_shim, "SHIM could not locate shim locks: %s\n",
+                      strerror ( rc ) );
+               goto err_locate;
        }
+
+       /* Unlock each shim lock */
+       for ( i = 0 ; i < num_handles ; i++ ) {
+
+               /* Open shim lock protocol */
+               if ( ( rc = efi_open ( handles[i], protocol, &lock ) ) != 0 ) {
+                       DBGC ( &efi_shim, "SHIM could not open lock %d (%p): "
+                              "%s\n", i, handles[i], strerror ( rc ) );
+                       continue;
+               }
+
+               /* Unlock shim lock */
+               lock->Verify ( empty, sizeof ( empty ) );
+               DBGC ( &efi_shim, "SHIM unlocked lock %d (%p) via %p\n",
+                      i, handles[i], lock );
+       }
+
+       bs->FreePool ( handles );
+ err_locate:
+       return;
 }
 
 /**