[RFC PATCH v2 2/8] FWU: Free metadata copy if gpt_get_mdata() failed

Masami Hiramatsu masami.hiramatsu at linaro.org
Thu Feb 17 16:11:36 CET 2022


It is better if a function which returns an error then release
all allocated memory resources. This simplifies the mind model
and less chance to forgot to free memory and double free.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
---
 drivers/fwu-mdata/fwu-mdata-uclass.c  |   24 ++++++----------
 drivers/fwu-mdata/fwu_mdata_gpt_blk.c |   49 ++++++++++++++++++---------------
 2 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c b/drivers/fwu-mdata/fwu-mdata-uclass.c
index b98eda3789..c1cd77243f 100644
--- a/drivers/fwu-mdata/fwu-mdata-uclass.c
+++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
@@ -79,12 +79,12 @@ int fwu_verify_mdata(struct fwu_mdata *mdata, bool pri_part)
 int fwu_get_active_index(u32 *active_idx)
 {
 	int ret;
-	struct fwu_mdata *mdata = NULL;
+	struct fwu_mdata *mdata;
 
 	ret = fwu_get_mdata(&mdata);
 	if (ret < 0) {
 		log_err("Unable to get valid FWU metadata\n");
-		goto out;
+		return ret;
 	}
 
 	/*
@@ -96,8 +96,6 @@ int fwu_get_active_index(u32 *active_idx)
 		log_err("Active index value read is incorrect\n");
 		ret = -EINVAL;
 	}
-
-out:
 	free(mdata);
 
 	return ret;
@@ -115,17 +113,17 @@ out:
 int fwu_update_active_index(u32 active_idx)
 {
 	int ret;
-	struct fwu_mdata *mdata = NULL;
+	struct fwu_mdata *mdata;
 
 	if (active_idx > CONFIG_FWU_NUM_BANKS - 1) {
 		log_err("Active index value to be updated is incorrect\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	ret = fwu_get_mdata(&mdata);
 	if (ret < 0) {
 		log_err("Unable to get valid FWU metadata\n");
-		goto out;
+		return ret;
 	}
 
 	/*
@@ -144,8 +142,6 @@ int fwu_update_active_index(u32 active_idx)
 		log_err("Failed to update FWU metadata partitions\n");
 		ret = -EIO;
 	}
-
-out:
 	free(mdata);
 
 	return ret;
@@ -225,12 +221,12 @@ int fwu_revert_boot_index(void)
 {
 	int ret;
 	u32 cur_active_index;
-	struct fwu_mdata *mdata = NULL;
+	struct fwu_mdata *mdata;
 
 	ret = fwu_get_mdata(&mdata);
 	if (ret < 0) {
 		log_err("Unable to get valid FWU metadata\n");
-		goto out;
+		return ret;
 	}
 
 	/*
@@ -250,8 +246,6 @@ int fwu_revert_boot_index(void)
 		log_err("Failed to update FWU metadata partitions\n");
 		ret = -EIO;
 	}
-
-out:
 	free(mdata);
 
 	return ret;
@@ -277,14 +271,14 @@ static int fwu_set_clear_image_accept(efi_guid_t *img_type_id,
 {
 	int ret, i;
 	u32 nimages;
-	struct fwu_mdata *mdata = NULL;
+	struct fwu_mdata *mdata;
 	struct fwu_image_entry *img_entry;
 	struct fwu_image_bank_info *img_bank_info;
 
 	ret = fwu_get_mdata(&mdata);
 	if (ret < 0) {
 		log_err("Unable to get valid FWU metadata\n");
-		goto out;
+		return ret;
 	}
 
 	nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
diff --git a/drivers/fwu-mdata/fwu_mdata_gpt_blk.c b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
index 9170c3f6af..a32195db2e 100644
--- a/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
+++ b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
@@ -177,18 +177,9 @@ static int fwu_gpt_update_mdata(struct udevice * dev, struct fwu_mdata *mdata)
 	return 0;
 }
 
-static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata **mdata)
+static int gpt_get_mdata_part(struct blk_desc *desc, struct fwu_mdata **mdata, u16 part)
 {
 	int ret;
-	u16 primary_mpart = 0, secondary_mpart = 0;
-
-	ret = gpt_get_mdata_partitions(desc, &primary_mpart,
-				       &secondary_mpart);
-
-	if (ret < 0) {
-		log_err("Error getting the FWU metadata partitions\n");
-		return -ENODEV;
-	}
 
 	*mdata = malloc(sizeof(struct fwu_mdata));
 	if (!*mdata) {
@@ -196,28 +187,42 @@ static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata **mdata)
 		return -ENOMEM;
 	}
 
-	ret = gpt_read_mdata(desc, *mdata, primary_mpart);
+	ret = gpt_read_mdata(desc, *mdata, part);
 	if (ret < 0) {
 		log_err("Failed to read the FWU metadata from the device\n");
-		return -EIO;
+		ret = -EIO;
+	} else {
+		ret = fwu_verify_mdata(*mdata, 1);
+		if (!ret)
+			return 0;
 	}
+	free(*mdata);
+
+	return ret;
+}
 
-	ret = fwu_verify_mdata(*mdata, 1);
+static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata **mdata)
+{
+	int ret;
+	u16 primary_mpart = 0, secondary_mpart = 0;
+
+	ret = gpt_get_mdata_partitions(desc, &primary_mpart,
+				       &secondary_mpart);
+
+	if (ret < 0) {
+		log_err("Error getting the FWU metadata partitions\n");
+		return -ENODEV;
+	}
+
+	ret = gpt_get_mdata_part(desc, mdata, primary_mpart);
 	if (!ret)
 		return 0;
 
 	/*
-	 * Verification of the primary FWU metadata copy failed.
+	 * Reading of the primary FWU metadata copy failed.
 	 * Try to read the replica.
 	 */
-	memset(*mdata, 0, sizeof(struct fwu_mdata));
-	ret = gpt_read_mdata(desc, *mdata, secondary_mpart);
-	if (ret < 0) {
-		log_err("Failed to read the FWU metadata from the device\n");
-		return -EIO;
-	}
-
-	ret = fwu_verify_mdata(*mdata, 0);
+	ret = gpt_get_mdata_part(desc, mdata, secondary_mpart);
 	if (!ret)
 		return 0;
 



More information about the U-Boot mailing list