| From 7a948065fdb903e280757eb92c70b5594298ce56 Mon Sep 17 00:00:00 2001 |
| From: Stewart Smith <stewart@linux.vnet.ibm.com> |
| Date: Thu, 15 Mar 2018 18:45:54 +1100 |
| Subject: [PATCH] Revert "Check the Section Headers in Non-Secure Mode" |
| |
| This reverts commit c82b626e6ea1d56c0d25cbd5954064e256135002. |
| |
| It breaks flashing custom skiboot (PAYLOAD) as the code checking |
| headers checks the name of the partition against the secureboot |
| header magic number, and this obviously does not work at all. |
| |
| This ends up being the case as I *believe* that older pflash |
| may clear FFS_VERS_SHA512 flag that causes hostboot to go down |
| code paths it doesn't expect. |
| |
| As always, FFS structures are full of lies :) |
| |
| Change-Id: I6f3994cd1ceb67a17b53487a45b26a9e4c10270e |
| Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com> |
| --- |
| src/include/usr/pnor/pnor_reasoncodes.H | 4 +- |
| src/include/usr/pnor/pnorif.H | 12 +----- |
| src/usr/pnor/pnor_common.C | 19 +-------- |
| src/usr/pnor/pnor_utils.C | 24 ++++++++++- |
| src/usr/pnor/pnorrp.C | 73 ++++++--------------------------- |
| src/usr/pnor/runtime/rt_pnor.C | 7 ---- |
| 6 files changed, 39 insertions(+), 100 deletions(-) |
| |
| diff --git a/src/include/usr/pnor/pnor_reasoncodes.H b/src/include/usr/pnor/pnor_reasoncodes.H |
| index e0c156e746de..589337a3be47 100644 |
| --- a/src/include/usr/pnor/pnor_reasoncodes.H |
| +++ b/src/include/usr/pnor/pnor_reasoncodes.H |
| @@ -5,7 +5,7 @@ |
| /* */ |
| /* OpenPOWER HostBoot Project */ |
| /* */ |
| -/* Contributors Listed Below - COPYRIGHT 2011,2018 */ |
| +/* Contributors Listed Below - COPYRIGHT 2011,2017 */ |
| /* [+] Google Inc. */ |
| /* [+] International Business Machines Corp. */ |
| /* */ |
| @@ -96,7 +96,6 @@ namespace PNOR |
| |
| // pnor_common.C |
| MOD_PNORCOMMON_PARSETOC = 0xC0, /**< PNOR::parseTOC */ |
| - MOD_PNORCOMMON_GETSECTIONINFO = 0xC1, /**< PNOR::getSectionInfo */ |
| |
| // spnorrp.C |
| // Note: 0xD0 is available, so should be the next one used for spnorrp. |
| @@ -188,7 +187,6 @@ namespace PNOR |
| RC_SECURE_SIZE_MISMATCH = PNOR_COMP_ID | 0x3A, |
| RC_NOT_PAGE_ALIGNED = PNOR_COMP_ID | 0x3B, |
| RC_SECURE_PRO_SIZE_MISMATCH = PNOR_COMP_ID | 0x3C, |
| - RC_BAD_HEADER_FORMAT = PNOR_COMP_ID | 0x3D, |
| |
| //@fixme-RTC:131607-Temporary value to allow HWSV compile |
| //termination_rc |
| diff --git a/src/include/usr/pnor/pnorif.H b/src/include/usr/pnor/pnorif.H |
| index b4d1a668fe8b..cef8617fdcb1 100644 |
| --- a/src/include/usr/pnor/pnorif.H |
| +++ b/src/include/usr/pnor/pnorif.H |
| @@ -5,7 +5,7 @@ |
| /* */ |
| /* OpenPOWER HostBoot Project */ |
| /* */ |
| -/* Contributors Listed Below - COPYRIGHT 2011,2018 */ |
| +/* Contributors Listed Below - COPYRIGHT 2011,2017 */ |
| /* [+] Google Inc. */ |
| /* [+] International Business Machines Corp. */ |
| /* */ |
| @@ -225,16 +225,6 @@ const char * SectionIdToString( uint32_t i_secIdIndex ); |
| */ |
| bool cmpSecurebootMagicNumber(const uint8_t* i_vaddr); |
| |
| -/** |
| - * @brief Determines whether requested PNOR section has a recognized header |
| - * @param[in] i_vaddr: vaddr of the beginning of the secureboot header. |
| - * @param[in] o_magicNumber: the read value of the header's magic number. |
| - Used for error logging purposes. Always populated. |
| - * @return bool: True if the header was recognized, false otherwise. |
| - */ |
| -bool hasKnownHeader(const uint8_t* i_vaddr, |
| - uint64_t& o_magicNumber); |
| - |
| /** |
| * @brief Determine if a PNOR section is empty by checking if first PAGE |
| * is all 0xFF's or 0x00's depending on ECC or not. |
| diff --git a/src/usr/pnor/pnor_common.C b/src/usr/pnor/pnor_common.C |
| index ceb7709b8c54..d262ebe8238f 100644 |
| --- a/src/usr/pnor/pnor_common.C |
| +++ b/src/usr/pnor/pnor_common.C |
| @@ -5,7 +5,7 @@ |
| /* */ |
| /* OpenPOWER HostBoot Project */ |
| /* */ |
| -/* Contributors Listed Below - COPYRIGHT 2014,2018 */ |
| +/* Contributors Listed Below - COPYRIGHT 2014,2017 */ |
| /* [+] Google Inc. */ |
| /* [+] International Business Machines Corp. */ |
| /* */ |
| @@ -406,20 +406,3 @@ bool PNOR::isSectionEmpty(const PNOR::SectionId i_section) |
| |
| return l_result; |
| } |
| - |
| -bool PNOR::hasKnownHeader(const uint8_t* i_vaddr, |
| - uint64_t& o_magicNumber) |
| -{ |
| - // Left symbolic constant defined in the function so it's easier to strip |
| - // out later and nothing becomes dependent on it |
| - const char VERSION_MAGIC[] = "VERSION"; |
| - const auto versionMagicSize = sizeof(VERSION_MAGIC); |
| - |
| - bool secureHeader = PNOR::cmpSecurebootMagicNumber(i_vaddr); |
| - bool versionHeader = (memcmp(i_vaddr,VERSION_MAGIC,versionMagicSize) == 0); |
| - |
| - memcpy(&o_magicNumber, i_vaddr, sizeof(o_magicNumber)); |
| - |
| - return (versionHeader || secureHeader); |
| -} |
| - |
| diff --git a/src/usr/pnor/pnor_utils.C b/src/usr/pnor/pnor_utils.C |
| index 4fcad21c635f..9e0753066a92 100644 |
| --- a/src/usr/pnor/pnor_utils.C |
| +++ b/src/usr/pnor/pnor_utils.C |
| @@ -5,7 +5,7 @@ |
| /* */ |
| /* OpenPOWER HostBoot Project */ |
| /* */ |
| -/* Contributors Listed Below - COPYRIGHT 2011,2018 */ |
| +/* Contributors Listed Below - COPYRIGHT 2011,2017 */ |
| /* [+] International Business Machines Corp. */ |
| /* */ |
| /* */ |
| @@ -328,6 +328,28 @@ PNOR::parseEntries (ffs_hdr* i_ffs_hdr, |
| #else |
| io_TOC[secId].secure = false; |
| #endif |
| + |
| + // If secureboot is compiled in, skip header if not a secure section |
| + // Otherwise always skip header as the secure flag is always false and |
| + // SpnorRp will not handle skipping the header if one is indicated in PNOR |
| + if ( (io_TOC[secId].version & FFS_VERS_SHA512) |
| + && !io_TOC[secId].secure) |
| + { |
| + //increment flash addr for sha header |
| + if (io_TOC[secId].integrity == FFS_INTEG_ECC_PROTECT) |
| + { |
| + io_TOC[secId].flashAddr += PAGESIZE_PLUS_ECC ; |
| + } |
| + else |
| + { |
| + io_TOC[secId].flashAddr += PAGESIZE ; |
| + } |
| + |
| + // now that we've skipped the header |
| + // adjust the size to reflect that |
| + io_TOC[secId].size -= PAGESIZE; |
| + } |
| + |
| } // For TOC Entries |
| |
| #ifndef BOOTLOADER |
| diff --git a/src/usr/pnor/pnorrp.C b/src/usr/pnor/pnorrp.C |
| index 1262db0b889f..e33a1b0c377c 100644 |
| --- a/src/usr/pnor/pnorrp.C |
| +++ b/src/usr/pnor/pnorrp.C |
| @@ -489,8 +489,6 @@ errlHndl_t PnorRP::getSectionInfo( PNOR::SectionId i_section, |
| { |
| TRACDCOMP( g_trac_pnor, "PnorRP::getSectionInfo: i_section=%d, id=%d", i_section, iv_TOC[i_section].id ); |
| |
| - uint64_t l_sectionVaddr = iv_TOC[id].virtAddr; |
| - uint64_t l_sectionSize = iv_TOC[id].size; |
| // copy my data into the external format |
| o_info.id = iv_TOC[id].id; |
| o_info.name = SectionIdToString(id); |
| @@ -504,17 +502,16 @@ errlHndl_t PnorRP::getSectionInfo( PNOR::SectionId i_section, |
| // sections in SPnorRP's address space |
| if (o_info.secure) |
| { |
| - uint8_t* l_vaddrPtr = |
| - reinterpret_cast<uint8_t*>(l_sectionVaddr); |
| + uint8_t* l_vaddr = reinterpret_cast<uint8_t*>(iv_TOC[id].virtAddr); |
| // By adding VMM_VADDR_SPNOR_DELTA twice we can translate a pnor |
| - // address into a secure pnor address, since pnor, temp, and |
| - // spnor spaces are equidistant. |
| + // address into a secure pnor address, since pnor, temp, and spnor |
| + // spaces are equidistant. |
| // See comments in SPnorRP::verifySections() method in spnorrp.C |
| // and the definition of VMM_VADDR_SPNOR_DELTA in vmmconst.h |
| // for specifics. |
| - l_sectionVaddr = reinterpret_cast<uint64_t>(l_vaddrPtr) |
| - + VMM_VADDR_SPNOR_DELTA |
| - + VMM_VADDR_SPNOR_DELTA; |
| + o_info.vaddr = reinterpret_cast<uint64_t>(l_vaddr) |
| + + VMM_VADDR_SPNOR_DELTA |
| + + VMM_VADDR_SPNOR_DELTA; |
| |
| // Get size of the secured payload for the secure section |
| // Note: the payloadSize we get back is untrusted because |
| @@ -524,7 +521,7 @@ errlHndl_t PnorRP::getSectionInfo( PNOR::SectionId i_section, |
| // and has valid beginning bytes. For optional Secure PNOR sections. |
| |
| SECUREBOOT::ContainerHeader l_conHdr; |
| - l_errhdl = l_conHdr.setHeader(l_vaddrPtr); |
| + l_errhdl = l_conHdr.setHeader(l_vaddr); |
| if (l_errhdl) |
| { |
| TRACFCOMP(g_trac_pnor, ERR_MRK"PnorRP::getSectionInfo: setheader failed"); |
| @@ -560,69 +557,25 @@ errlHndl_t PnorRP::getSectionInfo( PNOR::SectionId i_section, |
| } |
| |
| // skip secure header for secure sections at this point in time |
| - l_sectionVaddr += PAGESIZE; |
| + o_info.vaddr += PAGESIZE; |
| // now that we've skipped the header we also need to adjust the |
| // size of the section to reflect that. |
| // Note: For unsecured sections, the header skip and size decrement |
| // was done previously in pnor_common.C |
| - l_sectionSize -= PAGESIZE; |
| + o_info.size -= PAGESIZE; |
| |
| // cache the value in SectionInfo struct so that we can |
| // parse the container header less often |
| o_info.secureProtectedPayloadSize = payloadTextSize; |
| } |
| -#else |
| - // If secureboot is not compiled, still check the sections that are |
| - // marked with sha512 tag in the xml to catch sections without fake |
| - // headers. If we expect a header to be present and it's not, |
| - // the virtual address of the section will not be pointing to the |
| - // correct offset into the section. |
| - if(iv_TOC[id].version & FFS_VERS_SHA512) |
| + else |
| +#endif |
| { |
| - uint64_t l_magicNumber = 0; |
| - bool l_knownHeader = PNOR::hasKnownHeader( |
| - reinterpret_cast<uint8_t*>(l_sectionVaddr), |
| - l_magicNumber); |
| - if(!l_knownHeader) |
| - { |
| - TRACFCOMP(g_trac_pnor, ERR_MRK"PnorRP::getSectionInfo: " |
| - "The header of the partition %s" |
| - " is not of a known header format. Magic number" |
| - " = 0x%016llx", |
| - PNOR::SectionIdToString(id), |
| - l_magicNumber); |
| - /*@ |
| - * @errortype ERRORLOG::ERRL_SEV_UNRECOVERABLE |
| - * @moduleid PNOR::MOD_PNORCOMMON_GETSECTIONINFO |
| - * @reasoncode PNOR::RC_BAD_HEADER_FORMAT |
| - * @userdata1 Partition ID |
| - * @userdata2 Partition's magic number |
| - * @devdesc Error parsing partition header |
| - * @custdesc Boot firmware integrity error; |
| - * reinstall the boot firmware |
| - */ |
| - l_errhdl = new ERRORLOG::ErrlEntry( |
| - ERRORLOG::ERRL_SEV_UNRECOVERABLE, |
| - PNOR::MOD_PNORCOMMON_GETSECTIONINFO, |
| - PNOR::RC_BAD_HEADER_FORMAT, |
| - id, |
| - l_magicNumber, |
| - true/*SW Error*/); |
| - l_errhdl->collectTrace(PNOR_COMP_NAME); |
| - l_errhdl->collectTrace(SECURE_COMP_NAME); |
| - break; |
| - } |
| - // Skip the fake header in memory after we've checked it. |
| - // The vaddr of the parition will now point to the start |
| - // of the actual partition. |
| - l_sectionSize -= PAGESIZE; |
| - l_sectionVaddr += PAGESIZE; |
| + o_info.vaddr = iv_TOC[id].virtAddr; |
| } |
| |
| -#endif |
| o_info.flashAddr = iv_TOC[id].flashAddr; |
| - o_info.size = l_sectionSize; |
| - o_info.vaddr = l_sectionVaddr; |
| + o_info.size = iv_TOC[id].size; |
| o_info.eccProtected = ((iv_TOC[id].integrity & FFS_INTEG_ECC_PROTECT) |
| != 0) ? true : false; |
| o_info.sha512Version = ((iv_TOC[id].version & FFS_VERS_SHA512) |
| diff --git a/src/usr/pnor/runtime/rt_pnor.C b/src/usr/pnor/runtime/rt_pnor.C |
| index 02b230456661..ba23cecb5ab4 100644 |
| --- a/src/usr/pnor/runtime/rt_pnor.C |
| +++ b/src/usr/pnor/runtime/rt_pnor.C |
| @@ -263,13 +263,6 @@ errlHndl_t RtPnor::getSectionInfo(PNOR::SectionId i_section, |
| o_info.sha512perEC = |
| (iv_TOC[i_section].version & FFS_VERS_SHA512_PER_EC) ? true : false; |
| o_info.secure = iv_TOC[i_section].secure; |
| -#ifndef CONFIG_SECUREBOOT |
| - if(iv_TOC[i_section].version & FFS_VERS_SHA512) |
| - { |
| - o_info.size -= PAGESIZE; |
| - o_info.vaddr += PAGESIZE; |
| - } |
| -#endif |
| } while (0); |
| |
| TRACFCOMP(g_trac_pnor, EXIT_MRK"RtPnor::getSectionInfo %d", i_section); |
| -- |
| 2.14.3 |
| |