[U-Boot] [PATCH 0/6] gpt: command: Add support for "gpt verify" command

[U-Boot] [PATCH 0/6] gpt: command: Add support for "gpt verify" command

[Lukasz Majewski]

Up till now "gpt" command was only able to write (i.e. restore) GPT
partition on specified medium with information provided at "$partitions"
env variable.

This patch series adds complementary feature - namely "gpt verify", which
allows checking (at e.g. boot time) if previously created GPT layout is
still correct.

This patch series clearly applies on top of u-boot/master:
SHA1: cad04990715f7eaecd45196e84cf10e9e3248dae

It has been tested on Beaglebone Black development board (Sitara AM335x
CPU).

Lukasz Majewski (6):
  gpt: command: Remove duplicated check for empty partition description
  gpt: command: cosmetic: Replace printf with puts
  gpt: doc: README: Update README entry for gpt verify extension
  gpt: doc: Update gpt command's help description
  gpt: part: Definition and declaration of GPT verification functions
  gpt: command: Extend gpt command to support GPT table verification

 common/cmd_gpt.c | 106 ++++++++++++++++++++++++++++++++++++++-----------------
 disk/part_efi.c  | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 doc/README.gpt   |  26 +++++++++++++-
 include/part.h   |  35 ++++++++++++++++++
 4 files changed, 235 insertions(+), 33 deletions(-)

-- 
2.1.4

[U-Boot] [PATCH 1/6] gpt: command: Remove duplicated check for empty partition description

[Lukasz Majewski]

Exactly the same check is performed in set_gpt_info() function executed
just after this check.

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
 common/cmd_gpt.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
index c56fe15..4da2de7 100644
--- a/common/cmd_gpt.c
+++ b/common/cmd_gpt.c
@@ -276,9 +276,6 @@ static int gpt_default(block_dev_desc_t *blk_dev_desc, const char *str_part)
 	u8 part_count = 0;
 	disk_partition_t *partitions = NULL;
 
-	if (!str_part)
-		return -1;
-
 	/* fill partitions */
 	ret = set_gpt_info(blk_dev_desc, str_part,
 			&str_disk_guid, &partitions, &part_count);
-- 
2.1.4

[U-Boot] [PATCH 1/6] gpt: command: Remove duplicated check for empty partition description

[Tom Rini]

On Fri, Nov 13, 2015 at 07:42:07AM +0100, Lukasz Majewski wrote:

> Exactly the same check is performed in set_gpt_info() function executed
> just after this check.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151118/f5c6307f/attachment.sig>

[U-Boot] [PATCH 2/6] gpt: command: cosmetic: Replace printf with puts

[Lukasz Majewski]

This code is not processing any data to serial console output and hence
can be replaced with puts.

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
 common/cmd_gpt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
index 4da2de7..4c71125 100644
--- a/common/cmd_gpt.c
+++ b/common/cmd_gpt.c
@@ -281,11 +281,11 @@ static int gpt_default(block_dev_desc_t *blk_dev_desc, const char *str_part)
 			&str_disk_guid, &partitions, &part_count);
 	if (ret) {
 		if (ret == -1)
-			printf("No partition list provided\n");
+			puts("No partition list provided\n");
 		if (ret == -2)
-			printf("Missing disk guid\n");
+			puts("Missing disk guid\n");
 		if ((ret == -3) || (ret == -4))
-			printf("Partition list incomplete\n");
+			puts("Partition list incomplete\n");
 		return -1;
 	}
 
-- 
2.1.4

[U-Boot] [PATCH 2/6] gpt: command: cosmetic: Replace printf with puts

[Tom Rini]

On Fri, Nov 13, 2015 at 07:42:08AM +0100, Lukasz Majewski wrote:

> This code is not processing any data to serial console output and hence
> can be replaced with puts.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

No, printf is fine, always.  We went over this before and it's not a win
to change things.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151118/26c9e2b1/attachment.sig>

[U-Boot] [PATCH 2/6] gpt: command: cosmetic: Replace printf with puts

[Lukasz Majewski]

Hi Tom,

> On Fri, Nov 13, 2015 at 07:42:08AM +0100, Lukasz Majewski wrote:
> 
> > This code is not processing any data to serial console output and
> > hence can be replaced with puts.
> > 
> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> 
> No, printf is fine, always.  We went over this before and it's not a
> win to change things.

Ok, I see. Then I will drop this patch.

> 

Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151119/7dd28a9f/attachment.sig>

[U-Boot] [PATCH 3/6] gpt: doc: README: Update README entry for gpt verify extension

[Lukasz Majewski]

./doc/README.gpt entry has been updated to explain usage of "gpt verify"
command.

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
 doc/README.gpt | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/doc/README.gpt b/doc/README.gpt
index 59fdeeb..9fe97b0 100644
--- a/doc/README.gpt
+++ b/doc/README.gpt
@@ -167,9 +167,33 @@ To restore GUID partition table one needs to:
 
 2. Define 'CONFIG_EFI_PARTITION' and 'CONFIG_CMD_GPT'
 
-2. From u-boot prompt type:
+3. From u-boot prompt type:
    gpt write mmc 0 $partitions
 
+Checking (validating) GPT partitions in U-Boot:
+===============================================
+
+Procedure is the same as above. The only change is at point 3.
+
+At u-boot prompt one needs to write:
+   gpt verify mmc 0 [$partitions]
+
+where [$partitions] is an optional parameter.
+
+When it is not provided, only basic checks based on CRC32 calculation for GPT
+header and PTEs are performed.
+When provided, additionally partition data - name, size and starting
+offset (last two in LBA) - are compared with data defined in '$partitions'
+environment variable.
+
+After running this command, return code is set to 0 if no errors found in
+on non-volatile medium stored GPT.
+
+Following line can be used to assess if GPT verification has succeed:
+
+U-BOOT> gpt verify mmc 0 $partitions
+U-BOOT> if test $? = 0; then echo "GPT OK"; else echo "GPT ERR"; fi
+
 Useful info:
 ============
 
-- 
2.1.4

[U-Boot] [PATCH 3/6] gpt: doc: README: Update README entry for gpt verify extension

[Tom Rini]

On Fri, Nov 13, 2015 at 07:42:09AM +0100, Lukasz Majewski wrote:

> ./doc/README.gpt entry has been updated to explain usage of "gpt verify"
> command.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151118/bf91c619/attachment.sig>

[U-Boot] [PATCH 4/6] gpt: doc: Update gpt command's help description

[Lukasz Majewski]

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
 common/cmd_gpt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
index 4c71125..be54eae 100644
--- a/common/cmd_gpt.c
+++ b/common/cmd_gpt.c
@@ -350,7 +350,10 @@ static int do_gpt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
 	"GUID Partition Table",
 	"<command> <interface> <dev> <partitions_list>\n"
-	" - GUID partition table restoration\n"
-	" Restore GPT information on a device connected\n"
+	" - GUID partition table restoration and validity check\n"
+	" Restore or verify GPT information on a device connected\n"
 	" to interface\n"
+	" Example usage:\n"
+	" gpt write mmc 0 $partitions\n"
+	" gpt verify mmc 0 $partitions\n"
 );
-- 
2.1.4

[U-Boot] [PATCH 4/6] gpt: doc: Update gpt command's help description

[Tom Rini]

On Fri, Nov 13, 2015 at 07:42:10AM +0100, Lukasz Majewski wrote:

> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151118/9642df67/attachment.sig>

[U-Boot] [U-Boot, 4/6] gpt: doc: Update gpt command's help description

[Tom Rini]

On Fri, Nov 13, 2015 at 07:42:10AM +0100, Lukasz Majewski wrote:

> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151123/dbbe9563/attachment.sig>

[U-Boot] [PATCH 5/6] gpt: part: Definition and declaration of GPT verification functions

[Lukasz Majewski]

This commit provides definition and declaration of GPT verification
functions - namely gpt_verify_headers() and gpt_verify_partitions().
The former is used to only check CRC32 of GPT's header and PTEs.
The latter examines each partition entry and compare attributes such as:
name, start offset and size with ones provided at '$partitions' env
variable.

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
 disk/part_efi.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/part.h  |  35 ++++++++++++++++++++
 2 files changed, 136 insertions(+)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 15627f2..59edf38 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -549,6 +549,107 @@ err:
 	return ret;
 }
 
+static void gpt_convert_efi_name_to_char(char *s, efi_char16_t *es, int n)
+{
+	char *ess = (char *)es;
+	int i, j;
+
+	memset(s, '\0', n);
+
+	for (i = 0, j = 0; j < n; i += 2, j++) {
+		s[j] = ess[i];
+		if (!ess[i])
+			return;
+	}
+}
+
+int gpt_verify_headers(block_dev_desc_t *dev_desc, gpt_header *gpt_head,
+		       gpt_entry **gpt_pte)
+{
+	/*
+	 * This function validates AND
+	 * fills in the GPT header and PTE
+	 */
+	if (is_gpt_valid(dev_desc,
+			 GPT_PRIMARY_PARTITION_TABLE_LBA,
+			 gpt_head, gpt_pte) != 1) {
+		printf("%s: *** ERROR: Invalid GPT ***\n",
+		       __func__);
+		return -1;
+	}
+	if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
+			 gpt_head, gpt_pte) != 1) {
+		printf("%s: *** ERROR: Invalid Backup GPT ***\n",
+		       __func__);
+		return -1;
+	}
+
+	return 0;
+}
+
+int gpt_verify_partitions(block_dev_desc_t *dev_desc,
+			  disk_partition_t *partitions, int parts,
+			  gpt_header *gpt_head, gpt_entry **gpt_pte)
+{
+	char efi_str[PARTNAME_SZ + 1];
+	u64 gpt_part_size;
+	gpt_entry *gpt_e;
+	int ret, i;
+
+	ret = gpt_verify_headers(dev_desc, gpt_head, gpt_pte);
+	if (ret)
+		return ret;
+
+	gpt_e = *gpt_pte;
+
+	for (i = 0; i < parts; i++) {
+		if (i == gpt_head->num_partition_entries) {
+			error("More partitions than allowed!\n");
+			return -1;
+		}
+
+		/* Check if GPT and ENV partition names match */
+		gpt_convert_efi_name_to_char(efi_str, gpt_e[i].partition_name,
+					     PARTNAME_SZ + 1);
+
+		debug("%s: part: %2d name - GPT: %16s, ENV: %16s ",
+		      __func__, i, efi_str, partitions[i].name);
+
+		if (strncmp(efi_str, (char *)partitions[i].name,
+			    sizeof(partitions->name))) {
+			error("Partition name: %s does not match %s!\n",
+			      efi_str, (char *)partitions[i].name);
+			return -1;
+		}
+
+		/* Check if GPT and ENV start LBAs match */
+		debug("start LBA - GPT: %8llu, ENV: %8llu ",
+		      le64_to_cpu(gpt_e[i].starting_lba),
+		      (u64) partitions[i].start);
+
+		if (le64_to_cpu(gpt_e[i].starting_lba) != partitions[i].start) {
+			error("Partition %s start: %llu does not match %llu!\n",
+			      efi_str, le64_to_cpu(gpt_e[i].starting_lba),
+			      (u64) partitions[i].start);
+			return -1;
+		}
+
+		/* Check if GPT and ENV sizes match */
+		gpt_part_size = le64_to_cpu(gpt_e[i].ending_lba) -
+			le64_to_cpu(gpt_e[i].starting_lba) + 1;
+		debug("size(LBA) - GPT: %8llu, ENV: %8llu\n",
+		      gpt_part_size, (u64) partitions[i].size);
+
+		if (le64_to_cpu(gpt_part_size) != partitions[i].size) {
+			error("Partition %s size: %llu does not match %llu!\n",
+			      efi_str, gpt_part_size, (u64) partitions[i].size);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
 int is_valid_gpt_buf(block_dev_desc_t *dev_desc, void *buf)
 {
 	gpt_header *gpt_h;
diff --git a/include/part.h b/include/part.h
index 8ea9b30..4152052 100644
--- a/include/part.h
+++ b/include/part.h
@@ -264,6 +264,41 @@ int is_valid_gpt_buf(block_dev_desc_t *dev_desc, void *buf);
  * @return - '0' on success, otherwise error
  */
 int write_mbr_and_gpt_partitions(block_dev_desc_t *dev_desc, void *buf);
+
+/**
+ * gpt_verify_headers() - Function to read and CRC32 check of the GPT's header
+ *                        and partition table entries (PTE)
+ *
+ * As a side effect if sets gpt_head and gpt_pte so they point to GPT data.
+ *
+ * @param dev_desc - block device descriptor
+ * @param gpt_head - pointer to GPT header data read from medium
+ * @param gpt_pte - pointer to GPT partition table enties read from medium
+ *
+ * @return - '0' on success, otherwise error
+ */
+int gpt_verify_headers(block_dev_desc_t *dev_desc, gpt_header *gpt_head,
+		       gpt_entry **gpt_pte);
+
+/**
+ * gpt_verify_partitions() - Function to check if partitions' name, start and
+ *                           size correspond to '$partitions' env variable
+ *
+ * This function checks if on medium stored GPT data is in sync with information
+ * provided in '$partitions' environment variable. Specificially, name, start
+ * and size of the partition is checked.
+ *
+ * @param dev_desc - block device descriptor
+ * @param partitions - partition data read from '$partitions' env variable
+ * @param parts - number of partitions read from '$partitions' env variable
+ * @param gpt_head - pointer to GPT header data read from medium
+ * @param gpt_pte - pointer to GPT partition table enties read from medium
+ *
+ * @return - '0' on success, otherwise error
+ */
+int gpt_verify_partitions(block_dev_desc_t *dev_desc,
+			  disk_partition_t *partitions, int parts,
+			  gpt_header *gpt_head, gpt_entry **gpt_pte);
 #endif
 
 #endif /* _PART_H */
-- 
2.1.4

[U-Boot] [PATCH 5/6] gpt: part: Definition and declaration of GPT verification functions

[Tom Rini]

On Fri, Nov 13, 2015 at 07:42:11AM +0100, Lukasz Majewski wrote:

> This commit provides definition and declaration of GPT verification
> functions - namely gpt_verify_headers() and gpt_verify_partitions().
> The former is used to only check CRC32 of GPT's header and PTEs.
> The latter examines each partition entry and compare attributes such as:
> name, start offset and size with ones provided at '$partitions' env
> variable.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151118/497b674f/attachment.sig>

[U-Boot] [PATCH 6/6] gpt: command: Extend gpt command to support GPT table verification

[Lukasz Majewski]

This commit adds support for "gpt verify" command, which verifies
correctness of on-board stored GPT partition table.
As the optional parameter one can provide '$partitons' environment variable
to check if partition data (size, offset, name) is correct.

This command should be regarded as complementary one to "gpt restore".

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
 common/cmd_gpt.c | 90 +++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 66 insertions(+), 24 deletions(-)

diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
index be54eae..ddad4b2 100644
--- a/common/cmd_gpt.c
+++ b/common/cmd_gpt.c
@@ -1,6 +1,9 @@
 /*
  * cmd_gpt.c -- GPT (GUID Partition Table) handling command
  *
+ * Copyright (C) 2015
+ * Lukasz Majewski <l.majewski@majess.pl>
+ *
  * Copyright (C) 2012 Samsung Electronics
  * author: Lukasz Majewski <l.majewski@samsung.com>
  * author: Piotr Wilczek <p.wilczek@samsung.com>
@@ -15,6 +18,7 @@
 #include <exports.h>
 #include <linux/ctype.h>
 #include <div64.h>
+#include <memalign.h>
 
 #ifndef CONFIG_PARTITION_UUIDS
 #error CONFIG_PARTITION_UUIDS must be enabled for CONFIG_CMD_GPT to be enabled
@@ -297,6 +301,43 @@ static int gpt_default(block_dev_desc_t *blk_dev_desc, const char *str_part)
 	return ret;
 }
 
+static int gpt_verify(block_dev_desc_t *blk_dev_desc, const char *str_part)
+{
+	ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1,
+				     blk_dev_desc->blksz);
+	disk_partition_t *partitions = NULL;
+	gpt_entry *gpt_pte = NULL;
+	char *str_disk_guid;
+	u8 part_count = 0;
+	int ret = 0;
+
+	/* fill partitions */
+	ret = set_gpt_info(blk_dev_desc, str_part,
+			&str_disk_guid, &partitions, &part_count);
+	if (ret) {
+		if (ret == -1) {
+			puts("No partition list provided - only basic check\n");
+			ret = gpt_verify_headers(blk_dev_desc, gpt_head,
+						 &gpt_pte);
+			goto out;
+		}
+		if (ret == -2)
+			printf("Missing disk guid\n");
+		if ((ret == -3) || (ret == -4))
+			printf("Partition list incomplete\n");
+		return -1;
+	}
+
+	/* Check partition layout with provided pattern */
+	ret = gpt_verify_partitions(blk_dev_desc, partitions, part_count,
+				    gpt_head, &gpt_pte);
+	free(str_disk_guid);
+	free(partitions);
+ out:
+	free(gpt_pte);
+	return ret;
+}
+
 /**
  * do_gpt(): Perform GPT operations
  *
@@ -312,39 +353,40 @@ static int do_gpt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	int ret = CMD_RET_SUCCESS;
 	int dev = 0;
 	char *ep;
-	block_dev_desc_t *blk_dev_desc;
+	block_dev_desc_t *blk_dev_desc = NULL;
 
-	if (argc < 5)
+	if (argc < 4 || argc > 5)
 		return CMD_RET_USAGE;
 
-	/* command: 'write' */
-	if ((strcmp(argv[1], "write") == 0) && (argc == 5)) {
-		dev = (int)simple_strtoul(argv[3], &ep, 10);
-		if (!ep || ep[0] != '\0') {
-			printf("'%s' is not a number\n", argv[3]);
-			return CMD_RET_USAGE;
-		}
-		blk_dev_desc = get_dev(argv[2], dev);
-		if (!blk_dev_desc) {
-			printf("%s: %s dev %d NOT available\n",
-			       __func__, argv[2], dev);
-			return CMD_RET_FAILURE;
-		}
+	dev = (int)simple_strtoul(argv[3], &ep, 10);
+	if (!ep || ep[0] != '\0') {
+		printf("'%s' is not a number\n", argv[3]);
+		return CMD_RET_USAGE;
+	}
+	blk_dev_desc = get_dev(argv[2], dev);
+	if (!blk_dev_desc) {
+		printf("%s: %s dev %d NOT available\n",
+		       __func__, argv[2], dev);
+		return CMD_RET_FAILURE;
+	}
 
+	if ((strcmp(argv[1], "write") == 0) && (argc == 5)) {
 		puts("Writing GPT: ");
-
 		ret = gpt_default(blk_dev_desc, argv[4]);
-		if (!ret) {
-			puts("success!\n");
-			return CMD_RET_SUCCESS;
-		} else {
-			puts("error!\n");
-			return CMD_RET_FAILURE;
-		}
+	} else if ((strcmp(argv[1], "verify") == 0)) {
+		ret = gpt_verify(blk_dev_desc, argv[4]);
+		puts("Verify GPT: ");
 	} else {
 		return CMD_RET_USAGE;
 	}
-	return ret;
+
+	if (ret) {
+		puts("error!\n");
+		return CMD_RET_FAILURE;
+	}
+
+	puts("success!\n");
+	return CMD_RET_SUCCESS;
 }
 
 U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
-- 
2.1.4

[U-Boot] [PATCH 6/6] gpt: command: Extend gpt command to support GPT table verification

[Tom Rini]

On Fri, Nov 13, 2015 at 07:42:12AM +0100, Lukasz Majewski wrote:

> This commit adds support for "gpt verify" command, which verifies
> correctness of on-board stored GPT partition table.
> As the optional parameter one can provide '$partitons' environment variable
> to check if partition data (size, offset, name) is correct.
> 
> This command should be regarded as complementary one to "gpt restore".
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

Note you're using puts and printf both here, please just switch to
printf.  But:
Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151118/cf2c5270/attachment.sig>

[U-Boot] [PATCH 6/6] gpt: command: Extend gpt command to support GPT table verification

[Lukasz Majewski]

On Wed, 18 Nov 2015 18:40:48 -0500
Tom Rini <trini@konsulko.com> wrote:

> On Fri, Nov 13, 2015 at 07:42:12AM +0100, Lukasz Majewski wrote:
> 
> > This commit adds support for "gpt verify" command, which verifies
> > correctness of on-board stored GPT partition table.
> > As the optional parameter one can provide '$partitons' environment
> > variable to check if partition data (size, offset, name) is correct.
> > 
> > This command should be regarded as complementary one to "gpt
> > restore".
> > 
> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> 
> Note you're using puts and printf both here, please just switch to
> printf.  But:
> Reviewed-by: Tom Rini <trini@konsulko.com>
> 

I will fix this in v2. I also need to fix some issues with
"$partitions" parsing code.

Anyway, thanks for review.

Best regards,
?ukasz Majewski

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151119/0488d704/attachment.sig>

[U-Boot] [PATCH v2 0/5] gpt: command: Add support for "gpt verify" command

[Lukasz Majewski]

Up till now "gpt" command was only able to write (i.e. restore) GPT
partition on specified medium with information provided at "$partitions"
env variable.

This patch series adds complementary feature - namely "gpt verify", which
allows checking (at e.g. boot time) if previously created GPT layout is
still correct.

This patch series clearly applies on top of u-boot/master:
SHA1: 736d1746fb7b8f7cd70657a4a72db2b6bd8de40e

It has been tested on Beaglebone Black development board (Sitara AM335x
CPU).


Lukasz Majewski (5):
  gpt: command: Remove duplicated check for empty partition description
  gpt: doc: README: Update README entry for gpt verify extension
  gpt: doc: Update gpt command's help description
  gpt: part: Definition and declaration of GPT verification functions
  gpt: command: Extend gpt command to support GPT table verification

 common/cmd_gpt.c | 102 ++++++++++++++++++++++++++++++++++++---------------
 disk/part_efi.c  | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 doc/README.gpt   |  27 +++++++++++++-
 include/part.h   |  35 ++++++++++++++++++
 4 files changed, 243 insertions(+), 31 deletions(-)

-- 
2.1.4

[U-Boot] [PATCH v2 1/5] gpt: command: Remove duplicated check for empty partition description

[Lukasz Majewski]

Exactly the same check is performed in set_gpt_info() function executed
just after this check.

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Reviewed-by: Tom Rini <trini@konsulko.com>

---
Changes for v2:
- None
---
 common/cmd_gpt.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
index e3c0297..6bea2c7 100644
--- a/common/cmd_gpt.c
+++ b/common/cmd_gpt.c
@@ -293,9 +293,6 @@ static int gpt_default(block_dev_desc_t *blk_dev_desc, const char *str_part)
 	u8 part_count = 0;
 	disk_partition_t *partitions = NULL;
 
-	if (!str_part)
-		return -1;
-
 	/* fill partitions */
 	ret = set_gpt_info(blk_dev_desc, str_part,
 			&str_disk_guid, &partitions, &part_count);
-- 
2.1.4

[U-Boot] [U-Boot, v2, 1/5] gpt: command: Remove duplicated check for empty partition description

[Tom Rini]

On Fri, Nov 20, 2015 at 08:06:13AM +0100, Lukasz Majewski wrote:

> Exactly the same check is performed in set_gpt_info() function executed
> just after this check.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151123/01e3df67/attachment.sig>

[U-Boot] [PATCH v2 2/5] gpt: doc: README: Update README entry for gpt verify extension

[Lukasz Majewski]

./doc/README.gpt entry has been updated to explain usage of "gpt verify"
command.

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Reviewed-by: Tom Rini <trini@konsulko.com>

---
Changes for v2:
- Adjust README.gpt changes to the newest mainline
---
 doc/README.gpt | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/doc/README.gpt b/doc/README.gpt
index 35902ce..db439f9 100644
--- a/doc/README.gpt
+++ b/doc/README.gpt
@@ -168,9 +168,34 @@ To restore GUID partition table one needs to:
 
 2. Define 'CONFIG_EFI_PARTITION' and 'CONFIG_CMD_GPT'
 
-2. From u-boot prompt type:
+3. From u-boot prompt type:
    gpt write mmc 0 $partitions
 
+Checking (validating) GPT partitions in U-Boot:
+===============================================
+
+Procedure is the same as above. The only change is at point 3.
+
+At u-boot prompt one needs to write:
+   gpt verify mmc 0 [$partitions]
+
+where [$partitions] is an optional parameter.
+
+When it is not provided, only basic checks based on CRC32 calculation for GPT
+header and PTEs are performed.
+When provided, additionally partition data - name, size and starting
+offset (last two in LBA) - are compared with data defined in '$partitions'
+environment variable.
+
+After running this command, return code is set to 0 if no errors found in
+on non-volatile medium stored GPT.
+
+Following line can be used to assess if GPT verification has succeed:
+
+U-BOOT> gpt verify mmc 0 $partitions
+U-BOOT> if test $? = 0; then echo "GPT OK"; else echo "GPT ERR"; fi
+
+
 Partition type GUID:
 ====================
 
-- 
2.1.4

[U-Boot] [U-Boot, v2, 2/5] gpt: doc: README: Update README entry for gpt verify extension

[Tom Rini]

On Fri, Nov 20, 2015 at 08:06:14AM +0100, Lukasz Majewski wrote:

> ./doc/README.gpt entry has been updated to explain usage of "gpt verify"
> command.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151123/7ae7303e/attachment.sig>

[U-Boot] [PATCH v2 4/5] gpt: part: Definition and declaration of GPT verification functions

[Lukasz Majewski]

This commit provides definition and declaration of GPT verification
functions - namely gpt_verify_headers() and gpt_verify_partitions().
The former is used to only check CRC32 of GPT's header and PTEs.
The latter examines each partition entry and compare attributes such as:
name, start offset and size with ones provided at '$partitions' env
variable.

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Reviewed-by: Tom Rini <trini@konsulko.com>

---
Changes for v2:
- Provide support for situation where "start" attribute at '$partitions'
  env variable is not provided.
---
 disk/part_efi.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/part.h  |  35 ++++++++++++++++++
 2 files changed, 145 insertions(+)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index ea9c615..40f0b36 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -578,6 +578,116 @@ err:
 	return ret;
 }
 
+static void gpt_convert_efi_name_to_char(char *s, efi_char16_t *es, int n)
+{
+	char *ess = (char *)es;
+	int i, j;
+
+	memset(s, '\0', n);
+
+	for (i = 0, j = 0; j < n; i += 2, j++) {
+		s[j] = ess[i];
+		if (!ess[i])
+			return;
+	}
+}
+
+int gpt_verify_headers(block_dev_desc_t *dev_desc, gpt_header *gpt_head,
+		       gpt_entry **gpt_pte)
+{
+	/*
+	 * This function validates AND
+	 * fills in the GPT header and PTE
+	 */
+	if (is_gpt_valid(dev_desc,
+			 GPT_PRIMARY_PARTITION_TABLE_LBA,
+			 gpt_head, gpt_pte) != 1) {
+		printf("%s: *** ERROR: Invalid GPT ***\n",
+		       __func__);
+		return -1;
+	}
+	if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
+			 gpt_head, gpt_pte) != 1) {
+		printf("%s: *** ERROR: Invalid Backup GPT ***\n",
+		       __func__);
+		return -1;
+	}
+
+	return 0;
+}
+
+int gpt_verify_partitions(block_dev_desc_t *dev_desc,
+			  disk_partition_t *partitions, int parts,
+			  gpt_header *gpt_head, gpt_entry **gpt_pte)
+{
+	char efi_str[PARTNAME_SZ + 1];
+	u64 gpt_part_size;
+	gpt_entry *gpt_e;
+	int ret, i;
+
+	ret = gpt_verify_headers(dev_desc, gpt_head, gpt_pte);
+	if (ret)
+		return ret;
+
+	gpt_e = *gpt_pte;
+
+	for (i = 0; i < parts; i++) {
+		if (i == gpt_head->num_partition_entries) {
+			error("More partitions than allowed!\n");
+			return -1;
+		}
+
+		/* Check if GPT and ENV partition names match */
+		gpt_convert_efi_name_to_char(efi_str, gpt_e[i].partition_name,
+					     PARTNAME_SZ + 1);
+
+		debug("%s: part: %2d name - GPT: %16s, ENV: %16s ",
+		      __func__, i, efi_str, partitions[i].name);
+
+		if (strncmp(efi_str, (char *)partitions[i].name,
+			    sizeof(partitions->name))) {
+			error("Partition name: %s does not match %s!\n",
+			      efi_str, (char *)partitions[i].name);
+			return -1;
+		}
+
+		/* Check if GPT and ENV sizes match */
+		gpt_part_size = le64_to_cpu(gpt_e[i].ending_lba) -
+			le64_to_cpu(gpt_e[i].starting_lba) + 1;
+		debug("size(LBA) - GPT: %8llu, ENV: %8llu ",
+		      gpt_part_size, (u64) partitions[i].size);
+
+		if (le64_to_cpu(gpt_part_size) != partitions[i].size) {
+			error("Partition %s size: %llu does not match %llu!\n",
+			      efi_str, gpt_part_size, (u64) partitions[i].size);
+			return -1;
+		}
+
+		/*
+		 * Start address is optional - check only if provided
+		 * in '$partition' variable
+		 */
+		if (!partitions[i].start) {
+			debug("\n");
+			continue;
+		}
+
+		/* Check if GPT and ENV start LBAs match */
+		debug("start LBA - GPT: %8llu, ENV: %8llu\n",
+		      le64_to_cpu(gpt_e[i].starting_lba),
+		      (u64) partitions[i].start);
+
+		if (le64_to_cpu(gpt_e[i].starting_lba) != partitions[i].start) {
+			error("Partition %s start: %llu does not match %llu!\n",
+			      efi_str, le64_to_cpu(gpt_e[i].starting_lba),
+			      (u64) partitions[i].start);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
 int is_valid_gpt_buf(block_dev_desc_t *dev_desc, void *buf)
 {
 	gpt_header *gpt_h;
diff --git a/include/part.h b/include/part.h
index 8b5ac12..720a867 100644
--- a/include/part.h
+++ b/include/part.h
@@ -267,6 +267,41 @@ int is_valid_gpt_buf(block_dev_desc_t *dev_desc, void *buf);
  * @return - '0' on success, otherwise error
  */
 int write_mbr_and_gpt_partitions(block_dev_desc_t *dev_desc, void *buf);
+
+/**
+ * gpt_verify_headers() - Function to read and CRC32 check of the GPT's header
+ *                        and partition table entries (PTE)
+ *
+ * As a side effect if sets gpt_head and gpt_pte so they point to GPT data.
+ *
+ * @param dev_desc - block device descriptor
+ * @param gpt_head - pointer to GPT header data read from medium
+ * @param gpt_pte - pointer to GPT partition table enties read from medium
+ *
+ * @return - '0' on success, otherwise error
+ */
+int gpt_verify_headers(block_dev_desc_t *dev_desc, gpt_header *gpt_head,
+		       gpt_entry **gpt_pte);
+
+/**
+ * gpt_verify_partitions() - Function to check if partitions' name, start and
+ *                           size correspond to '$partitions' env variable
+ *
+ * This function checks if on medium stored GPT data is in sync with information
+ * provided in '$partitions' environment variable. Specificially, name, start
+ * and size of the partition is checked.
+ *
+ * @param dev_desc - block device descriptor
+ * @param partitions - partition data read from '$partitions' env variable
+ * @param parts - number of partitions read from '$partitions' env variable
+ * @param gpt_head - pointer to GPT header data read from medium
+ * @param gpt_pte - pointer to GPT partition table enties read from medium
+ *
+ * @return - '0' on success, otherwise error
+ */
+int gpt_verify_partitions(block_dev_desc_t *dev_desc,
+			  disk_partition_t *partitions, int parts,
+			  gpt_header *gpt_head, gpt_entry **gpt_pte);
 #endif
 
 #endif /* _PART_H */
-- 
2.1.4

[U-Boot] [PATCH v2 4/5] gpt: part: Definition and declaration of GPT verification functions

[Przemyslaw Marczak]

Hi Lukasz,

On 11/20/2015 08:06 AM, Lukasz Majewski wrote:
> This commit provides definition and declaration of GPT verification
> functions - namely gpt_verify_headers() and gpt_verify_partitions().
> The former is used to only check CRC32 of GPT's header and PTEs.
> The latter examines each partition entry and compare attributes such as:
> name, start offset and size with ones provided at '$partitions' env
> variable.
>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> ---
> Changes for v2:
> - Provide support for situation where "start" attribute at '$partitions'
>    env variable is not provided.

I've got few comments to this code.

> ---
>   disk/part_efi.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   include/part.h  |  35 ++++++++++++++++++
>   2 files changed, 145 insertions(+)
>
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index ea9c615..40f0b36 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -578,6 +578,116 @@ err:
>   	return ret;
>   }
>

Probably print_efiname() could meet your requirements.

> +static void gpt_convert_efi_name_to_char(char *s, efi_char16_t *es, int n)
> +{
> +	char *ess = (char *)es;
> +	int i, j;
> +
> +	memset(s, '\0', n);
> +
> +	for (i = 0, j = 0; j < n; i += 2, j++) {
> +		s[j] = ess[i];
> +		if (!ess[i])
> +			return;
> +	}
> +}
> +
> +int gpt_verify_headers(block_dev_desc_t *dev_desc, gpt_header *gpt_head,
> +		       gpt_entry **gpt_pte)
> +{
> +	/*
> +	 * This function validates AND
> +	 * fills in the GPT header and PTE
> +	 */
> +	if (is_gpt_valid(dev_desc,
> +			 GPT_PRIMARY_PARTITION_TABLE_LBA,
> +			 gpt_head, gpt_pte) != 1) {
> +		printf("%s: *** ERROR: Invalid GPT ***\n",
> +		       __func__);
> +		return -1;
> +	}
> +	if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
> +			 gpt_head, gpt_pte) != 1) {
> +		printf("%s: *** ERROR: Invalid Backup GPT ***\n",
> +		       __func__);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +

I've got an error for Trats2:

--------------------------------------------------------------
Trats2 # print partitions
partitions=uuid_disk=${uuid_gpt_disk};name=csa-mmc,start=5MiB,size=8MiB,uuid=${u
uid_gpt_csa-mmc};name=boot,size=60MiB,uuid=${uuid_gpt_boot};name=qboot,size=100M
iB,uuid=${uuid_gpt_qboot};name=csc,size=150MiB,uuid=${uuid_gpt_csc};name=platfor
m,size=1536MiB,uuid=${uuid_gpt_platform};name=data,size=3000MiB,uuid=${uuid_gpt_
data};name=ums,size=-,uuid=${uuid_gpt_ums}
Trats2 # gpt write mmc 0 $partitions
Writing GPT: success!
Trats2 # gpt verify mmc 0 $partitions
ERROR: Partition ums size: 20826079 does not match 0!

at disk/part_efi.c:662/gpt_verify_partitions()
Verify GPT: error!
Trats2 #
--------------------------------------------------------------

The function set_gpt_info() in file common/cmd_gpt.c doesn't fill the 
'partitions' as your code expects. There are some assumptions, respected 
by gpt_fill_pte(), like empty start, which you fixed, and case for which 
'size' is empty (e.g. for the last partition) - which cause printing an 
error as above.

The function gpt_verify_partitions() should actually do the same what 
gpt_fill_pte() does, but it has no sense to duplicate the code.

> +int gpt_verify_partitions(block_dev_desc_t *dev_desc,
> +			  disk_partition_t *partitions, int parts,
> +			  gpt_header *gpt_head, gpt_entry **gpt_pte)
> +{
> +	char efi_str[PARTNAME_SZ + 1];
> +	u64 gpt_part_size;
> +	gpt_entry *gpt_e;
> +	int ret, i;
> +
> +	ret = gpt_verify_headers(dev_desc, gpt_head, gpt_pte);
> +	if (ret)
> +		return ret;
> +
> +	gpt_e = *gpt_pte;
> +

To make proper verification in this function it would be better if you 
remove this loop and call function gpt_fill_pte() to fill some temporary 
'gpt_e' array from the parsed env $partitions.

Then you can compare if the temporary created gpt array entries are 
equal to the device's gpt entries.

Then you don't need to take care about the assumptions to parsing 
$partitions - just compare two headers - if $partition will change, then 
you will see the difference in the gpt headers.

> +	for (i = 0; i < parts; i++) {
> +		if (i == gpt_head->num_partition_entries) {
> +			error("More partitions than allowed!\n");
> +			return -1;
> +		}
> +
> +		/* Check if GPT and ENV partition names match */
> +		gpt_convert_efi_name_to_char(efi_str, gpt_e[i].partition_name,
> +					     PARTNAME_SZ + 1);
> +
> +		debug("%s: part: %2d name - GPT: %16s, ENV: %16s ",
> +		      __func__, i, efi_str, partitions[i].name);
> +
> +		if (strncmp(efi_str, (char *)partitions[i].name,
> +			    sizeof(partitions->name))) {
> +			error("Partition name: %s does not match %s!\n",
> +			      efi_str, (char *)partitions[i].name);
> +			return -1;
> +		}
> +
> +		/* Check if GPT and ENV sizes match */
> +		gpt_part_size = le64_to_cpu(gpt_e[i].ending_lba) -
> +			le64_to_cpu(gpt_e[i].starting_lba) + 1;
> +		debug("size(LBA) - GPT: %8llu, ENV: %8llu ",
> +		      gpt_part_size, (u64) partitions[i].size);
> +
> +		if (le64_to_cpu(gpt_part_size) != partitions[i].size) {
> +			error("Partition %s size: %llu does not match %llu!\n",
> +			      efi_str, gpt_part_size, (u64) partitions[i].size);
> +			return -1;
> +		}
> +
> +		/*
> +		 * Start address is optional - check only if provided
> +		 * in '$partition' variable
> +		 */
> +		if (!partitions[i].start) {
> +			debug("\n");
> +			continue;
> +		}
> +
> +		/* Check if GPT and ENV start LBAs match */
> +		debug("start LBA - GPT: %8llu, ENV: %8llu\n",
> +		      le64_to_cpu(gpt_e[i].starting_lba),
> +		      (u64) partitions[i].start);
> +
> +		if (le64_to_cpu(gpt_e[i].starting_lba) != partitions[i].start) {
> +			error("Partition %s start: %llu does not match %llu!\n",
> +			      efi_str, le64_to_cpu(gpt_e[i].starting_lba),
> +			      (u64) partitions[i].start);
> +			return -1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   int is_valid_gpt_buf(block_dev_desc_t *dev_desc, void *buf)
>   {
>   	gpt_header *gpt_h;
> diff --git a/include/part.h b/include/part.h
> index 8b5ac12..720a867 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -267,6 +267,41 @@ int is_valid_gpt_buf(block_dev_desc_t *dev_desc, void *buf);
>    * @return - '0' on success, otherwise error
>    */
>   int write_mbr_and_gpt_partitions(block_dev_desc_t *dev_desc, void *buf);
> +
> +/**
> + * gpt_verify_headers() - Function to read and CRC32 check of the GPT's header
> + *                        and partition table entries (PTE)
> + *
> + * As a side effect if sets gpt_head and gpt_pte so they point to GPT data.
> + *
> + * @param dev_desc - block device descriptor
> + * @param gpt_head - pointer to GPT header data read from medium
> + * @param gpt_pte - pointer to GPT partition table enties read from medium
> + *
> + * @return - '0' on success, otherwise error
> + */
> +int gpt_verify_headers(block_dev_desc_t *dev_desc, gpt_header *gpt_head,
> +		       gpt_entry **gpt_pte);
> +
> +/**
> + * gpt_verify_partitions() - Function to check if partitions' name, start and
> + *                           size correspond to '$partitions' env variable
> + *
> + * This function checks if on medium stored GPT data is in sync with information
> + * provided in '$partitions' environment variable. Specificially, name, start
> + * and size of the partition is checked.
> + *
> + * @param dev_desc - block device descriptor
> + * @param partitions - partition data read from '$partitions' env variable
> + * @param parts - number of partitions read from '$partitions' env variable
> + * @param gpt_head - pointer to GPT header data read from medium
> + * @param gpt_pte - pointer to GPT partition table enties read from medium
> + *
> + * @return - '0' on success, otherwise error
> + */
> +int gpt_verify_partitions(block_dev_desc_t *dev_desc,
> +			  disk_partition_t *partitions, int parts,
> +			  gpt_header *gpt_head, gpt_entry **gpt_pte);
>   #endif
>
>   #endif /* _PART_H */
>

Reviewed-by: Przemyslaw Marczak <p.marczak@samsung.com>

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

[U-Boot] [U-Boot, v2, 4/5] gpt: part: Definition and declaration of GPT verification functions

[Tom Rini]

On Fri, Nov 20, 2015 at 08:06:16AM +0100, Lukasz Majewski wrote:

> This commit provides definition and declaration of GPT verification
> functions - namely gpt_verify_headers() and gpt_verify_partitions().
> The former is used to only check CRC32 of GPT's header and PTEs.
> The latter examines each partition entry and compare attributes such as:
> name, start offset and size with ones provided at '$partitions' env
> variable.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Przemyslaw Marczak <p.marczak@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151123/762879c3/attachment.sig>

[U-Boot] [U-Boot, v2, 4/5] gpt: part: Definition and declaration of GPT verification functions

[Przemyslaw Marczak]

Hello Tom,

On 11/23/2015 11:44 PM, Tom Rini wrote:
> On Fri, Nov 20, 2015 at 08:06:16AM +0100, Lukasz Majewski wrote:
>
>> This commit provides definition and declaration of GPT verification
>> functions - namely gpt_verify_headers() and gpt_verify_partitions().
>> The former is used to only check CRC32 of GPT's header and PTEs.
>> The latter examines each partition entry and compare attributes such as:
>> name, start offset and size with ones provided at '$partitions' env
>> variable.
>>
>> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
>> Reviewed-by: Przemyslaw Marczak <p.marczak@samsung.com>
>
> Applied to u-boot/master, thanks!
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

Why merged so quickly?

I tested this patchset on my device and posted about the issues. [1]

This should be reworked, since the verify assumptions are too simple and 
doesn't fully match the GPT header creation.
So this command will fail for some cases of write/verify sequence, 
depending on what the $partitions includes.

[1] https://www.mail-archive.com/u-boot at lists.denx.de/msg193216.html

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

[U-Boot] [U-Boot, v2, 4/5] gpt: part: Definition and declaration of GPT verification functions

[Tom Rini]

On Tue, Nov 24, 2015 at 10:56:41AM +0100, Przemyslaw Marczak wrote:
> Hello Tom,
> 
> On 11/23/2015 11:44 PM, Tom Rini wrote:
> >On Fri, Nov 20, 2015 at 08:06:16AM +0100, Lukasz Majewski wrote:
> >
> >>This commit provides definition and declaration of GPT verification
> >>functions - namely gpt_verify_headers() and gpt_verify_partitions().
> >>The former is used to only check CRC32 of GPT's header and PTEs.
> >>The latter examines each partition entry and compare attributes such as:
> >>name, start offset and size with ones provided at '$partitions' env
> >>variable.
> >>
> >>Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> >>Reviewed-by: Tom Rini <trini@konsulko.com>
> >>Reviewed-by: Przemyslaw Marczak <p.marczak@samsung.com>
> >
> >Applied to u-boot/master, thanks!
> >
> >
> >
> >_______________________________________________
> >U-Boot mailing list
> >U-Boot at lists.denx.de
> >http://lists.denx.de/mailman/listinfo/u-boot
> >
> 
> Why merged so quickly?
> 
> I tested this patchset on my device and posted about the issues. [1]
> 
> This should be reworked, since the verify assumptions are too simple
> and doesn't fully match the GPT header creation.
> So this command will fail for some cases of write/verify sequence,
> depending on what the $partitions includes.
> 
> [1] https://www.mail-archive.com/u-boot at lists.denx.de/msg193216.html

Mainly because I skimmed things too quickly, sorry.  Also in the future
(and this applies to anyone that's a custodian, and people can also
manage their own patches if they login) please update patches you're
asking for changes on in patchwork, it really does help me keep an eye
on things.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151124/6054d188/attachment.sig>

[U-Boot] [U-Boot, v2, 4/5] gpt: part: Definition and declaration of GPT verification functions

[Przemyslaw Marczak]

Hi Tom,

On 11/24/2015 07:56 PM, Tom Rini wrote:
> On Tue, Nov 24, 2015 at 10:56:41AM +0100, Przemyslaw Marczak wrote:
>> Hello Tom,
>>
>> On 11/23/2015 11:44 PM, Tom Rini wrote:
>>> On Fri, Nov 20, 2015 at 08:06:16AM +0100, Lukasz Majewski wrote:
>>>
>>>> This commit provides definition and declaration of GPT verification
>>>> functions - namely gpt_verify_headers() and gpt_verify_partitions().
>>>> The former is used to only check CRC32 of GPT's header and PTEs.
>>>> The latter examines each partition entry and compare attributes such as:
>>>> name, start offset and size with ones provided at '$partitions' env
>>>> variable.
>>>>
>>>> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>> Reviewed-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>>
>>> Applied to u-boot/master, thanks!
>>>
>>>
>>>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>>
>> Why merged so quickly?
>>
>> I tested this patchset on my device and posted about the issues. [1]
>>
>> This should be reworked, since the verify assumptions are too simple
>> and doesn't fully match the GPT header creation.
>> So this command will fail for some cases of write/verify sequence,
>> depending on what the $partitions includes.
>>
>> [1] https://www.mail-archive.com/u-boot at lists.denx.de/msg193216.html
>
> Mainly because I skimmed things too quickly, sorry.  Also in the future
> (and this applies to anyone that's a custodian, and people can also
> manage their own patches if they login) please update patches you're
> asking for changes on in patchwork, it really does help me keep an eye
> on things.  Thanks!
>

OK, that's a good point.

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

[U-Boot] [PATCH v2 5/5] gpt: command: Extend gpt command to support GPT table verification

[Lukasz Majewski]

This commit adds support for "gpt verify" command, which verifies
correctness of on-board stored GPT partition table.
As the optional parameter one can provide '$partitons' environment variable
to check if partition data (size, offset, name) is correct.

This command should be regarded as complementary one to "gpt restore".

Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Reviewed-by: Tom Rini <trini@konsulko.com>

---
Changes for v2:
- Replace puts() with printf()
---
 common/cmd_gpt.c | 92 +++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 67 insertions(+), 25 deletions(-)

diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
index 12419b6..460809c 100644
--- a/common/cmd_gpt.c
+++ b/common/cmd_gpt.c
@@ -1,6 +1,9 @@
 /*
  * cmd_gpt.c -- GPT (GUID Partition Table) handling command
  *
+ * Copyright (C) 2015
+ * Lukasz Majewski <l.majewski@majess.pl>
+ *
  * Copyright (C) 2012 Samsung Electronics
  * author: Lukasz Majewski <l.majewski@samsung.com>
  * author: Piotr Wilczek <p.wilczek@samsung.com>
@@ -15,6 +18,7 @@
 #include <exports.h>
 #include <linux/ctype.h>
 #include <div64.h>
+#include <memalign.h>
 
 #ifndef CONFIG_PARTITION_UUIDS
 #error CONFIG_PARTITION_UUIDS must be enabled for CONFIG_CMD_GPT to be enabled
@@ -314,6 +318,43 @@ static int gpt_default(block_dev_desc_t *blk_dev_desc, const char *str_part)
 	return ret;
 }
 
+static int gpt_verify(block_dev_desc_t *blk_dev_desc, const char *str_part)
+{
+	ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1,
+				     blk_dev_desc->blksz);
+	disk_partition_t *partitions = NULL;
+	gpt_entry *gpt_pte = NULL;
+	char *str_disk_guid;
+	u8 part_count = 0;
+	int ret = 0;
+
+	/* fill partitions */
+	ret = set_gpt_info(blk_dev_desc, str_part,
+			&str_disk_guid, &partitions, &part_count);
+	if (ret) {
+		if (ret == -1) {
+			printf("No partition list provided - only basic check\n");
+			ret = gpt_verify_headers(blk_dev_desc, gpt_head,
+						 &gpt_pte);
+			goto out;
+		}
+		if (ret == -2)
+			printf("Missing disk guid\n");
+		if ((ret == -3) || (ret == -4))
+			printf("Partition list incomplete\n");
+		return -1;
+	}
+
+	/* Check partition layout with provided pattern */
+	ret = gpt_verify_partitions(blk_dev_desc, partitions, part_count,
+				    gpt_head, &gpt_pte);
+	free(str_disk_guid);
+	free(partitions);
+ out:
+	free(gpt_pte);
+	return ret;
+}
+
 /**
  * do_gpt(): Perform GPT operations
  *
@@ -329,39 +370,40 @@ static int do_gpt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	int ret = CMD_RET_SUCCESS;
 	int dev = 0;
 	char *ep;
-	block_dev_desc_t *blk_dev_desc;
+	block_dev_desc_t *blk_dev_desc = NULL;
 
-	if (argc < 5)
+	if (argc < 4 || argc > 5)
 		return CMD_RET_USAGE;
 
-	/* command: 'write' */
-	if ((strcmp(argv[1], "write") == 0) && (argc == 5)) {
-		dev = (int)simple_strtoul(argv[3], &ep, 10);
-		if (!ep || ep[0] != '\0') {
-			printf("'%s' is not a number\n", argv[3]);
-			return CMD_RET_USAGE;
-		}
-		blk_dev_desc = get_dev(argv[2], dev);
-		if (!blk_dev_desc) {
-			printf("%s: %s dev %d NOT available\n",
-			       __func__, argv[2], dev);
-			return CMD_RET_FAILURE;
-		}
-
-		puts("Writing GPT: ");
+	dev = (int)simple_strtoul(argv[3], &ep, 10);
+	if (!ep || ep[0] != '\0') {
+		printf("'%s' is not a number\n", argv[3]);
+		return CMD_RET_USAGE;
+	}
+	blk_dev_desc = get_dev(argv[2], dev);
+	if (!blk_dev_desc) {
+		printf("%s: %s dev %d NOT available\n",
+		       __func__, argv[2], dev);
+		return CMD_RET_FAILURE;
+	}
 
+	if ((strcmp(argv[1], "write") == 0) && (argc == 5)) {
+		printf("Writing GPT: ");
 		ret = gpt_default(blk_dev_desc, argv[4]);
-		if (!ret) {
-			puts("success!\n");
-			return CMD_RET_SUCCESS;
-		} else {
-			puts("error!\n");
-			return CMD_RET_FAILURE;
-		}
+	} else if ((strcmp(argv[1], "verify") == 0)) {
+		ret = gpt_verify(blk_dev_desc, argv[4]);
+		printf("Verify GPT: ");
 	} else {
 		return CMD_RET_USAGE;
 	}
-	return ret;
+
+	if (ret) {
+		printf("error!\n");
+		return CMD_RET_FAILURE;
+	}
+
+	printf("success!\n");
+	return CMD_RET_SUCCESS;
 }
 
 U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
-- 
2.1.4

[U-Boot] [PATCH v2 5/5] gpt: command: Extend gpt command to support GPT table verification

[Przemyslaw Marczak]

Hello Lukasz,

On 11/20/2015 08:06 AM, Lukasz Majewski wrote:
> This commit adds support for "gpt verify" command, which verifies
> correctness of on-board stored GPT partition table.
> As the optional parameter one can provide '$partitons' environment variable

$partitions

> to check if partition data (size, offset, name) is correct.
>
> This command should be regarded as complementary one to "gpt restore".

... one to "gpt write" ?

>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> ---
> Changes for v2:
> - Replace puts() with printf()
> ---
>   common/cmd_gpt.c | 92 +++++++++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 67 insertions(+), 25 deletions(-)
>
> diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
> index 12419b6..460809c 100644
> --- a/common/cmd_gpt.c
> +++ b/common/cmd_gpt.c
> @@ -1,6 +1,9 @@
>   /*
>    * cmd_gpt.c -- GPT (GUID Partition Table) handling command
>    *
> + * Copyright (C) 2015
> + * Lukasz Majewski <l.majewski@majess.pl>
> + *
>    * Copyright (C) 2012 Samsung Electronics
>    * author: Lukasz Majewski <l.majewski@samsung.com>
>    * author: Piotr Wilczek <p.wilczek@samsung.com>
> @@ -15,6 +18,7 @@
>   #include <exports.h>
>   #include <linux/ctype.h>
>   #include <div64.h>
> +#include <memalign.h>
>
>   #ifndef CONFIG_PARTITION_UUIDS
>   #error CONFIG_PARTITION_UUIDS must be enabled for CONFIG_CMD_GPT to be enabled
> @@ -314,6 +318,43 @@ static int gpt_default(block_dev_desc_t *blk_dev_desc, const char *str_part)
>   	return ret;
>   }
>
> +static int gpt_verify(block_dev_desc_t *blk_dev_desc, const char *str_part)
> +{
> +	ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1,
> +				     blk_dev_desc->blksz);
> +	disk_partition_t *partitions = NULL;
> +	gpt_entry *gpt_pte = NULL;
> +	char *str_disk_guid;
> +	u8 part_count = 0;
> +	int ret = 0;
> +

Can you move the simple verify to the beginning?

if (!str_part) {
	ret = gpt_verify_headers(blk_dev_desc, gpt_head, &gpt_pte);
	goto out;
}


> +	/* fill partitions */
> +	ret = set_gpt_info(blk_dev_desc, str_part,
> +			&str_disk_guid, &partitions, &part_count);
> +	if (ret) {
> +		if (ret == -1) {
> +			printf("No partition list provided - only basic check\n");
> +			ret = gpt_verify_headers(blk_dev_desc, gpt_head,
> +						 &gpt_pte);
> +			goto out;
> +		}
> +		if (ret == -2)
> +			printf("Missing disk guid\n");
> +		if ((ret == -3) || (ret == -4))
> +			printf("Partition list incomplete\n");
> +		return -1;
> +	}
> +
> +	/* Check partition layout with provided pattern */
> +	ret = gpt_verify_partitions(blk_dev_desc, partitions, part_count,
> +				    gpt_head, &gpt_pte);
> +	free(str_disk_guid);
> +	free(partitions);
> + out:
> +	free(gpt_pte);
> +	return ret;
> +}
> +
>   /**
>    * do_gpt(): Perform GPT operations
>    *
> @@ -329,39 +370,40 @@ static int do_gpt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   	int ret = CMD_RET_SUCCESS;
>   	int dev = 0;
>   	char *ep;
> -	block_dev_desc_t *blk_dev_desc;
> +	block_dev_desc_t *blk_dev_desc = NULL;
>
> -	if (argc < 5)
> +	if (argc < 4 || argc > 5)
>   		return CMD_RET_USAGE;
>
> -	/* command: 'write' */
> -	if ((strcmp(argv[1], "write") == 0) && (argc == 5)) {
> -		dev = (int)simple_strtoul(argv[3], &ep, 10);
> -		if (!ep || ep[0] != '\0') {
> -			printf("'%s' is not a number\n", argv[3]);
> -			return CMD_RET_USAGE;
> -		}
> -		blk_dev_desc = get_dev(argv[2], dev);
> -		if (!blk_dev_desc) {
> -			printf("%s: %s dev %d NOT available\n",
> -			       __func__, argv[2], dev);
> -			return CMD_RET_FAILURE;
> -		}
> -
> -		puts("Writing GPT: ");
> +	dev = (int)simple_strtoul(argv[3], &ep, 10);
> +	if (!ep || ep[0] != '\0') {
> +		printf("'%s' is not a number\n", argv[3]);
> +		return CMD_RET_USAGE;
> +	}
> +	blk_dev_desc = get_dev(argv[2], dev);
> +	if (!blk_dev_desc) {
> +		printf("%s: %s dev %d NOT available\n",
> +		       __func__, argv[2], dev);
> +		return CMD_RET_FAILURE;
> +	}
>
> +	if ((strcmp(argv[1], "write") == 0) && (argc == 5)) {
> +		printf("Writing GPT: ");
>   		ret = gpt_default(blk_dev_desc, argv[4]);
> -		if (!ret) {
> -			puts("success!\n");
> -			return CMD_RET_SUCCESS;
> -		} else {
> -			puts("error!\n");
> -			return CMD_RET_FAILURE;
> -		}
> +	} else if ((strcmp(argv[1], "verify") == 0)) {
> +		ret = gpt_verify(blk_dev_desc, argv[4]);
> +		printf("Verify GPT: ");
>   	} else {
>   		return CMD_RET_USAGE;
>   	}
> -	return ret;
> +
> +	if (ret) {
> +		printf("error!\n");
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	printf("success!\n");
> +	return CMD_RET_SUCCESS;
>   }
>
>   U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
>

Reviewed-by: Przemyslaw Marczak <p.marczak@samsung.com>

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

[U-Boot] [U-Boot, v2, 5/5] gpt: command: Extend gpt command to support GPT table verification

[Tom Rini]

On Fri, Nov 20, 2015 at 08:06:17AM +0100, Lukasz Majewski wrote:

> This commit adds support for "gpt verify" command, which verifies
> correctness of on-board stored GPT partition table.
> As the optional parameter one can provide '$partitons' environment variable
> to check if partition data (size, offset, name) is correct.
> 
> This command should be regarded as complementary one to "gpt restore".
> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Przemyslaw Marczak <p.marczak@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151123/85147ed2/attachment.sig>

[U-Boot] [PATCH v2 0/5] gpt: command: Add support for "gpt verify" command

[Tom Rini]

On Fri, Nov 20, 2015 at 08:06:12AM +0100, Lukasz Majewski wrote:

> Up till now "gpt" command was only able to write (i.e. restore) GPT
> partition on specified medium with information provided at "$partitions"
> env variable.
> 
> This patch series adds complementary feature - namely "gpt verify", which
> allows checking (at e.g. boot time) if previously created GPT layout is
> still correct.
> 
> This patch series clearly applies on top of u-boot/master:
> SHA1: 736d1746fb7b8f7cd70657a4a72db2b6bd8de40e
> 
> It has been tested on Beaglebone Black development board (Sitara AM335x
> CPU).
> 
> 
> Lukasz Majewski (5):
>   gpt: command: Remove duplicated check for empty partition description
>   gpt: doc: README: Update README entry for gpt verify extension
>   gpt: doc: Update gpt command's help description
>   gpt: part: Definition and declaration of GPT verification functions
>   gpt: command: Extend gpt command to support GPT table verification

So, I was going to say one thing here, but it turns out I guessed wrong
about which part of the previous series still applied (it was 4/6, not
3/6, even 'tho #3 was not included in the v2 posts).  In the future,
unless you're just changing one patch in the series, please repost the
whole thing, it makes applying easier (and you can just say Changes in
vX: None in the changelog for the unchanged patch).  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151123/5acf5aaf/attachment.sig>