Skip to content

Commit

Permalink
Check the Section Headers in Non-Secure Mode
Browse files Browse the repository at this point in the history 
When a PNOR section without a header is flashed onto a system that
doesn't have SECUREBOOT compiled in, no header checks are performed,
but the code still acts as if the header is present, and so the
virtual address of the section is set to point past the secure
header, which is 0x1000 into the section image, which causes all
kinds of issues. This change adds logic to check the headers even
when Secure Boot features are compiled out.

Change-Id: Ieece371014192f160273939a35cb175aef0ddb25
Resolves: #126
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/54831
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Michael Baiocchi <mbaiocch@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
  • Loading branch information
@dcrowell77
Ilya Smirnov authored and dcrowell77 committed Mar 12, 2018
1 parent e4a7de3 commit c82b626
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 39 deletions.
4 changes: 3 additions & 1 deletion src/include/usr/pnor/pnor_reasoncodes.H
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/* */
/* OpenPOWER HostBoot Project */
/* */
/* Contributors Listed Below - COPYRIGHT 2011,2017 */
/* Contributors Listed Below - COPYRIGHT 2011,2018 */
/* [+] Google Inc. */
/* [+] International Business Machines Corp. */
/* */
Expand Down Expand Up @@ -96,6 +96,7 @@ 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.
Expand Down Expand Up @@ -187,6 +188,7 @@ 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
Expand Down
12 changes: 11 additions & 1 deletion src/include/usr/pnor/pnorif.H
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/* */
/* OpenPOWER HostBoot Project */
/* */
/* Contributors Listed Below - COPYRIGHT 2011,2017 */
/* Contributors Listed Below - COPYRIGHT 2011,2018 */
/* [+] Google Inc. */
/* [+] International Business Machines Corp. */
/* */
Expand Down Expand Up @@ -225,6 +225,16 @@ 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.
Expand Down
19 changes: 18 additions & 1 deletion src/usr/pnor/pnor_common.C
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/* */
/* OpenPOWER HostBoot Project */
/* */
/* Contributors Listed Below - COPYRIGHT 2014,2017 */
/* Contributors Listed Below - COPYRIGHT 2014,2018 */
/* [+] Google Inc. */
/* [+] International Business Machines Corp. */
/* */
Expand Down Expand Up @@ -406,3 +406,20 @@ 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);
}

24 changes: 1 addition & 23 deletions src/usr/pnor/pnor_utils.C
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/* */
/* OpenPOWER HostBoot Project */
/* */
/* Contributors Listed Below - COPYRIGHT 2011,2017 */
/* Contributors Listed Below - COPYRIGHT 2011,2018 */
/* [+] International Business Machines Corp. */
/* */
/* */
Expand Down Expand Up @@ -328,28 +328,6 @@ 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
Expand Down
73 changes: 60 additions & 13 deletions src/usr/pnor/pnorrp.C
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,8 @@ 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);
Expand All @@ -502,16 +504,17 @@ errlHndl_t PnorRP::getSectionInfo( PNOR::SectionId i_section,
// sections in SPnorRP's address space
if (o_info.secure)
{
uint8_t* l_vaddr = reinterpret_cast<uint8_t*>(iv_TOC[id].virtAddr);
uint8_t* l_vaddrPtr =
reinterpret_cast<uint8_t*>(l_sectionVaddr);
// 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.
o_info.vaddr = reinterpret_cast<uint64_t>(l_vaddr)
+ VMM_VADDR_SPNOR_DELTA
+ VMM_VADDR_SPNOR_DELTA;
l_sectionVaddr = reinterpret_cast<uint64_t>(l_vaddrPtr)
+ 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
Expand All @@ -521,7 +524,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_vaddr);
l_errhdl = l_conHdr.setHeader(l_vaddrPtr);
if (l_errhdl)
{
TRACFCOMP(g_trac_pnor, ERR_MRK"PnorRP::getSectionInfo: setheader failed");
Expand Down Expand Up @@ -557,25 +560,69 @@ errlHndl_t PnorRP::getSectionInfo( PNOR::SectionId i_section,
}

// skip secure header for secure sections at this point in time
o_info.vaddr += PAGESIZE;
l_sectionVaddr += 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
o_info.size -= PAGESIZE;
l_sectionSize -= PAGESIZE;

// cache the value in SectionInfo struct so that we can
// parse the container header less often
o_info.secureProtectedPayloadSize = payloadTextSize;
}
else
#endif
#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)
{
o_info.vaddr = iv_TOC[id].virtAddr;
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;
}

#endif
o_info.flashAddr = iv_TOC[id].flashAddr;
o_info.size = iv_TOC[id].size;
o_info.size = l_sectionSize;
o_info.vaddr = l_sectionVaddr;
o_info.eccProtected = ((iv_TOC[id].integrity & FFS_INTEG_ECC_PROTECT)
!= 0) ? true : false;
o_info.sha512Version = ((iv_TOC[id].version & FFS_VERS_SHA512)
Expand Down
7 changes: 7 additions & 0 deletions src/usr/pnor/runtime/rt_pnor.C
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,13 @@ 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);
Expand Down

0 comments on commit c82b626

Please sign in to comment.