[RFC] Add ability to lock bootblock on Winbond Flash ROMs

Submitted by tpearson@raptorengineering.com on Aug. 23, 2016, 9:32 p.m.

Details

Message ID 809516223.92250.1471987969563.JavaMail.zimbra@raptorengineeringinc.com
State New
Headers show

Commit Message

tpearson@raptorengineering.com Aug. 23, 2016, 9:32 p.m.
RFC PATCH

TPM-enabled systems require an immutable (or at least very difficult to modify) CRTM, which can be provided via a locked bootblock sector.

Add initial support for locking the upper 4K sector on Winbond Flash ROMs.  Unlocking the sector after it has been locked requires hardware access to the write protect pin, satisfying the above requirements for many use cases.

To use, pass the --lock flag to Flashrom.  This flag can be used in isolation, or as part of a normal read / write / verify cycle.

Comments

David Hendricks Aug. 24, 2016, 12:12 a.m.
Hi Timothy,
Check out Hatim's GSOC work for manipulating status register and write
protect bits:
https://patchwork.coreboot.org/patch/4471/
https://patchwork.coreboot.org/patch/4472/
https://patchwork.coreboot.org/patch/4469/
https://patchwork.coreboot.org/patch/4467/
https://patchwork.coreboot.org/patch/4468/
https://patchwork.coreboot.org/patch/4470/

There's also similar work done in Chromium.org fork of flashrom which might
do what you need (--wp-enable command):
https://chromium.googlesource.com/chromiumos/third_party/flashrom/


On Tue, Aug 23, 2016 at 2:32 PM, Timothy Pearson <
tpearson@raptorengineering.com> wrote:

> RFC PATCH
>
> TPM-enabled systems require an immutable (or at least very difficult to
> modify) CRTM, which can be provided via a locked bootblock sector.
>
> Add initial support for locking the upper 4K sector on Winbond Flash
> ROMs.  Unlocking the sector after it has been locked requires hardware
> access to the write protect pin, satisfying the above requirements for many
> use cases.
>
> To use, pass the --lock flag to Flashrom.  This flag can be used in
> isolation, or as part of a normal read / write / verify cycle.
>
> Index: spi25_statusreg.c
> ===================================================================
> --- spi25_statusreg.c   (revision 1955)
> +++ spi25_statusreg.c   (working copy)
> @@ -123,6 +123,78 @@
>         return readarr[0];
>  }
>
> +static uint8_t spi_read_status_register_2(struct flashctx *flash)
> +{
> +       static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR2
> };
> +       /* FIXME: No workarounds for driver/hardware bugs in generic code.
> */
> +       unsigned char readarr[2]; /* JEDEC_RDSR2_INSIZE=1 but wbsio needs
> 2 */
> +       int ret;
> +
> +       /* Read Status Register */
> +       ret = spi_send_command(flash, sizeof(cmd), sizeof(readarr), cmd,
> readarr);
> +       if (ret)
> +               msg_cerr("RDSR2 failed!\n");
> +
> +       return readarr[0];
> +}
> +
> +/* Winbond sector protection enable that tries to set the status register
> bits given. */
> +int spi_enable_sectorprotect_winbond(struct flashctx *flash, const
> uint8_t status_flags)
> +{
> +       uint8_t status, status2;
> +       int result;
> +       uint8_t lock_mask = 0x80;
> +       uint8_t lock2_mask = 0x1;
> +
> +       status = spi_read_status_register(flash);
> +       status2 = spi_read_status_register_2(flash);
> +       if ((status & lock_mask) != 0) {
> +               msg_cdbg("\n\tNeed to disable the register lock first...
> ");
> +               if ((status2 & lock2_mask) != 0) {
> +                       msg_cerr("Hardware protection is active, enabling
> write protection is impossible.\n");
> +                       return 1;
> +               }
> +               /* All bits except the register lock bit (often called
> SPRL, SRWD, WPEN) are readonly. */
> +               result = spi_write_status_register(flash, status &
> ~lock_mask);
> +               if (result) {
> +                       msg_cerr("spi_write_status_register failed.\n");
> +                       return result;
> +               }
> +               status = spi_read_status_register(flash);
> +               if ((status & lock_mask) != 0) {
> +                       msg_cerr("Unsetting lock bit(s) failed.\n");
> +                       return 1;
> +               }
> +               msg_cdbg("done.\n");
> +       }
> +
> +       /* Set requested protect bits */
> +       result = spi_write_status_register(flash, status | (status_flags
> << 2) );
> +       if (result) {
> +               msg_cerr("spi_write_status_register failed.\n");
> +               return result;
> +       }
> +       status = spi_read_status_register(flash);
> +       if ((status & (status_flags << 2)) != (status_flags << 2)) {
> +               msg_cerr("Setting lock bit(s) failed.\n");
> +               return 1;
> +       }
> +
> +       /* Set lock bit */
> +       result = spi_write_status_register(flash, status | lock_mask);
> +       if (result) {
> +               msg_cerr("spi_write_status_register failed.\n");
> +               return result;
> +       }
> +       status = spi_read_status_register(flash);
> +       if ((status & lock_mask) == 0) {
> +               msg_cerr("Setting lock bit(s) failed.\n");
> +               return 1;
> +       }
> +
> +       return 0;
> +}
> +
>  /* A generic block protection disable.
>   * Tests if a protection is enabled with the block protection mask
> (bp_mask) and returns success otherwise.
>   * Tests if the register bits are locked with the lock_mask (lock_mask).
> Index: flashrom.c
> ===================================================================
> --- flashrom.c  (revision 1955)
> +++ flashrom.c  (working copy)
> @@ -1977,7 +1977,7 @@
>   * Besides that, the function itself is a textbook example of abysmal
> code flow.
>   */
>  int doit(struct flashctx *flash, int force, const char *filename, int
> read_it,
> -        int write_it, int erase_it, int verify_it)
> +        int write_it, int erase_it, int verify_it, int lock_it)
>  {
>         uint8_t *oldcontents;
>         uint8_t *newcontents;
> @@ -2129,6 +2129,13 @@
>                         msg_cinfo("VERIFIED.\n");
>         }
>
> +       if (lock_it && flash->chip->lock) {
> +               msg_cinfo("Locking upper 4K (bootblock)... ");
> +               ret = flash->chip->lock(flash, 0x11);
> +               if (!ret)
> +                       msg_cinfo("LOCKED.\n");
> +       }
> +
>  out:
>         free(oldcontents);
>         free(newcontents);
> Index: cli_classic.c
> ===================================================================
> --- cli_classic.c       (revision 1955)
> +++ cli_classic.c       (working copy)
> @@ -49,6 +49,7 @@
>                " -r | --read <file>                 read flash and save to
> <file>\n"
>                " -w | --write <file>                write <file> to
> flash\n"
>                " -v | --verify <file>               verify flash against
> <file>\n"
> +              " -u | --lock                        lock upper Flash
> region (typically the 4K bootblock)\n"
>                " -E | --erase                       erase flash memory\n"
>                " -V | --verbose                     more verbose output\n"
>                " -c | --chip <chipname>             probe only for
> specified flash chip\n"
> @@ -101,18 +102,19 @@
>  #if CONFIG_PRINT_WIKI == 1
>         int list_supported_wiki = 0;
>  #endif
> -       int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
> +       int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0,
> lock_it = 0;
>         int dont_verify_it = 0, list_supported = 0, operation_specified =
> 0;
>         enum programmer prog = PROGRAMMER_INVALID;
>         int ret = 0;
>
> -       static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:Lzho:";
> +       static const char optstring[] = "r:Rw:v:nuVEfc:l:i:p:Lzho:";
>         static const struct option long_options[] = {
>                 {"read",                1, NULL, 'r'},
>                 {"write",               1, NULL, 'w'},
>                 {"erase",               0, NULL, 'E'},
>                 {"verify",              1, NULL, 'v'},
>                 {"noverify",            0, NULL, 'n'},
> +               {"lock",                0, NULL, 'u'},
>                 {"chip",                1, NULL, 'c'},
>                 {"verbose",             0, NULL, 'V'},
>                 {"force",               0, NULL, 'f'},
> @@ -187,6 +189,9 @@
>                         }
>                         dont_verify_it = 1;
>                         break;
> +               case 'u':
> +                       lock_it = 1;
> +                       break;
>                 case 'c':
>                         chip_to_probe = strdup(optarg);
>                         break;
> @@ -522,7 +527,7 @@
>                 goto out_shutdown;
>         }
>
> -       if (!(read_it | write_it | verify_it | erase_it)) {
> +       if (!(read_it | write_it | verify_it | erase_it | lock_it)) {
>                 msg_ginfo("No operations were specified.\n");
>                 goto out_shutdown;
>         }
> @@ -542,7 +547,7 @@
>          * Give the chip time to settle.
>          */
>         programmer_delay(100000);
> -       ret |= doit(fill_flash, force, filename, read_it, write_it,
> erase_it, verify_it);
> +       ret |= doit(fill_flash, force, filename, read_it, write_it,
> erase_it, verify_it, lock_it);
>
>         unmap_flash(fill_flash);
>  out_shutdown:
> Index: flashchips.c
> ===================================================================
> --- flashchips.c        (revision 1955)
> +++ flashchips.c        (working copy)
> @@ -14581,6 +14581,7 @@
>                 },
>                 .printlock      = spi_prettyprint_status_register_plain,
> /* TODO: improve */
>                 .unlock         = spi_disable_blockprotect,
> +               .lock           = spi_enable_sectorprotect_winbond,
>                 .write          = spi_chip_write_256,
>                 .read           = spi_chip_read,
>                 .voltage        = {2700, 3600},
> Index: chipdrivers.h
> ===================================================================
> --- chipdrivers.h       (revision 1955)
> +++ chipdrivers.h       (working copy)
> @@ -73,6 +73,7 @@
>  int spi_prettyprint_status_register_bp4_srwd(struct flashctx *flash);
>  int spi_prettyprint_status_register_bp2_bpl(struct flashctx *flash);
>  int spi_prettyprint_status_register_bp2_tb_bpl(struct flashctx *flash);
> +int spi_enable_sectorprotect_winbond(struct flashctx *flash, const
> uint8_t status_flags);
>  int spi_disable_blockprotect(struct flashctx *flash);
>  int spi_disable_blockprotect_bp1_srwd(struct flashctx *flash);
>  int spi_disable_blockprotect_bp2_srwd(struct flashctx *flash);
> Index: flash.h
> ===================================================================
> --- flash.h     (revision 1955)
> +++ flash.h     (working copy)
> @@ -200,6 +200,7 @@
>         } block_erasers[NUM_ERASEFUNCTIONS];
>
>         int (*printlock) (struct flashctx *flash);
> +       int (*lock) (struct flashctx *flash, const uint8_t status_flags);
>         int (*unlock) (struct flashctx *flash);
>         int (*write) (struct flashctx *flash, const uint8_t *buf, unsigned
> int start, unsigned int len);
>         int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int
> start, unsigned int len);
> @@ -281,7 +282,7 @@
>  void print_banner(void);
>  void list_programmers_linebreak(int startcol, int cols, int paren);
>  int selfcheck(void);
> -int doit(struct flashctx *flash, int force, const char *filename, int
> read_it, int write_it, int erase_it, int verify_it);
> +int doit(struct flashctx *flash, int force, const char *filename, int
> read_it, int write_it, int erase_it, int verify_it, int lock_it);
>  int read_buf_from_file(unsigned char *buf, unsigned long size, const char
> *filename);
>  int write_buf_to_file(const unsigned char *buf, unsigned long size, const
> char *filename);
>
> Index: spi.h
> ===================================================================
> --- spi.h       (revision 1955)
> +++ spi.h       (working copy)
> @@ -121,6 +121,11 @@
>  #define JEDEC_RDSR_OUTSIZE     0x01
>  #define JEDEC_RDSR_INSIZE      0x01
>
> +/* Read Status Register 2 */
> +#define JEDEC_RDSR2            0x35
> +#define JEDEC_RDSR2_OUTSIZE    0x01
> +#define JEDEC_RDSR2_INSIZE     0x01
> +
>  /* Status Register Bits */
>  #define SPI_SR_WIP     (0x01 << 0)
>  #define SPI_SR_WEL     (0x01 << 1)
>
> _______________________________________________
> flashrom mailing list
> flashrom@flashrom.org
> https://www.flashrom.org/mailman/listinfo/flashrom
>

Patch hide | download patch | download mbox

Index: spi25_statusreg.c
===================================================================
--- spi25_statusreg.c	(revision 1955)
+++ spi25_statusreg.c	(working copy)
@@ -123,6 +123,78 @@ 
 	return readarr[0];
 }
 
+static uint8_t spi_read_status_register_2(struct flashctx *flash)
+{
+	static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR2 };
+	/* FIXME: No workarounds for driver/hardware bugs in generic code. */
+	unsigned char readarr[2]; /* JEDEC_RDSR2_INSIZE=1 but wbsio needs 2 */
+	int ret;
+
+	/* Read Status Register */
+	ret = spi_send_command(flash, sizeof(cmd), sizeof(readarr), cmd, readarr);
+	if (ret)
+		msg_cerr("RDSR2 failed!\n");
+
+	return readarr[0];
+}
+
+/* Winbond sector protection enable that tries to set the status register bits given. */
+int spi_enable_sectorprotect_winbond(struct flashctx *flash, const uint8_t status_flags)
+{
+	uint8_t status, status2;
+	int result;
+	uint8_t lock_mask = 0x80;
+	uint8_t lock2_mask = 0x1;
+
+	status = spi_read_status_register(flash);
+	status2 = spi_read_status_register_2(flash);
+	if ((status & lock_mask) != 0) {
+		msg_cdbg("\n\tNeed to disable the register lock first... ");
+		if ((status2 & lock2_mask) != 0) {
+			msg_cerr("Hardware protection is active, enabling write protection is impossible.\n");
+			return 1;
+		}
+		/* All bits except the register lock bit (often called SPRL, SRWD, WPEN) are readonly. */
+		result = spi_write_status_register(flash, status & ~lock_mask);
+		if (result) {
+			msg_cerr("spi_write_status_register failed.\n");
+			return result;
+		}
+		status = spi_read_status_register(flash);
+		if ((status & lock_mask) != 0) {
+			msg_cerr("Unsetting lock bit(s) failed.\n");
+			return 1;
+		}
+		msg_cdbg("done.\n");
+	}
+
+	/* Set requested protect bits */
+	result = spi_write_status_register(flash, status | (status_flags << 2) );
+	if (result) {
+		msg_cerr("spi_write_status_register failed.\n");
+		return result;
+	}
+	status = spi_read_status_register(flash);
+	if ((status & (status_flags << 2)) != (status_flags << 2)) {
+		msg_cerr("Setting lock bit(s) failed.\n");
+		return 1;
+	}
+
+	/* Set lock bit */
+	result = spi_write_status_register(flash, status | lock_mask);
+	if (result) {
+		msg_cerr("spi_write_status_register failed.\n");
+		return result;
+	}
+	status = spi_read_status_register(flash);
+	if ((status & lock_mask) == 0) {
+		msg_cerr("Setting lock bit(s) failed.\n");
+		return 1;
+	}
+
+	return 0;
+}
+
 /* A generic block protection disable.
  * Tests if a protection is enabled with the block protection mask (bp_mask) and returns success otherwise.
  * Tests if the register bits are locked with the lock_mask (lock_mask).
Index: flashrom.c
===================================================================
--- flashrom.c	(revision 1955)
+++ flashrom.c	(working copy)
@@ -1977,7 +1977,7 @@ 
  * Besides that, the function itself is a textbook example of abysmal code flow.
  */
 int doit(struct flashctx *flash, int force, const char *filename, int read_it,
-	 int write_it, int erase_it, int verify_it)
+	 int write_it, int erase_it, int verify_it, int lock_it)
 {
 	uint8_t *oldcontents;
 	uint8_t *newcontents;
@@ -2129,6 +2129,13 @@ 
 			msg_cinfo("VERIFIED.\n");
 	}
 
+	if (lock_it && flash->chip->lock) {
+		msg_cinfo("Locking upper 4K (bootblock)... ");
+		ret = flash->chip->lock(flash, 0x11);
+		if (!ret)
+			msg_cinfo("LOCKED.\n");
+	}
+
 out:
 	free(oldcontents);
 	free(newcontents);
Index: cli_classic.c
===================================================================
--- cli_classic.c	(revision 1955)
+++ cli_classic.c	(working copy)
@@ -49,6 +49,7 @@ 
 	       " -r | --read <file>                 read flash and save to <file>\n"
 	       " -w | --write <file>                write <file> to flash\n"
 	       " -v | --verify <file>               verify flash against <file>\n"
+	       " -u | --lock                        lock upper Flash region (typically the 4K bootblock)\n"
 	       " -E | --erase                       erase flash memory\n"
 	       " -V | --verbose                     more verbose output\n"
 	       " -c | --chip <chipname>             probe only for specified flash chip\n"
@@ -101,18 +102,19 @@ 
 #if CONFIG_PRINT_WIKI == 1
 	int list_supported_wiki = 0;
 #endif
-	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
+	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0, lock_it = 0;
 	int dont_verify_it = 0, list_supported = 0, operation_specified = 0;
 	enum programmer prog = PROGRAMMER_INVALID;
 	int ret = 0;
 
-	static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:Lzho:";
+	static const char optstring[] = "r:Rw:v:nuVEfc:l:i:p:Lzho:";
 	static const struct option long_options[] = {
 		{"read",		1, NULL, 'r'},
 		{"write",		1, NULL, 'w'},
 		{"erase",		0, NULL, 'E'},
 		{"verify",		1, NULL, 'v'},
 		{"noverify",		0, NULL, 'n'},
+		{"lock",		0, NULL, 'u'},
 		{"chip",		1, NULL, 'c'},
 		{"verbose",		0, NULL, 'V'},
 		{"force",		0, NULL, 'f'},
@@ -187,6 +189,9 @@ 
 			}
 			dont_verify_it = 1;
 			break;
+		case 'u':
+			lock_it = 1;
+			break;
 		case 'c':
 			chip_to_probe = strdup(optarg);
 			break;
@@ -522,7 +527,7 @@ 
 		goto out_shutdown;
 	}
 
-	if (!(read_it | write_it | verify_it | erase_it)) {
+	if (!(read_it | write_it | verify_it | erase_it | lock_it)) {
 		msg_ginfo("No operations were specified.\n");
 		goto out_shutdown;
 	}
@@ -542,7 +547,7 @@ 
 	 * Give the chip time to settle.
 	 */
 	programmer_delay(100000);
-	ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it);
+	ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it, lock_it);
 
 	unmap_flash(fill_flash);
 out_shutdown:
Index: flashchips.c
===================================================================
--- flashchips.c	(revision 1955)
+++ flashchips.c	(working copy)
@@ -14581,6 +14581,7 @@ 
 		},
 		.printlock	= spi_prettyprint_status_register_plain, /* TODO: improve */
 		.unlock		= spi_disable_blockprotect,
+		.lock		= spi_enable_sectorprotect_winbond,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
Index: chipdrivers.h
===================================================================
--- chipdrivers.h	(revision 1955)
+++ chipdrivers.h	(working copy)
@@ -73,6 +73,7 @@ 
 int spi_prettyprint_status_register_bp4_srwd(struct flashctx *flash);
 int spi_prettyprint_status_register_bp2_bpl(struct flashctx *flash);
 int spi_prettyprint_status_register_bp2_tb_bpl(struct flashctx *flash);
+int spi_enable_sectorprotect_winbond(struct flashctx *flash, const uint8_t status_flags);
 int spi_disable_blockprotect(struct flashctx *flash);
 int spi_disable_blockprotect_bp1_srwd(struct flashctx *flash);
 int spi_disable_blockprotect_bp2_srwd(struct flashctx *flash);
Index: flash.h
===================================================================
--- flash.h	(revision 1955)
+++ flash.h	(working copy)
@@ -200,6 +200,7 @@ 
 	} block_erasers[NUM_ERASEFUNCTIONS];
 
 	int (*printlock) (struct flashctx *flash);
+	int (*lock) (struct flashctx *flash, const uint8_t status_flags);
 	int (*unlock) (struct flashctx *flash);
 	int (*write) (struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len);
 	int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
@@ -281,7 +282,7 @@ 
 void print_banner(void);
 void list_programmers_linebreak(int startcol, int cols, int paren);
 int selfcheck(void);
-int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it);
+int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it, int lock_it);
 int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename);
 int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename);
 
Index: spi.h
===================================================================
--- spi.h	(revision 1955)
+++ spi.h	(working copy)
@@ -121,6 +121,11 @@ 
 #define JEDEC_RDSR_OUTSIZE	0x01
 #define JEDEC_RDSR_INSIZE	0x01
 
+/* Read Status Register 2 */
+#define JEDEC_RDSR2		0x35
+#define JEDEC_RDSR2_OUTSIZE	0x01
+#define JEDEC_RDSR2_INSIZE	0x01
+
 /* Status Register Bits */
 #define SPI_SR_WIP	(0x01 << 0)
 #define SPI_SR_WEL	(0x01 << 1)