lkml.org 
[lkml]   [2020]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 07/13] x86: Secure Launch kernel early boot stub
From
Date
On 9/25/20 3:18 PM, Arvind Sankar wrote:
> On Fri, Sep 25, 2020 at 10:56:43AM -0400, Ross Philipson wrote:
>> On 9/24/20 1:38 PM, Arvind Sankar wrote:
>>> On Thu, Sep 24, 2020 at 10:58:35AM -0400, Ross Philipson wrote:
>>>
>>>> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
>>>> index 97d37f0..42043bf 100644
>>>> --- a/arch/x86/boot/compressed/head_64.S
>>>> +++ b/arch/x86/boot/compressed/head_64.S
>>>> @@ -279,6 +279,21 @@ SYM_INNER_LABEL(efi32_pe_stub_entry, SYM_L_LOCAL)
>>>> SYM_FUNC_END(efi32_stub_entry)
>>>> #endif
>>>>
>>>> +#ifdef CONFIG_SECURE_LAUNCH
>>>> +SYM_FUNC_START(sl_stub_entry)
>>>> + /*
>>>> + * On entry, %ebx has the entry abs offset to sl_stub_entry. To
>>>> + * find the beginning of where we are loaded, sub off from the
>>>> + * beginning.
>>>> + */
>>>
>>> This requirement should be added to the documentation. Is it necessary
>>> or can this stub just figure out the address the same way as the other
>>> 32-bit entry points, using the scratch space in bootparams as a little
>>> stack?
>>
>> It is based on the state of the BSP when TXT vectors to the measured
>> launch environment. It is documented in the TXT spec and the SDMs.
>>
>
> I think it would be useful to add to the x86 boot documentation how
> exactly this new entry point is called, even if it's just adding a link
> to some section of those specs. The doc should also say that an
> mle_header_offset of 0 means the kernel isn't secure launch enabled.

Ok will do.

>
>>>
>>> For the 32-bit assembler code that's being added, tip/master now has
>>> changes that prevent the compressed kernel from having any runtime
>>> relocations. You'll need to revise some of the code and the data
>>> structures initial values to avoid creating relocations.
>>
>> Could you elaborate on this some more? I am not sure I see places in the
>> secure launch asm that would be creating relocations like this.
>>
>> Thank you,
>> Ross
>>
>
> You should see them if you do
> readelf -r arch/x86/boot/compressed/vmlinux
>
> In terms of the code, things like:
>
> addl %ebx, (sl_gdt_desc + 2)(%ebx)
>
> will create a relocation, because the linker interprets this as wanting
> the runtime address of sl_gdt_desc, rather than just the offset from
> startup_32.
>
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/tree/arch/x86/boot/compressed/head_64.S*n48__;Iw!!GqivPVa7Brio!JpZWv1cCPZdjD2jbCCGT7P9UIVl_lhX7YjckAnUcvi927jwZI7X3nX0MpIAZOyktJds$
>
> has a comment with some explanation and a macro that the 32-bit code in
> startup_32 uses to avoid creating relocations.
>
> Since the SL code is in a different assembler file (and a different
> section), you can't directly use the same macro. I would suggest getting
> rid of sl_stub_entry and entering directly at sl_stub, and then the code
> in sl_stub.S can use sl_stub for the base address, defining the rva()
> macro there as
>
> #define rva(X) ((X) - sl_stub)
>
> You will also need to avoid initializing data with symbol addresses.
>
> .long mle_header
> .long sl_stub_entry
> .long sl_gdt
>
> will create relocations. The third one is easy, just replace it with
> sl_gdt - sl_gdt_desc and initialize it at runtime with
>
> leal rva(sl_gdt_desc)(%ebx), %eax
> addl %eax, 2(%eax)
> lgdt (%eax)
>
> The other two are more messy, unfortunately there is no easy way to tell
> the linker what we want here. The other entry point addresses (for the
> EFI stub) are populated in a post-processing step after the compressed
> kernel has been linked, we could teach it to also update kernel_info.
>
> Without that, for kernel_info, you could change it to store the offset
> of the MLE header from kernel_info, instead of from the start of the
> image.
>
> For the MLE header, it could be moved to .head.text in head_64.S, and
> initialized with
> .long rva(sl_stub)
> This will also let it be placed at a fixed offset from startup_32, so
> that kernel_info can just be populated with a constant.

Thank you for the detailed reply. I am going to start digging into this now.

Ross

>

\
 
 \ /
  Last update: 2020-09-29 16:05    [W:0.134 / U:0.472 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site