[coreboot-gerrit] New patch to review for coreboot: chromeos: vpd: Avoid reading uninitialized VPDs

Patrick Georgi (pgeorgi@google.com) gerrit at coreboot.org
Tue Jan 26 15:40:32 CET 2016


Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13467

-gerrit

commit ba8579c1e73a8c307901267e87675b1b1f79f344
Author: Julius Werner <jwerner at chromium.org>
Date:   Thu Jan 21 11:12:38 2016 -0800

    chromeos: vpd: Avoid reading uninitialized VPDs
    
    This patch adds a check to the VPD parsing code to avoid reading the
    whole thing if the first byte ('type' of the first VPD entry) is 0x00
    or 0xff. These values match the TERMINATOR and IMPLICIT_TERMINATOR types
    which should never occur as the first entry, so this usually means that
    the VPD FMAP section has simply never been initialized correctly. This
    early abort avoids wasting time to read the whole section from SPI flash
    (which we'd otherwise have to since we're not going to find a Google VPD
    2.0 header either).
    
    BRANCH=None
    BUG=None
    TEST=Booted Oak, confirmed that VPD read times dropped from 100ms to
    1.5ms.
    
    Change-Id: I9fc473e06440aef4e1023238fb9e53d45097ee9d
    Signed-off-by: Patrick Georgi <pgeorgi at chromium.org>
    Original-Commit-Id: 20a726237e03941ad626a6146700170a45ee7720
    Original-Change-Id: I09bfec3c24d24214fa4e9180878b58d00454f399
    Original-Signed-off-by: Julius Werner <jwerner at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/322897
    Original-Reviewed-by: Hung-Te Lin <hungte at chromium.org>
---
 src/vendorcode/google/chromeos/cros_vpd.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/vendorcode/google/chromeos/cros_vpd.c b/src/vendorcode/google/chromeos/cros_vpd.c
index e826d36..d0e2cc1 100644
--- a/src/vendorcode/google/chromeos/cros_vpd.c
+++ b/src/vendorcode/google/chromeos/cros_vpd.c
@@ -65,11 +65,21 @@ static int32_t get_vpd_size(const char *fmap_name, int32_t *base)
 	}
 
 	/* Try if we can find a google_vpd_info, otherwise read whole VPD. */
-	if (rdev_readat(&vpd, &info, *base, sizeof(info)) == sizeof(info) &&
-	    memcmp(info.header.magic, VPD_INFO_MAGIC, sizeof(info.header.magic))
+	if (rdev_readat(&vpd, &info, *base, sizeof(info)) != sizeof(info)) {
+		printk(BIOS_ERR, "ERROR: Failed to read %s header.\n",
+		       fmap_name);
+		return 0;
+	}
+
+	if (memcmp(info.header.magic, VPD_INFO_MAGIC, sizeof(info.header.magic))
 	    == 0 && size >= info.size + sizeof(info)) {
 		*base += sizeof(info);
 		size = info.size;
+	} else if (info.header.tlv.type == VPD_TYPE_TERMINATOR ||
+		   info.header.tlv.type == VPD_TYPE_IMPLICIT_TERMINATOR) {
+		printk(BIOS_WARNING, "WARNING: %s is uninitialized or empty.\n",
+		       fmap_name);
+		size = 0;
 	} else {
 		size -= GOOGLE_VPD_2_0_OFFSET;
 	}



More information about the coreboot-gerrit mailing list