From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Fri, 20 Nov 2015 12:19:01 +0100 Subject: [U-Boot] [PATCH v2 4/5] gpt: part: Definition and declaration of GPT verification functions In-Reply-To: <1448003177-26917-5-git-send-email-l.majewski@majess.pl> References: <1447396932-26644-1-git-send-email-l.majewski@majess.pl> <1448003177-26917-1-git-send-email-l.majewski@majess.pl> <1448003177-26917-5-git-send-email-l.majewski@majess.pl> Message-ID: <564F01A5.7040801@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > Reviewed-by: Tom Rini > > --- > 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 Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com