On May 5, 2017 2:26:39 AM PDT, Gary Lin <g...@suse.com> 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.biesheu...@linaro.org> >Cc: "H. Peter Anvin" <h...@zytor.com> >Cc: Thomas Gleixner <t...@linutronix.de> >Cc: Ingo Molnar <mi...@redhat.com> >Cc: Joey Lee <j...@suse.com> >Signed-off-by: Gary Lin <g...@suse.com> >--- > 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 >+sec_distro: >+ .word CONFIG_SEC_DISTRO >+sec_version: >+ .word CONFIG_SEC_VERSION >+secdata_end: >diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld >index 96a6c7563538..43ddbaabaf7a 100644 >--- a/arch/x86/boot/setup.ld >+++ b/arch/x86/boot/setup.ld >@@ -18,6 +18,7 @@ SECTIONS > .entrytext : { *(.entrytext) } > .inittext : { *(.inittext) } > .initdata : { *(.initdata) } >+ .secdata : { *(.secdata) } > __end_init = .; > > .text : { *(.text) } >diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c >index 0702d2531bc7..ec4b311d7b2d 100644 >--- a/arch/x86/boot/tools/build.c >+++ b/arch/x86/boot/tools/build.c >@@ -287,6 +287,15 @@ static inline int reserve_pecoff_reloc_section(int >c) > } > #endif /* CONFIG_EFI_STUB */ > >+static void security_fields_update(void) >+{ >+ unsigned int security_offset; >+ char *dest; >+ >+ security_offset = get_unaligned_le32(&buf[0x268]); >+ dest = (char *)&buf[security_offset + 4]; >+ strncpy(dest, CONFIG_SEC_SIGNER, 8); >+} > > /* >* Parse zoffset.h and find the entry points. We could just #include >zoffset.h >@@ -401,6 +410,8 @@ int main(int argc, char ** argv) > > efi_stub_entry_update(); > >+ security_fields_update(); >+ > crc = partial_crc32(buf, i, crc); > if (fwrite(buf, 1, i, dest) != i) > die("Writing setup failed"); >diff --git a/arch/x86/include/uapi/asm/bootparam.h >b/arch/x86/include/uapi/asm/bootparam.h >index 07244ea16765..713ae5d933a4 100644 >--- a/arch/x86/include/uapi/asm/bootparam.h >+++ b/arch/x86/include/uapi/asm/bootparam.h >@@ -85,6 +85,7 @@ struct setup_header { > __u64 pref_address; > __u32 init_size; > __u32 handover_offset; >+ __u32 secdata_offset; > } __attribute__((packed)); > > struct sys_desc_table {
There is no need to burn a 4-byte field if this is inside the setup area, as that is limited to ~32K anyway. Since this structure is a static ABI, it should have a way to grow it (a length field is usually a good choice.) What are the semantics of these fields? If they are supposed to be distribution-independent, they need semantics defined, otherwise they might better be a distro-specific use field. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
