Wrap CMOS read/write functions so they automatically pacify the nvram driver.factory-1284.B

A legacy checksum in CMOS is not maintained by coreboot and may be invalid. If
the Linux kernel driver sees that the checksum is wrong, it will return EIO
when read from or written to. That makes crossystem return an error since it
can't read the CMOS, and it also prevents it from changing some settings.

One way to fix the problem would be to remove the checksum check in the kernel
driver. This would change the semantics of the driver so that either x86 was
inconsistent with the other architectures, or change the semantics of those
other architectures as well.

Another option would be to fix the checksum during manufacturing since nothing
should be changing those particular bytes of CMOS. The problem with this
approach is that something might corrupt the CMOS after manufacturing, and
we'd have the same problem again.

Yet another solution would be to make the firmware, most likely coreboot,
actually keep the checksum up to date. This seems like an awful waste of boot
time, the bytes protected by the checksum aren't actually used by anything,
and the bytes of the CMOS that are used are protected by vboot using its own
checksum.

The solution implemented here is to make crossystem recognize when the driver
has determined that the checksum is invalid and make it call an ioctl that
gets the driver to fix the checksum. Wrapper functions for fread, fwrite,
fgetc, and fputc are implemented which first attempt to read or write, on
failure check for the EIO error code, and if they find it to call the
appropriate ioctl and attempt the access again. This way, we won't take any
extra time to talk to the CMOS when everything is working properly, and when
there's a problem it gets fixed up transparently. One problem with this
approach is that using the /dev/nvram device file will still fail until
crossystem is run at least once and given a chance to fix the checksum.

BUG=chrome-os-partner:6718
TEST=For version 1, verified that crossystem reported an error on Sameer's
Lumpy. Used strace to verify that crossystem received an EIO error when trying
to read or write /dev/nvram. Built a new image with this change and booted it
using a USB stick. Ran crossystem and verified that crossystem no longer
reported an error. Rebooted into the original image and verified that
crossystem worked there as well, indicating that the persistent problem in the
CMOS had been corrected.

For version 2, the same as version 1 except that I used a custom version of
u-boot to purposefully corrupt the CMOS rather than using Sameer's Lumpy.

Change-Id: I929535bd2a7d666e41a707b6b24c3f0b0df1147f
Signed-off-by: Gabe Black <gabeblack@google.com>
Reviewed-on: https://gerrit.chromium.org/gerrit/11373
Tested-by: Gabe Black <gabeblack@chromium.org>
Reviewed-by: Stefan Reinauer <reinauer@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/11556

1 files changed, 67 insertions, 44 deletions

diff --git a/host/arch/x86/lib/crossystem_arch.c b/host/arch/x86/lib/crossystem_arch.c
index 9feb6c37..92f0b9f5 100644
--- a/host/arch/x86/lib/crossystem_arch.c
+++ b/host/arch/x86/lib/crossystem_arch.c
@@ -5,8 +5,11 @@
 
 #include <ctype.h>
 #include <dirent.h>
+#include <errno.h>
+#include <linux/nvram.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -85,8 +88,62 @@
 #define NEED_FWUPDATE_PATH "/mnt/stateful_partition/.need_firmware_update"
 
 
-int VbReadNvStorage(VbNvContext* vnc) {
+static void VbFixCmosChecksum(FILE* file) {
+  int fd = fileno(file);
+  ioctl(fd, NVRAM_SETCKS);
+}
+
+
+static int VbCmosRead(int offs, size_t size, void *ptr) {
+  size_t res;
+  FILE* f;
+
+  f = fopen(NVRAM_PATH, "rb");
+  if (!f)
+    return -1;
+
+  if (0 != fseek(f, offs, SEEK_SET)) {
+    fclose(f);
+    return -1;
+  }
+
+  res = fread(ptr, size, 1, f);
+  if (1 != res && errno == EIO && ferror(f)) {
+    VbFixCmosChecksum(f);
+    res = fread(ptr, size, 1, f);
+  }
+  if (1 != res) {
+    fclose(f);
+    return -1;
+  }
+
+  return 0;
+}
+
+
+static int VbCmosWrite(int offs, size_t size, const void *ptr) {
+  size_t res;
   FILE* f;
+
+  f = fopen(NVRAM_PATH, "w+b");
+  if (!f)
+    return -1;
+
+  res = fwrite(ptr, size, 1, f);
+  if (1 != res && errno == EIO && ferror(f)) {
+    VbFixCmosChecksum(f);
+    res = fwrite(ptr, size, 1, f);
+  }
+  if (1 != res) {
+    fclose(f);
+    return -1;
+  }
+
+  return 0;
+}
+
+
+int VbReadNvStorage(VbNvContext* vnc) {
   int offs;
 
   /* Get the byte offset from VBNV */
@@ -96,23 +153,14 @@ int VbReadNvStorage(VbNvContext* vnc) {
   if (VBNV_BLOCK_SIZE > ReadFileInt(ACPI_VBNV_PATH ".1"))
     return -1;  /* NV storage block is too small */
 
-  f = fopen(NVRAM_PATH, "rb");
-  if (!f)
+  if (0 != VbCmosRead(offs, VBNV_BLOCK_SIZE, vnc->raw))
     return -1;
 
-  if (0 != fseek(f, offs, SEEK_SET) ||
-      1 != fread(vnc->raw, VBNV_BLOCK_SIZE, 1, f)) {
-    fclose(f);
-    return -1;
-  }
-
-  fclose(f);
   return 0;
 }
 
 
 int VbWriteNvStorage(VbNvContext* vnc) {
-  FILE* f;
   int offs;
 
   if (!vnc->raw_changed)
@@ -125,17 +173,9 @@ int VbWriteNvStorage(VbNvContext* vnc) {
   if (VBNV_BLOCK_SIZE > ReadFileInt(ACPI_VBNV_PATH ".1"))
     return -1;  /* NV storage block is too small */
 
-  f = fopen(NVRAM_PATH, "w+b");
-  if (!f)
-    return -1;
-
-  if (0 != fseek(f, offs, SEEK_SET) ||
-      1 != fwrite(vnc->raw, VBNV_BLOCK_SIZE, 1, f)) {
-    fclose(f);
+  if (0 != VbCmosWrite(offs, VBNV_BLOCK_SIZE, vnc->raw))
     return -1;
-  }
 
-  fclose(f);
   return 0;
 }
 
@@ -272,24 +312,17 @@ VbSharedDataHeader* VbSharedDataRead(void) {
  *
  * Returns 0 if the mask is clear in the field, 1 if set, or -1 if error. */
 static int VbGetCmosRebootField(uint8_t mask) {
-  FILE* f;
-  int chnv, nvbyte;
+  int chnv;
+  uint8_t nvbyte;
 
   /* Get the byte offset from CHNV */
   chnv = ReadFileInt(ACPI_CHNV_PATH);
   if (chnv == -1)
     return -1;
 
-  f = fopen(NVRAM_PATH, "rb");
-  if (!f)
-    return -1;
-
-  if (0 != fseek(f, chnv, SEEK_SET) || EOF == (nvbyte = fgetc(f))) {
-    fclose(f);
+  if (0 != VbCmosRead(chnv, 1, &nvbyte))
     return -1;
-  }
 
-  fclose(f);
   return (nvbyte & mask ? 1 : 0);
 }
 
@@ -300,23 +333,16 @@ static int VbGetCmosRebootField(uint8_t mask) {
  *
  * Returns 0 if success, or -1 if error. */
 static int VbSetCmosRebootField(uint8_t mask, int value) {
-  FILE* f;
-  int chnv, nvbyte;
+  int chnv;
+  uint8_t nvbyte;
 
   /* Get the byte offset from CHNV */
   chnv = ReadFileInt(ACPI_CHNV_PATH);
   if (chnv == -1)
     return -1;
 
-  f = fopen(NVRAM_PATH, "w+b");
-  if (!f)
-    return -1;
-
-  /* Read the current value */
-  if (0 != fseek(f, chnv, SEEK_SET) || EOF == (nvbyte = fgetc(f))) {
-    fclose(f);
+  if (0 != VbCmosRead(chnv, 1, &nvbyte))
     return -1;
-  }
 
   /* Set/clear the mask */
   if (value)
@@ -325,13 +351,10 @@ static int VbSetCmosRebootField(uint8_t mask, int value) {
     nvbyte &= ~mask;
 
   /* Write the byte back */
-  if (0 != fseek(f, chnv, SEEK_SET) || EOF == (fputc(nvbyte, f))) {
-    fclose(f);
+  if (0 != VbCmosWrite(chnv, 1, &nvbyte))
     return -1;
-  }
 
   /* Success */
-  fclose(f);
   return 0;
 }