From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Date: Thu, 12 Sep 2024 13:41:41 +0000 (+0200)
Subject: env: mmc: rework mmc_env_partition_by_guid() to work with two separate partitions
X-Git-Url: http://git.dujemihanovic.xyz/login.html?a=commitdiff_plain;h=c1131aca9adfb2861dd34ba0e45f593874adcd81;p=u-boot.git

env: mmc: rework mmc_env_partition_by_guid() to work with two separate partitions

Having two separate partitions for use in a redundant environment
setup works just fine, if one only relies on setting CONFIG_ENV_OFFSET
and CONFIG_ENV_OFFSET_REDUND. However, if CONFIG_PARTITION_TYPE_GUID
is enabled, the current logic in mmc_env_partition_by_guid() means
that only the first partition will ever be considered, and prior to
the previous commit, lead to silent data corruption.

Extend the logic so that, when we are looking for the location for the
second copy of the environment, we keep track of whether we have
already found one matching partition. If a second match is found,
return that, but also modify *copy so that the logic in the caller
will use the last ENV_SIZE bytes of that second partition - in my
case, and I suppose that would be typical, both partitions have been
created with a size of exactly the desired ENV_SIZE.

When only a single matching partition exists, the behaviour is
unchanged: We return that single partition, and *copy is left as-is,
so the logic in the caller will either use the last (copy==0) or
second-to-last (copy==1) ENV_SIZE bytes.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---

diff --git a/env/mmc.c b/env/mmc.c
index 5d09140655..e2f8e7ece2 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -69,21 +69,49 @@ static int mmc_env_partition_by_name(struct blk_desc *desc, const char *str,
 	}
 }
 
-static int mmc_env_partition_by_guid(struct blk_desc *desc, struct disk_partition *info)
+/*
+ * Look for one or two partitions with the U-Boot environment GUID.
+ *
+ * If *copy is 0, return the first such partition.
+ *
+ * If *copy is 1 on entry and two partitions are found, return the
+ * second partition and set *copy = 0.
+ *
+ * If *copy is 1 on entry and only one partition is found, return that
+ * partition, leaving *copy unmodified.
+ */
+static int mmc_env_partition_by_guid(struct blk_desc *desc, struct disk_partition *info,
+				     int *copy)
 {
 	const efi_guid_t env_guid = PARTITION_U_BOOT_ENVIRONMENT;
 	efi_guid_t type_guid;
-	int i, ret;
+	int i, ret, found = 0;
+	struct disk_partition dp;
 
 	for (i = 1;; i++) {
-		ret = part_get_info(desc, i, info);
+		ret = part_get_info(desc, i, &dp);
 		if (ret < 0)
-			return ret;
-
-		uuid_str_to_bin(disk_partition_type_guid(info), type_guid.b, UUID_STR_FORMAT_GUID);
-		if (!memcmp(&env_guid, &type_guid, sizeof(efi_guid_t)))
-			return 0;
+			break;
+
+		uuid_str_to_bin(disk_partition_type_guid(&dp), type_guid.b, UUID_STR_FORMAT_GUID);
+		if (!memcmp(&env_guid, &type_guid, sizeof(efi_guid_t))) {
+			memcpy(info, &dp, sizeof(dp));
+			/* If *copy is 0, we are only looking for the first partition. */
+			if (*copy == 0)
+				return 0;
+			/* This was the second such partition. */
+			if (found) {
+				*copy = 0;
+				return 0;
+			}
+			found = 1;
+		}
 	}
+
+	/* The loop ended after finding at most one matching partition. */
+	if (found)
+		ret = 0;
+	return ret;
 }
 
 
@@ -103,7 +131,7 @@ static inline int mmc_offset_try_partition(const char *str, int copy, s64 *val)
 	if (str) {
 		ret = mmc_env_partition_by_name(desc, str, &info);
 	} else if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID) && !str) {
-		ret = mmc_env_partition_by_guid(desc, &info);
+		ret = mmc_env_partition_by_guid(desc, &info, &copy);
 	}
 	if (ret < 0)
 		return ret;