Re: [RFC PATCH] x86/boot: Add the secdata section to the setup header

[hpa]

On May 6, 2017 10:34:49 AM PDT, Brian Gerst <brgerst@xxxxxxxxx> wrote:
>On Fri, May 5, 2017 at 5:26 AM, Gary Lin <glin@xxxxxxxx> wrote:
>> This is a different approach to replace my previous implementation of
>> Security Version(*). Instead of using the fields in the PE/COFF
>header,
>> this commit adds secdata_offset in the setup header for the file
>offset
>> of secdata. Currently, the secdata section contains the signer's
>name,
>> the distro version, and the security version as defined in the wiki
>page.
>> Since we don't rely on the PE/COFF header anymore, the size of signer
>is
>> increased to 8 bytes to store more characters.
>>
>> Since this is just a tentative patch, I haven't started the other
>parts
>> (shim and mokutil) yet, and it's flexible to change. Any comment and
>> suggestion are welcome.
>>
>> (*) https://github.com/lcp/shim/wiki/Security-Version
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
>> Cc: Joey Lee <jlee@xxxxxxxx>
>> Signed-off-by: Gary Lin <glin@xxxxxxxx>
>> ---
>>  arch/x86/Kconfig                      | 14 ++++++++++++++
>>  arch/x86/boot/header.S                | 15 ++++++++++++++-
>>  arch/x86/boot/setup.ld                |  1 +
>>  arch/x86/boot/tools/build.c           | 11 +++++++++++
>>  arch/x86/include/uapi/asm/bootparam.h |  1 +
>>  5 files changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 5bbdef151805..09f99cd1e699 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1817,6 +1817,20 @@ config EFI_MIXED
>>
>>            If unsure, say N.
>>
>> +config SEC_SIGNER
>> +       string "The signer name"
>> +       default "none"
>> +
>> +config SEC_DISTRO
>> +       int "The distro version"
>> +       default 0
>> +       range 0 65535
>> +
>> +config SEC_VERSION
>> +       int "The security version"
>> +       default 0
>> +       range 0 65535
>> +
>>  config SECCOMP
>>         def_bool y
>>         prompt "Enable seccomp to safely compute untrusted bytecode"
>> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>> index 3dd5be33aaa7..f751790f1f44 100644
>> --- a/arch/x86/boot/header.S
>> +++ b/arch/x86/boot/header.S
>> @@ -301,7 +301,7 @@ _start:
>>         # Part 2 of the header, from the old setup.S
>>
>>                 .ascii  "HdrS"          # header signature
>> -               .word   0x020d          # header version number (>=
>0x0105)
>> +               .word   0x020e          # header version number (>=
>0x0105)
>>                                         # or else old loadlin-1.5
>will fail)
>>                 .globl realmode_swtch
>>  realmode_swtch:        .word   0, 0            # default_switch,
>SETUPSEG
>> @@ -552,6 +552,7 @@ pref_address:               .quad
>LOAD_PHYSICAL_ADDR        # preferred load addr
>>
>>  init_size:             .long INIT_SIZE         # kernel
>initialization size
>>  handover_offset:       .long 0                 # Filled in by
>build.c
>> +secdata_offset:                .long secdata_start
>>
>>  # End of setup header
>#####################################################
>>
>> @@ -629,3 +630,15 @@ die:
>>  setup_corrupt:
>>         .byte   7
>>         .string "No setup signature found...\n"
>> +
>> +       .section ".secdata", "a"
>> +secdata_start:
>> +sec_length:
>> +       .long   secdata_end - secdata_start
>> +sec_signer:
>> +       .quad   0                               # Filled by build.c
>
>A more flexible way to do this would be to make sec_signer an offset
>to a null-terminated string.  That way it can be any length, and you
>wouldn't need the build tool hack.
>
>--
>Brian Gerst

Also, there is no fundamental reason this has to be a fixed structure; it could directly contain variable width entries as long as it remains parsable.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.