Message ID | 20180319055912.12584-1-sam@mendozajonas.com |
---|---|
State | Accepted |
Headers | show |
Series | core: Correctly load initramfs in stb container | expand |
On 19/03/2018 02:59, Samuel Mendoza-Jonas wrote: > Skiboot does not calculate the actual size and start location of the > initramfs if it is wrapped by an STB container (for example if loading > an initramfs from the ROOTFS partition). > > Check if the initramfs is in an STB container and determine the size and > location correctly in the same manner as the kernel. Since > load_initramfs() is called after load_kernel() move the call to > trustedboot_exit_boot_services() into load_and_boot_kernel() so it is > called after both of these. > > Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> > --- > I'm pretty sure moving trustedboot_exit_boot_services() into > load_and_boot_kernel() is correct, but it would be great for a secure > boot person to confirm that :) Looks good. The trustedboot_exit_boot_services() should be called only at the point where skiboot has already verified and measured all the PNOR partitions that skiboot will consume. Claudio > > core/flash.c | 3 ++- > core/init.c | 26 ++++++++++++++++++++++---- > 2 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/core/flash.c b/core/flash.c > index d526ef33..c6ceb2c4 100644 > --- a/core/flash.c > +++ b/core/flash.c > @@ -751,7 +751,8 @@ static int flash_load_resource(enum resource_id id, uint32_t subid, > * Back to the old way of doing things, no STB header. > */ > if (subid == RESOURCE_SUBID_NONE) { > - if (id == RESOURCE_ID_KERNEL) { > + if (id == RESOURCE_ID_KERNEL || > + id == RESOURCE_ID_INITRAMFS) { > /* > * Because actualSize is a lie, we compute the > * size of the BOOTKERNEL based on what the ELF > diff --git a/core/init.c b/core/init.c > index a60b7843..ae429940 100644 > --- a/core/init.c > +++ b/core/init.c > @@ -428,13 +428,13 @@ static bool load_kernel(void) > SECURE_BOOT_HEADERS_SIZE + kernel_size); > } > > - trustedboot_exit_boot_services(); > - > return true; > } > > static void load_initramfs(void) > { > + uint64_t *initramfs_start; > + void *stb_container = NULL; > int loaded; > > loaded = wait_for_resource_loaded(RESOURCE_ID_INITRAMFS, > @@ -443,15 +443,31 @@ static void load_initramfs(void) > if (loaded != OPAL_SUCCESS || !initramfs_size) > return; > > + if (stb_is_container(INITRAMFS_LOAD_BASE, initramfs_size)) { > + stb_container = INITRAMFS_LOAD_BASE; > + initramfs_start = INITRAMFS_LOAD_BASE + SECURE_BOOT_HEADERS_SIZE; > + } else { > + initramfs_start = INITRAMFS_LOAD_BASE; > + } > + > dt_check_del_prop(dt_chosen, "linux,initrd-start"); > dt_check_del_prop(dt_chosen, "linux,initrd-end"); > > printf("INIT: Initramfs loaded, size: %zu bytes\n", initramfs_size); > > dt_add_property_u64(dt_chosen, "linux,initrd-start", > - (uint64_t)INITRAMFS_LOAD_BASE); > + (uint64_t)initramfs_start); > dt_add_property_u64(dt_chosen, "linux,initrd-end", > - (uint64_t)INITRAMFS_LOAD_BASE + initramfs_size); > + (uint64_t)initramfs_start + initramfs_size); > + > + if (chip_quirk(QUIRK_MAMBO_CALLOUTS)) { > + secureboot_verify(RESOURCE_ID_INITRAMFS, > + stb_container, > + SECURE_BOOT_HEADERS_SIZE + initramfs_size); > + trustedboot_measure(RESOURCE_ID_INITRAMFS, > + stb_container, > + SECURE_BOOT_HEADERS_SIZE + initramfs_size); > + } > } > > int64_t mem_dump_free(void); > @@ -484,6 +500,8 @@ void __noreturn load_and_boot_kernel(bool is_reboot) > > load_initramfs(); > > + trustedboot_exit_boot_services(); > + > ipmi_set_fw_progress_sensor(IPMI_FW_OS_BOOT); > > occ_pstates_init();
Samuel Mendoza-Jonas <sam@mendozajonas.com> writes: > Skiboot does not calculate the actual size and start location of the > initramfs if it is wrapped by an STB container (for example if loading > an initramfs from the ROOTFS partition). > > Check if the initramfs is in an STB container and determine the size and > location correctly in the same manner as the kernel. Since > load_initramfs() is called after load_kernel() move the call to > trustedboot_exit_boot_services() into load_and_boot_kernel() so it is > called after both of these. > > Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> Merged to master as of e7a2da8d5e654467df63c03e2be91b435796df25
diff --git a/core/flash.c b/core/flash.c index d526ef33..c6ceb2c4 100644 --- a/core/flash.c +++ b/core/flash.c @@ -751,7 +751,8 @@ static int flash_load_resource(enum resource_id id, uint32_t subid, * Back to the old way of doing things, no STB header. */ if (subid == RESOURCE_SUBID_NONE) { - if (id == RESOURCE_ID_KERNEL) { + if (id == RESOURCE_ID_KERNEL || + id == RESOURCE_ID_INITRAMFS) { /* * Because actualSize is a lie, we compute the * size of the BOOTKERNEL based on what the ELF diff --git a/core/init.c b/core/init.c index a60b7843..ae429940 100644 --- a/core/init.c +++ b/core/init.c @@ -428,13 +428,13 @@ static bool load_kernel(void) SECURE_BOOT_HEADERS_SIZE + kernel_size); } - trustedboot_exit_boot_services(); - return true; } static void load_initramfs(void) { + uint64_t *initramfs_start; + void *stb_container = NULL; int loaded; loaded = wait_for_resource_loaded(RESOURCE_ID_INITRAMFS, @@ -443,15 +443,31 @@ static void load_initramfs(void) if (loaded != OPAL_SUCCESS || !initramfs_size) return; + if (stb_is_container(INITRAMFS_LOAD_BASE, initramfs_size)) { + stb_container = INITRAMFS_LOAD_BASE; + initramfs_start = INITRAMFS_LOAD_BASE + SECURE_BOOT_HEADERS_SIZE; + } else { + initramfs_start = INITRAMFS_LOAD_BASE; + } + dt_check_del_prop(dt_chosen, "linux,initrd-start"); dt_check_del_prop(dt_chosen, "linux,initrd-end"); printf("INIT: Initramfs loaded, size: %zu bytes\n", initramfs_size); dt_add_property_u64(dt_chosen, "linux,initrd-start", - (uint64_t)INITRAMFS_LOAD_BASE); + (uint64_t)initramfs_start); dt_add_property_u64(dt_chosen, "linux,initrd-end", - (uint64_t)INITRAMFS_LOAD_BASE + initramfs_size); + (uint64_t)initramfs_start + initramfs_size); + + if (chip_quirk(QUIRK_MAMBO_CALLOUTS)) { + secureboot_verify(RESOURCE_ID_INITRAMFS, + stb_container, + SECURE_BOOT_HEADERS_SIZE + initramfs_size); + trustedboot_measure(RESOURCE_ID_INITRAMFS, + stb_container, + SECURE_BOOT_HEADERS_SIZE + initramfs_size); + } } int64_t mem_dump_free(void); @@ -484,6 +500,8 @@ void __noreturn load_and_boot_kernel(bool is_reboot) load_initramfs(); + trustedboot_exit_boot_services(); + ipmi_set_fw_progress_sensor(IPMI_FW_OS_BOOT); occ_pstates_init();
Skiboot does not calculate the actual size and start location of the initramfs if it is wrapped by an STB container (for example if loading an initramfs from the ROOTFS partition). Check if the initramfs is in an STB container and determine the size and location correctly in the same manner as the kernel. Since load_initramfs() is called after load_kernel() move the call to trustedboot_exit_boot_services() into load_and_boot_kernel() so it is called after both of these. Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> --- I'm pretty sure moving trustedboot_exit_boot_services() into load_and_boot_kernel() is correct, but it would be great for a secure boot person to confirm that :) core/flash.c | 3 ++- core/init.c | 26 ++++++++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-)