[U-Boot] [PATCH v2 4/5] gpt: part: Definition and declaration of GPT verification functions
Przemyslaw Marczak
p.marczak at samsung.com
Fri Nov 20 12:19:01 CET 2015
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 at majess.pl>
> Reviewed-by: Tom Rini <trini at 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 at samsung.com>
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
More information about the U-Boot
mailing list