Check the Section Headers in Non-Secure Mode

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>

4 files changed, 86 insertions, 37 deletions

diff --git a/src/usr/pnor/pnor_common.C b/src/usr/pnor/pnor_common.C
index d262ebe82..ceb7709b8 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,2017                        */
+/* Contributors Listed Below - COPYRIGHT 2014,2018                        */
 /* [+] Google Inc.                                                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
@@ -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);
+}
+
diff --git a/src/usr/pnor/pnor_utils.C b/src/usr/pnor/pnor_utils.C
index 9e0753066..4fcad21c6 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,2017                        */
+/* Contributors Listed Below - COPYRIGHT 2011,2018                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -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
diff --git a/src/usr/pnor/pnorrp.C b/src/usr/pnor/pnorrp.C
index e33a1b0c3..1262db0b8 100644
--- a/src/usr/pnor/pnorrp.C
+++ b/src/usr/pnor/pnorrp.C
@@ -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);
@@ -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
@@ -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");
@@ -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)
diff --git a/src/usr/pnor/runtime/rt_pnor.C b/src/usr/pnor/runtime/rt_pnor.C
index 47accc8d0..9d61e304e 100644
--- a/src/usr/pnor/runtime/rt_pnor.C
+++ b/src/usr/pnor/runtime/rt_pnor.C
@@ -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);