Re: [Xen-devel] [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2()

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Fri, May 04, 2018 at 09:46:33AM -0600, Jan Beulich wrote:
> >>> On 08.07.17 at 23:53, <daniel.kiper@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/boot/head.S
> > +++ b/xen/arch/x86/boot/head.S
> > @@ -383,9 +383,13 @@ __efi64_mb2_start:
> >          jmp     x86_32_switch
> >
> >  .Lefi_multiboot2_proto:
> > -        /* Zero EFI SystemTable and EFI ImageHandle addresses. */
> > +        /*
> > +         * Zero EFI SystemTable, EFI ImageHandle and
> > +         * dom0 kernel module struct addresses.
> > +         */
> >          xor     %esi,%esi
> >          xor     %edi,%edi
> > +        xor     %r14d,%r14d
> >
> >          /* Skip Multiboot2 information fixed part. */
> >          lea     (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
> > @@ -423,6 +427,15 @@ __efi64_mb2_start:
> >          cmove   MB2_efi64_ih(%rcx),%rdi
> >          je      .Lefi_mb2_next_tag
> >
> > +        /* Get dom0 kernel module struct address from Multiboot2 
> > information. */
> > +        cmpl    $MULTIBOOT2_TAG_TYPE_MODULE,MB2_tag_type(%rcx)
> > +        jne     .Lefi_mb2_end
> > +
> > +        test    %r14d,%r14d
> > +        cmovz   %ecx,%r14d
> > +        jmp     .Lefi_mb2_next_tag
> > +
> > +.Lefi_mb2_end:
> >          /* Is it the end of Multiboot2 information? */
> >          cmpl    $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
> >          je      .Lrun_bs
> > @@ -484,9 +497,12 @@ __efi64_mb2_start:
> >          /* Keep the stack aligned. Do not pop a single item off it. */
> >          mov     (%rsp),%rdi
> >
> > +        mov     %r14d,%edx
> > +
> >          /*
> >           * efi_multiboot2() is called according to System V AMD64 ABI:
> > -         *   - IN:  %rdi - EFI ImageHandle, %rsi - EFI SystemTable.
> > +         *   - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
> > +         *         %rdx - dom0 kernel module struct address.
>
> How come everything further up treats this as a 32-bit quantity only?

According to the Multiboot2 spec the bootloader is not allowed to
put the kernel (xen.gz) and the modules above 4 GiB boundary. And
comment below __efi64_mb2_start label is clear about that.

> > @@ -47,6 +49,7 @@ extern const struct pe_base_relocs {
> >
> >  static void __init efi_arch_relocate_image(unsigned long delta)
> >  {
> > +#if 0
> >      const struct pe_base_relocs *base_relocs;
> >
> >      for ( base_relocs = __base_relocs_start; base_relocs <
> > __base_relocs_end; )
> > @@ -95,6 +98,7 @@ static void __init efi_arch_relocate_image(unsigned long 
> > delta)
> >          }
> >          base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
> >      }
> > +#endif
> >  }
>
> ???

Please forget it. This is just an RFC. It will be fixed in v2.

> > @@ -669,7 +673,9 @@ static bool __init
> > efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
> >
> >  static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { }
> >
> > -void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> > *SystemTable)
> > +void __init efi_multiboot2(EFI_HANDLE ImageHandle,
> > +                           EFI_SYSTEM_TABLE *SystemTable,
> > +                           multiboot2_tag_module_t *dom0_kernel)
>
> const?

Will do.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel