[Skiboot] [SKIBOOT] [PATCH] core/flash: Only expect ELF header for BOOTKERNEL partition flash resource

[Suraj Jitindar Singh]

When loading a flash resource which isn't signed (secure and trusted
boot) and which doesn't have a subpartition, we assume it's the
BOOTKERNEL since previously this was the only such resource. Thus we
also assumed it had an ELF header which we parsed to get the size of the
partition rather than trusting the actual_size field in the FFS header.
A previous commit (9727fe3 DT: Add ibm,firmware-versions node) added the
version resource which isn't signed and also doesn't have a subpartition,
thus we expect it to have an ELF header. It doesn't so we print the
error message "FLASH: Invalid ELF header part VERSION".

It is a fluke that this works currently since we load the secure boot
header unconditionally and this happen to be the same size as the
version partition. We also don't update the return code on error so
happen to return OPAL_SUCCESS.

To make this explicitly correct; only check for an ELF header if we are
loading the BOOTKERNEL resource, otherwise use the partition size from
the FFS header. Also set the return code on error so we don't
erroneously return OPAL_SUCCESS. Add a check that the resource will fit
in the supplied buffer to prevent buffer overrun.

Fixes: 9727fe3 (DT: Add ibm,firmware-versions node)

Reported-by: Pridhiviraj Paidipeddi <ppaidipe at in.ibm.com>
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh at gmail.com>
---
 core/flash.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/core/flash.c b/core/flash.c
index feb7119..6255a65 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -692,6 +692,7 @@ static int flash_load_resource(enum resource_id id, uint32_t subid,
 
 		if (content_size > bufsz) {
 			prerror("FLASH: content size > buffer size\n");
+			rc = OPAL_PARAMETER;
 			goto out_free_ffs;
 		}
 
@@ -725,15 +726,28 @@ static int flash_load_resource(enum resource_id id, uint32_t subid,
 		 * Back to the old way of doing things, no STB header.
 		 */
 		if (subid == RESOURCE_SUBID_NONE) {
-			/*
-			 * Because actualSize is a lie, we compute the size
-			 * of the BOOTKERNEL based on what the ELF headers
-			 * say. Otherwise we end up reading more than we should
-			 */
-			content_size = sizeof_elf_from_hdr(buf);
-			if (!content_size) {
-				prerror("FLASH: Invalid ELF header part %s\n",
-					name);
+			if (id == RESOURCE_ID_KERNEL) {
+				/*
+				 * Because actualSize is a lie, we compute the
+				 * size of the BOOTKERNEL based on what the ELF
+				 * headers say. Otherwise we end up reading more
+				 * than we should
+				 */
+				content_size = sizeof_elf_from_hdr(buf);
+				if (!content_size) {
+					prerror("FLASH: Invalid ELF header part"
+						" %s\n", name);
+					rc = OPAL_RESOURCE;
+					goto out_free_ffs;
+				}
+			} else {
+				content_size = ffs_part_size;
+			}
+			if (content_size > bufsz) {
+				prerror("FLASH: %s content size %d > "
+					" buffer size %lu\n", name,
+					content_size, bufsz);
+				rc = OPAL_PARAMETER;
 				goto out_free_ffs;
 			}
 			prlog(PR_DEBUG, "FLASH: computed %s size %u\n",
-- 
2.9.4