[Skiboot] [PATCH 05/16] flash: Fix bugs for sub-partitions in flash_load_resource()

Michael Neuling mikey at neuling.org
Fri Feb 20 11:37:48 AEDT 2015


In the recent change:
  commit 73b262e768529f152ec64b4418b0c31691bc15c3
  Author: Jeremy Kerr <jk at ozlabs.org>
  core/flash: port pnor_load_resource to flash code

Some code was moved around, but it was modified introducing a number of bugs.

Firstly the size check was moved to before the sub-partition determination.
This is incorrect as the size check is against the sub-partition not the full
partition.

Secondly, it introduced a return in the error path for sub-partitions.  This
should be a goto to ensure the correct cleanup code is run.

Thirdly, subid is now set in the name lookup loop where it shouldn't be.

Fourthly, a check on the subid was removed unnecessarily.

Signed-off-by: Michael Neuling <mikey at neuling.org>
---
 core/flash.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/core/flash.c b/core/flash.c
index 95ce1f0..a256252 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -454,7 +454,6 @@ bool flash_load_resource(enum resource_id id, uint32_t subid,
 	for (i = 0, name = NULL; i < ARRAY_SIZE(part_name_map); i++) {
 		if (part_name_map[i].id == id) {
 			name = part_name_map[i].name;
-			subid = part_name_map[i].subid;
 			break;
 		}
 	}
@@ -462,6 +461,15 @@ bool flash_load_resource(enum resource_id id, uint32_t subid,
 		prerror("FLASH: Couldn't find partition for id %d\n", id);
 		goto out_unlock;
 	}
+	/*
+	 * If partition doesn't have a subindex but the caller specifies one,
+	 * we fail.  eg. kernel partition doesn't have a subindex
+	 */
+	if ((part_name_map[i].subid == RESOURCE_SUBID_NONE) &&
+	    (subid != RESOURCE_SUBID_NONE)) {
+		prerror("PLAT: Partition %s doesn't have subindex\n", name);
+		return false;
+	}
 
 	rc = ffs_open_flash(flash->chip, 0, flash->size, &ffs);
 	if (rc) {
@@ -481,18 +489,18 @@ bool flash_load_resource(enum resource_id id, uint32_t subid,
 		goto out_free_ffs;
 	}
 
-	if (part_size > *len) {
-		prerror("FLASH: %s image too large (%d > %zd)\n", name,
-			part_size, *len);
-		goto out_free_ffs;
-	}
-
 	/* Find the sub partition if required */
 	if (subid != RESOURCE_SUBID_NONE) {
 		rc = flash_find_subpartition(flash->chip, subid, &part_start,
 					    &part_size);
 		if (rc)
-			return false;
+			goto out_free_ffs;
+	}
+
+	if (part_size > *len) {
+		prerror("FLASH: %s image too large (%d > %zd)\n", name,
+			part_size, *len);
+		goto out_free_ffs;
 	}
 
 	rc = flash_read(flash->chip, part_start, buf, part_size);
-- 
2.1.0



More information about the Skiboot mailing list