diff mbox

[1/2] common/efi: bail if dom0 fails the shim verification step

Message ID 20170920205718.17747-1-tamas@tklengyel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tamas K Lengyel Sept. 20, 2017, 8:57 p.m. UTC 
From: Tamas K Lengyel <lengyelt@ainfosec.com>

If the shim protocol is located it is expected that the dom0 kernel image
will also pass the shim verification.

Signed-off-by: Tamas K Lengyel <lengyelt@ainfosec.com>
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/common/efi/boot.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Jan Beulich Sept. 21, 2017, 1:03 p.m. UTC | #1 
>>> On 20.09.17 at 22:57, <tamas@tklengyel.com> wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1226,9 +1226,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>          efi_bs->FreePool(name.w);
>  
>          if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> -                        (void **)&shim_lock)) &&
> -             (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS )
> -            PrintErrMesg(L"Dom0 kernel image could not be verified", status);
> +                        (void **)&shim_lock)))
> +        {
> +            if  ( shim_lock->Verify(kernel.ptr, kernel.size) != EFI_SUCCESS )
> +                blexit(L"Dom0 kernel image could not be verified by the shim.");
> +
> +            PrintStr(L"Dom0 kernel image was verified by the shim.\r\n");
> +        }

So what is the actual behavioral change you're trying to
accomplish? PrintErrMesg() already calls blexit(), and I hope
sure the purpose of the change is neither to open code
anything, nor to drop the printing of the error code. And I
don't see any value in the success case message - it'll be
visible for a very brief moment at best anyway.

Jan
Tamas K Lengyel Sept. 21, 2017, 3:07 p.m. UTC | #2 
On Thu, Sep 21, 2017 at 7:03 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 20.09.17 at 22:57, <tamas@tklengyel.com> wrote:
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -1226,9 +1226,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>>          efi_bs->FreePool(name.w);
>>
>>          if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
>> -                        (void **)&shim_lock)) &&
>> -             (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS )
>> -            PrintErrMesg(L"Dom0 kernel image could not be verified", status);
>> +                        (void **)&shim_lock)))
>> +        {
>> +            if  ( shim_lock->Verify(kernel.ptr, kernel.size) != EFI_SUCCESS )
>> +                blexit(L"Dom0 kernel image could not be verified by the shim.");
>> +
>> +            PrintStr(L"Dom0 kernel image was verified by the shim.\r\n");
>> +        }
>
> So what is the actual behavioral change you're trying to
> accomplish? PrintErrMesg() already calls blexit(),

Indeed, I've somehow missed that. Sorry for the noise.

Tamas
diff mbox

Patch

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 01d33004e0..a3a439b838 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1226,9 +1226,13 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         efi_bs->FreePool(name.w);
 
         if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
-                        (void **)&shim_lock)) &&
-             (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS )
-            PrintErrMesg(L"Dom0 kernel image could not be verified", status);
+                        (void **)&shim_lock)))
+        {
+            if  ( shim_lock->Verify(kernel.ptr, kernel.size) != EFI_SUCCESS )
+                blexit(L"Dom0 kernel image could not be verified by the shim.");
+
+            PrintStr(L"Dom0 kernel image was verified by the shim.\r\n");
+        }
 
         name.s = get_value(&cfg, section.s, "ramdisk");
         if ( name.s )