diff mbox series

[v4,09/18] core/flash.c: add SECBOOT read and write support

Message ID 20200511213152.24952-10-erichte@linux.ibm.com
State Changes Requested
Headers show
Series Add initial secure variable storage and backend drivers | expand

Checks

Context Check Description
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (0f1937ef40fca0c3212a9dff1010b832a24fb063)

Commit Message

Eric Richter May 11, 2020, 9:31 p.m. UTC
From: Claudio Carvalho <cclaudio@linux.ibm.com>

In secure boot enabled systems, the petitboot linux kernel verifies the
OS kernel against x509 certificates that are wrapped in secure variables
controlled by OPAL. These secure variables are stored in the PNOR SECBOOT
partition, as well as the updates submitted for them using userspace
tools.

This patch adds read and write support to the PNOR SECBOOT partition in
a similar fashion to that of NVRAM, so that OPAL can handle the secure
variables.

Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
---
 core/flash.c       | 130 +++++++++++++++++++++++++++++++++++++++++++++
 include/platform.h |   4 ++
 2 files changed, 134 insertions(+)

Comments

Oliver O'Halloran May 14, 2020, 2:16 a.m. UTC | #1
On Tue, May 12, 2020 at 7:35 AM Eric Richter <erichte@linux.ibm.com> wrote:
>
> From: Claudio Carvalho <cclaudio@linux.ibm.com>
>
> In secure boot enabled systems, the petitboot linux kernel verifies the
> OS kernel against x509 certificates that are wrapped in secure variables
> controlled by OPAL. These secure variables are stored in the PNOR SECBOOT
> partition, as well as the updates submitted for them using userspace
> tools.
>
> This patch adds read and write support to the PNOR SECBOOT partition in
> a similar fashion to that of NVRAM, so that OPAL can handle the secure
> variables.

Is there any reason to have this as a pile of platform callbacks
rather than calling the flash_read, etc from the storage driver? The
platform is already choosing the storage driver so handling it there
seems cleaner IMO. And I'd really like it if we could stop solving
every problem by adding callbacks to the platform structure...

*snip*

> +static int flash_secboot_probe(struct flash *flash, struct ffs_handle *ffs)
> +{
> +       uint32_t start, size, part;
> +       bool ecc;
> +       int rc;
> +
> +       prlog(PR_DEBUG, "FLASH: probing for SECBOOT\n");
> +
> +       rc = ffs_lookup_part(ffs, "SECBOOT", &part);
> +       if (rc) {
> +               prlog(PR_WARNING, "FLASH: no SECBOOT partition found\n");
> +               return OPAL_HARDWARE;
> +       }
> +
> +       rc = ffs_part_info(ffs, part, NULL,
> +                          &start, &size, NULL, &ecc);
> +       if (rc) {
> +               /**
> +                * @fwts-label SECBOOTNoPartition
> +                * @fwts-advice OPAL could not find an SECBOOT partition
> +                *     on the system flash. Check that the system flash
> +                *     has a valid partition table, and that the firmware
> +                *     build process has added a SECBOOT partition.
> +                */
> +               prlog(PR_ERR, "FLASH: Can't parse ffs info for SECBOOT\n");
> +               return OPAL_HARDWARE;
> +       }
> +
> +       secboot_flash = flash;
> +       secboot_offset = start;
> +       secboot_size = ecc ? ecc_buffer_size_minus_ecc(size) : size;

IMO the sec var partition should always be ECC protected. Even if the
contents of flash is fine we have seen errors introduced while
transferring data over the LPC bus and if a single bit is wrong we'll
the wrong hash.
diff mbox series

Patch

diff --git a/core/flash.c b/core/flash.c
index de748641..33d7f648 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -59,6 +59,10 @@  static struct lock flash_lock;
 static struct flash *nvram_flash;
 static u32 nvram_offset, nvram_size;
 
+/* secboot-on-flash support */
+static struct flash *secboot_flash;
+static u32 secboot_offset, secboot_size;
+
 bool flash_reserve(void)
 {
 	bool rc = false;
@@ -93,6 +97,91 @@  bool flash_unregister(void)
 	return true;
 }
 
+static int flash_secboot_info(uint32_t *total_size)
+{
+	int rc;
+
+	lock(&flash_lock);
+	if (!secboot_flash) {
+		rc = OPAL_HARDWARE;
+	} else if (secboot_flash->busy) {
+		rc = OPAL_BUSY;
+	} else {
+		*total_size = secboot_size;
+		rc = OPAL_SUCCESS;
+	}
+	unlock(&flash_lock);
+
+	return rc;
+}
+
+static int flash_secboot_read(void *dst, uint32_t src, uint32_t len)
+{
+	int rc;
+
+	if (!try_lock(&flash_lock))
+		return OPAL_BUSY;
+
+	if (!secboot_flash) {
+		rc = OPAL_HARDWARE;
+		goto out;
+	}
+
+	if (secboot_flash->busy) {
+		rc = OPAL_BUSY;
+		goto out;
+	}
+
+	if ((src + len) > secboot_size) {
+		prerror("FLASH_SECBOOT: read out of bound (0x%x,0x%x)\n",
+			src, len);
+		rc = OPAL_PARAMETER;
+		goto out;
+	}
+
+	secboot_flash->busy = true;
+	unlock(&flash_lock);
+
+	rc = blocklevel_read(secboot_flash->bl, secboot_offset + src, dst, len);
+
+	lock(&flash_lock);
+	secboot_flash->busy = false;
+out:
+	unlock(&flash_lock);
+	return rc;
+}
+
+static int flash_secboot_write(uint32_t dst, void *src, uint32_t len)
+{
+	int rc;
+
+	if (!try_lock(&flash_lock))
+		return OPAL_BUSY;
+
+	if (secboot_flash->busy) {
+		rc = OPAL_BUSY;
+		goto out;
+	}
+
+	if ((dst + len) > secboot_size) {
+		prerror("FLASH_SECBOOT: write out of bound (0x%x,0x%x)\n",
+			dst, len);
+		rc = OPAL_PARAMETER;
+		goto out;
+	}
+
+	secboot_flash->busy = true;
+	unlock(&flash_lock);
+
+	rc = blocklevel_write(secboot_flash->bl, secboot_offset + dst, src, len);
+
+	lock(&flash_lock);
+	secboot_flash->busy = false;
+out:
+	unlock(&flash_lock);
+	return rc;
+}
+
 static int flash_nvram_info(uint32_t *total_size)
 {
 	int rc;
@@ -182,6 +271,46 @@  out:
 	return rc;
 }
 
+
+static int flash_secboot_probe(struct flash *flash, struct ffs_handle *ffs)
+{
+	uint32_t start, size, part;
+	bool ecc;
+	int rc;
+
+	prlog(PR_DEBUG, "FLASH: probing for SECBOOT\n");
+
+	rc = ffs_lookup_part(ffs, "SECBOOT", &part);
+	if (rc) {
+		prlog(PR_WARNING, "FLASH: no SECBOOT partition found\n");
+		return OPAL_HARDWARE;
+	}
+
+	rc = ffs_part_info(ffs, part, NULL,
+			   &start, &size, NULL, &ecc);
+	if (rc) {
+		/**
+		 * @fwts-label SECBOOTNoPartition
+		 * @fwts-advice OPAL could not find an SECBOOT partition
+		 *     on the system flash. Check that the system flash
+		 *     has a valid partition table, and that the firmware
+		 *     build process has added a SECBOOT partition.
+		 */
+		prlog(PR_ERR, "FLASH: Can't parse ffs info for SECBOOT\n");
+		return OPAL_HARDWARE;
+	}
+
+	secboot_flash = flash;
+	secboot_offset = start;
+	secboot_size = ecc ? ecc_buffer_size_minus_ecc(size) : size;
+
+	platform.secboot_info = flash_secboot_info;
+	platform.secboot_read = flash_secboot_read;
+	platform.secboot_write = flash_secboot_write;
+
+	return 0;
+}
+
 static int flash_nvram_probe(struct flash *flash, struct ffs_handle *ffs)
 {
 	uint32_t start, size, part;
@@ -332,6 +461,7 @@  static void setup_system_flash(struct flash *flash, struct dt_node *node,
 	prlog(PR_INFO, "registered system flash device %s\n", name);
 
 	flash_nvram_probe(flash, ffs);
+	flash_secboot_probe(flash, ffs);
 }
 
 static int num_flashes(void)
diff --git a/include/platform.h b/include/platform.h
index 6aa263ae..db1a6e97 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -221,6 +221,10 @@  struct platform {
 
 	int (*secvar_init)(void);
 
+	int (*secboot_info)(uint32_t *total_size);
+	int (*secboot_read)(void *dst, uint32_t src, uint32_t len);
+	int (*secboot_write)(uint32_t dst, void *src, uint32_t len);
+
 	/*
 	 * OCC timeout. This return how long we should wait for the OCC
 	 * before timing out. This lets us use a high value on larger FSP