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

[Brian Gerst]

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