[U-Boot] [PATCH v2] fastboot: handle flash write to GPT partition

[U-Boot] [PATCH v2] fastboot: handle flash write to GPT partition

[Steve Rae]

Implement a feature to allow fastboot to write the downloaded image
to the space reserved for the Protective MBR and the Primary GUID
Partition Table.

Signed-off-by: Steve Rae <srae@broadcom.com>
---

Changes in v2:
add validation of the GPT before writing to flash
(suggested by: Lukasz Majewski <l.majewski@samsung.com>)

 README          |  7 +++++++
 common/fb_mmc.c | 26 +++++++++++++++++++++++---
 disk/part_efi.c | 37 +++++++++++++++++++++++++++++++++++++
 include/part.h  | 10 ++++++++++
 4 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/README b/README
index 4ca04d0..5f50cdd 100644
--- a/README
+++ b/README
@@ -1773,6 +1773,13 @@ The following options need to be configured:
 		regarding the non-volatile storage device. Define this to
 		the eMMC device that fastboot should use to store the image.
 
+		CONFIG_FASTBOOT_GPT_NAME
+		The fastboot "flash" command supports writing the downloaded
+		image to the Protective MBR and the Primary GUID Partition
+		Table. This occurs when the specified "partition name" on the
+		"fastboot flash" command line matches this value.
+		Default is GPT_ENTRY_NAME (currently "gpt") if undefined.
+
 - Journaling Flash filesystem support:
 		CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF, CONFIG_JFFS2_NAND_SIZE,
 		CONFIG_JFFS2_NAND_DEV
diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index fb06d8a..a646a7b 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -4,12 +4,17 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#include <config.h>
 #include <common.h>
 #include <fb_mmc.h>
 #include <part.h>
 #include <aboot.h>
 #include <sparse_format.h>
 
+#ifndef CONFIG_FASTBOOT_GPT_NAME
+#define CONFIG_FASTBOOT_GPT_NAME GPT_ENTRY_NAME
+#endif
+
 /* The 64 defined bytes plus the '\0' */
 #define RESPONSE_LEN	(64 + 1)
 
@@ -62,9 +67,9 @@ static void write_raw_image(block_dev_desc_t *dev_desc, disk_partition_t *info,
 void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 			unsigned int download_bytes, char *response)
 {
-	int ret;
 	block_dev_desc_t *dev_desc;
 	disk_partition_t info;
+	lbaint_t blksz;
 
 	/* initialize the response buffer */
 	response_str = response;
@@ -76,8 +81,23 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 		return;
 	}
 
-	ret = get_partition_info_efi_by_name(dev_desc, cmd, &info);
-	if (ret) {
+	if (strcmp(cmd, CONFIG_FASTBOOT_GPT_NAME) == 0) {
+		printf("%s: updating GUID Partition Table (including MBR)\n",
+		       __func__);
+		/* start at Protective MBR */
+		info.start = (GPT_PRIMARY_PARTITION_TABLE_LBA - 1);
+		blksz = dev_desc->blksz;
+		info.blksz = blksz;
+		/* assume that the Partition Entry Array starts in LBA 2 */
+		info.size = (2 + (GPT_ENTRY_NUMBERS * GPT_ENTRY_SIZE) / blksz);
+
+		if (is_valid_gpt_primary(download_buffer, dev_desc->blksz)) {
+			printf("%s: invalid GPT - refusing to write to flash\n",
+			       __func__);
+			fastboot_fail("invalid GPT partition");
+			return;
+		}
+	} else if (get_partition_info_efi_by_name(dev_desc, cmd, &info)) {
 		error("cannot find partition: '%s'\n", cmd);
 		fastboot_fail("cannot find partition");
 		return;
diff --git a/disk/part_efi.c b/disk/part_efi.c
index efed58f..86cb160 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -455,6 +455,43 @@ err:
 	free(gpt_h);
 	return ret;
 }
+
+int is_valid_gpt_primary(void *buf, unsigned long blksz)
+{
+	int rc = 0;
+	gpt_header *gpt_h;
+	gpt_entry *gpt_e;
+	gpt_header gpt_hdr;
+	uint32_t calc_crc32;
+
+	/* GPT Header starts at "LBA[1]" in the buffer */
+	gpt_h = buf + blksz;
+	if (gpt_h->signature != cpu_to_le64(GPT_HEADER_SIGNATURE)) {
+		printf("%s: 'signature' is invalid\n", __func__);
+		rc += 1;
+	}
+
+	memcpy(&gpt_hdr, gpt_h, sizeof(gpt_header));
+	gpt_hdr.header_crc32 = 0;
+	calc_crc32 = efi_crc32((const unsigned char *)&gpt_hdr,
+			       le32_to_cpu(gpt_h->header_size));
+	if (gpt_h->header_crc32 != cpu_to_le32(calc_crc32)) {
+		printf("%s: 'header_crc32' is invalid\n", __func__);
+		rc += 1;
+	}
+
+	/* GPT Entries start at "LBA[2]" in the buffer */
+	gpt_e = buf + (2 * blksz);
+	calc_crc32 = efi_crc32((const unsigned char *)gpt_e,
+			       le32_to_cpu(gpt_h->num_partition_entries) *
+			       le32_to_cpu(gpt_h->sizeof_partition_entry));
+	if (gpt_h->partition_entry_array_crc32 != cpu_to_le32(calc_crc32)) {
+		printf("%s: 'partition_entry_array_crc32' is invalid\n",
+		       __func__);
+		rc += 1;
+	}
+	return rc;
+}
 #endif
 
 /*
diff --git a/include/part.h b/include/part.h
index a496a4a..9cfa4c9 100644
--- a/include/part.h
+++ b/include/part.h
@@ -244,6 +244,16 @@ int gpt_fill_header(block_dev_desc_t *dev_desc, gpt_header *gpt_h,
  */
 int gpt_restore(block_dev_desc_t *dev_desc, char *str_disk_guid,
 		disk_partition_t *partitions, const int parts_count);
+
+/**
+ * is_valid_gpt_primary() - Ensure that the Primary GPT information is valid
+ *
+ * @param buf - buffer which contains the Primary GPT info
+ * @param blksz -  block size (in bytes)
+ *
+ * @return - '0' on success, otherwise error
+ */
+int is_valid_gpt_primary(void *buf, unsigned long blksz);
 #endif
 
 #endif /* _PART_H */
-- 
1.8.5

[U-Boot] [PATCH v2] fastboot: handle flash write to GPT partition

[Lukasz Majewski]

Hi Steve,

> Implement a feature to allow fastboot to write the downloaded image
> to the space reserved for the Protective MBR and the Primary GUID
> Partition Table.
> 
> Signed-off-by: Steve Rae <srae@broadcom.com>
> ---
> 
> Changes in v2:
> add validation of the GPT before writing to flash
> (suggested by: Lukasz Majewski <l.majewski@samsung.com>)
> 
>  README          |  7 +++++++
>  common/fb_mmc.c | 26 +++++++++++++++++++++++---
>  disk/part_efi.c | 37 +++++++++++++++++++++++++++++++++++++
>  include/part.h  | 10 ++++++++++
>  4 files changed, 77 insertions(+), 3 deletions(-)
> 
> diff --git a/README b/README
> index 4ca04d0..5f50cdd 100644
> --- a/README
> +++ b/README
> @@ -1773,6 +1773,13 @@ The following options need to be configured:
>  		regarding the non-volatile storage device. Define
> this to the eMMC device that fastboot should use to store the image.
>  
> +		CONFIG_FASTBOOT_GPT_NAME
> +		The fastboot "flash" command supports writing the
> downloaded
> +		image to the Protective MBR and the Primary GUID
> Partition
> +		Table. This occurs when the specified "partition
> name" on the
> +		"fastboot flash" command line matches this value.
> +		Default is GPT_ENTRY_NAME (currently "gpt") if
> undefined. +
>  - Journaling Flash filesystem support:
>  		CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF,
> CONFIG_JFFS2_NAND_SIZE, CONFIG_JFFS2_NAND_DEV
> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> index fb06d8a..a646a7b 100644
> --- a/common/fb_mmc.c
> +++ b/common/fb_mmc.c
> @@ -4,12 +4,17 @@
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
>  
> +#include <config.h>
>  #include <common.h>
>  #include <fb_mmc.h>
>  #include <part.h>
>  #include <aboot.h>
>  #include <sparse_format.h>
>  
> +#ifndef CONFIG_FASTBOOT_GPT_NAME
> +#define CONFIG_FASTBOOT_GPT_NAME GPT_ENTRY_NAME
> +#endif
> +
>  /* The 64 defined bytes plus the '\0' */
>  #define RESPONSE_LEN	(64 + 1)
>  
> @@ -62,9 +67,9 @@ static void write_raw_image(block_dev_desc_t
> *dev_desc, disk_partition_t *info, void fb_mmc_flash_write(const char
> *cmd, void *download_buffer, unsigned int download_bytes, char
> *response) {
> -	int ret;
>  	block_dev_desc_t *dev_desc;
>  	disk_partition_t info;
> +	lbaint_t blksz;
>  
>  	/* initialize the response buffer */
>  	response_str = response;
> @@ -76,8 +81,23 @@ void fb_mmc_flash_write(const char *cmd, void
> *download_buffer, return;
>  	}
>  
> -	ret = get_partition_info_efi_by_name(dev_desc, cmd, &info);
> -	if (ret) {
> +	if (strcmp(cmd, CONFIG_FASTBOOT_GPT_NAME) == 0) {
> +		printf("%s: updating GUID Partition Table (including
> MBR)\n",
> +		       __func__);
> +		/* start at Protective MBR */
> +		info.start = (GPT_PRIMARY_PARTITION_TABLE_LBA - 1);
> +		blksz = dev_desc->blksz;
> +		info.blksz = blksz;
> +		/* assume that the Partition Entry Array starts in
> LBA 2 */
> +		info.size = (2 + (GPT_ENTRY_NUMBERS *
> GPT_ENTRY_SIZE) / blksz); +
> +		if (is_valid_gpt_primary(download_buffer,
> dev_desc->blksz)) {
> +			printf("%s: invalid GPT - refusing to write
> to flash\n",
> +			       __func__);
> +			fastboot_fail("invalid GPT partition");
> +			return;
> +		}

IMHO some extra code to update secondary (backup) GPT is needed here.

A good example may be at part_efi.c - print_part_efi() function [1].

> +	} else if (get_partition_info_efi_by_name(dev_desc, cmd,
> &info)) { error("cannot find partition: '%s'\n", cmd);
>  		fastboot_fail("cannot find partition");
>  		return;
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index efed58f..86cb160 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -455,6 +455,43 @@ err:
>  	free(gpt_h);
>  	return ret;
>  }
> +
> +int is_valid_gpt_primary(void *buf, unsigned long blksz)

The name is a bit misleading - I rather though about something like
is_valid_gpt_buf() and write it in a similar way to is_valid_gpt()
which would allow using it for both primary and secondary GPTs (like in
[1]).

> +{
> +	int rc = 0;
> +	gpt_header *gpt_h;
> +	gpt_entry *gpt_e;
> +	gpt_header gpt_hdr;
> +	uint32_t calc_crc32;
> +
> +	/* GPT Header starts at "LBA[1]" in the buffer */
> +	gpt_h = buf + blksz;
> +	if (gpt_h->signature != cpu_to_le64(GPT_HEADER_SIGNATURE)) {
> +		printf("%s: 'signature' is invalid\n", __func__);
> +		rc += 1;
> +	}
> +
> +	memcpy(&gpt_hdr, gpt_h, sizeof(gpt_header));
> +	gpt_hdr.header_crc32 = 0;
> +	calc_crc32 = efi_crc32((const unsigned char *)&gpt_hdr,
> +			       le32_to_cpu(gpt_h->header_size));
> +	if (gpt_h->header_crc32 != cpu_to_le32(calc_crc32)) {
> +		printf("%s: 'header_crc32' is invalid\n", __func__);
> +		rc += 1;
> +	}
> +
> +	/* GPT Entries start at "LBA[2]" in the buffer */
> +	gpt_e = buf + (2 * blksz);
> +	calc_crc32 = efi_crc32((const unsigned char *)gpt_e,
> +
> le32_to_cpu(gpt_h->num_partition_entries) *
> +
> le32_to_cpu(gpt_h->sizeof_partition_entry));
> +	if (gpt_h->partition_entry_array_crc32 !=
> cpu_to_le32(calc_crc32)) {
> +		printf("%s: 'partition_entry_array_crc32' is
> invalid\n",
> +		       __func__);
> +		rc += 1;
> +	}
> +	return rc;
> +}
>  #endif
>  
>  /*
> diff --git a/include/part.h b/include/part.h
> index a496a4a..9cfa4c9 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -244,6 +244,16 @@ int gpt_fill_header(block_dev_desc_t *dev_desc,
> gpt_header *gpt_h, */
>  int gpt_restore(block_dev_desc_t *dev_desc, char *str_disk_guid,
>  		disk_partition_t *partitions, const int parts_count);
> +
> +/**
> + * is_valid_gpt_primary() - Ensure that the Primary GPT information
> is valid
> + *
> + * @param buf - buffer which contains the Primary GPT info
> + * @param blksz -  block size (in bytes)
> + *
> + * @return - '0' on success, otherwise error
> + */
> +int is_valid_gpt_primary(void *buf, unsigned long blksz);
>  #endif
>  
>  #endif /* _PART_H */



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [PATCH v2] fastboot: handle flash write to GPT partition

[Steve Rae]

Hi Lukasz,

On 14-12-09 01:05 AM, Lukasz Majewski wrote:
> Hi Steve,
>
>> Implement a feature to allow fastboot to write the downloaded image
>> to the space reserved for the Protective MBR and the Primary GUID
>> Partition Table.
>>
>> Signed-off-by: Steve Rae <srae@broadcom.com>
>> ---
>>
>> Changes in v2:
>> add validation of the GPT before writing to flash
>> (suggested by: Lukasz Majewski <l.majewski@samsung.com>)
>>
>>   README          |  7 +++++++
>>   common/fb_mmc.c | 26 +++++++++++++++++++++++---
>>   disk/part_efi.c | 37 +++++++++++++++++++++++++++++++++++++
>>   include/part.h  | 10 ++++++++++
>>   4 files changed, 77 insertions(+), 3 deletions(-)
>>
>> diff --git a/README b/README
>> index 4ca04d0..5f50cdd 100644
>> --- a/README
>> +++ b/README
>> @@ -1773,6 +1773,13 @@ The following options need to be configured:
>>   		regarding the non-volatile storage device. Define
>> this to the eMMC device that fastboot should use to store the image.
>>
>> +		CONFIG_FASTBOOT_GPT_NAME
>> +		The fastboot "flash" command supports writing the
>> downloaded
>> +		image to the Protective MBR and the Primary GUID
>> Partition
>> +		Table. This occurs when the specified "partition
>> name" on the
>> +		"fastboot flash" command line matches this value.
>> +		Default is GPT_ENTRY_NAME (currently "gpt") if
>> undefined. +
>>   - Journaling Flash filesystem support:
>>   		CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF,
>> CONFIG_JFFS2_NAND_SIZE, CONFIG_JFFS2_NAND_DEV
>> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
>> index fb06d8a..a646a7b 100644
>> --- a/common/fb_mmc.c
>> +++ b/common/fb_mmc.c
>> @@ -4,12 +4,17 @@
>>    * SPDX-License-Identifier:	GPL-2.0+
>>    */
>>
>> +#include <config.h>
>>   #include <common.h>
>>   #include <fb_mmc.h>
>>   #include <part.h>
>>   #include <aboot.h>
>>   #include <sparse_format.h>
>>
>> +#ifndef CONFIG_FASTBOOT_GPT_NAME
>> +#define CONFIG_FASTBOOT_GPT_NAME GPT_ENTRY_NAME
>> +#endif
>> +
>>   /* The 64 defined bytes plus the '\0' */
>>   #define RESPONSE_LEN	(64 + 1)
>>
>> @@ -62,9 +67,9 @@ static void write_raw_image(block_dev_desc_t
>> *dev_desc, disk_partition_t *info, void fb_mmc_flash_write(const char
>> *cmd, void *download_buffer, unsigned int download_bytes, char
>> *response) {
>> -	int ret;
>>   	block_dev_desc_t *dev_desc;
>>   	disk_partition_t info;
>> +	lbaint_t blksz;
>>
>>   	/* initialize the response buffer */
>>   	response_str = response;
>> @@ -76,8 +81,23 @@ void fb_mmc_flash_write(const char *cmd, void
>> *download_buffer, return;
>>   	}
>>
>> -	ret = get_partition_info_efi_by_name(dev_desc, cmd, &info);
>> -	if (ret) {
>> +	if (strcmp(cmd, CONFIG_FASTBOOT_GPT_NAME) == 0) {
>> +		printf("%s: updating GUID Partition Table (including
>> MBR)\n",
>> +		       __func__);
>> +		/* start at Protective MBR */
>> +		info.start = (GPT_PRIMARY_PARTITION_TABLE_LBA - 1);
>> +		blksz = dev_desc->blksz;
>> +		info.blksz = blksz;
>> +		/* assume that the Partition Entry Array starts in
>> LBA 2 */
>> +		info.size = (2 + (GPT_ENTRY_NUMBERS *
>> GPT_ENTRY_SIZE) / blksz); +
>> +		if (is_valid_gpt_primary(download_buffer,
>> dev_desc->blksz)) {
>> +			printf("%s: invalid GPT - refusing to write
>> to flash\n",
>> +			       __func__);
>> +			fastboot_fail("invalid GPT partition");
>> +			return;
>> +		}
>
> IMHO some extra code to update secondary (backup) GPT is needed here.
>
> A good example may be at part_efi.c - print_part_efi() function [1].
>

OK -- added "prepare and write Backup GPT" in v3

>> +	} else if (get_partition_info_efi_by_name(dev_desc, cmd,
>> &info)) { error("cannot find partition: '%s'\n", cmd);
>>   		fastboot_fail("cannot find partition");
>>   		return;
>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>> index efed58f..86cb160 100644
>> --- a/disk/part_efi.c
>> +++ b/disk/part_efi.c
>> @@ -455,6 +455,43 @@ err:
>>   	free(gpt_h);
>>   	return ret;
>>   }
>> +
>> +int is_valid_gpt_primary(void *buf, unsigned long blksz)
>
> The name is a bit misleading - I rather though about something like
> is_valid_gpt_buf() and write it in a similar way to is_valid_gpt()
> which would allow using it for both primary and secondary GPTs (like in
> [1]).

OK - I changed the name, however, it still can only handle a buffer 
which contains:
- 512 bytes of MBR, followed by
- 512 bytes of GPT Header, followed by
the GPT Entries at the specified offset (typically 2 * 512 bytes)
Thus, it cannot be used for the Backup GPT anyway....

>
>> +{
>> +	int rc = 0;
>> +	gpt_header *gpt_h;
>> +	gpt_entry *gpt_e;
>> +	gpt_header gpt_hdr;
>> +	uint32_t calc_crc32;
>> +
>> +	/* GPT Header starts at "LBA[1]" in the buffer */
>> +	gpt_h = buf + blksz;
>> +	if (gpt_h->signature != cpu_to_le64(GPT_HEADER_SIGNATURE)) {
>> +		printf("%s: 'signature' is invalid\n", __func__);
>> +		rc += 1;
>> +	}
>> +
>> +	memcpy(&gpt_hdr, gpt_h, sizeof(gpt_header));
>> +	gpt_hdr.header_crc32 = 0;
>> +	calc_crc32 = efi_crc32((const unsigned char *)&gpt_hdr,
>> +			       le32_to_cpu(gpt_h->header_size));
>> +	if (gpt_h->header_crc32 != cpu_to_le32(calc_crc32)) {
>> +		printf("%s: 'header_crc32' is invalid\n", __func__);
>> +		rc += 1;
>> +	}
>> +
>> +	/* GPT Entries start at "LBA[2]" in the buffer */
>> +	gpt_e = buf + (2 * blksz);
>> +	calc_crc32 = efi_crc32((const unsigned char *)gpt_e,
>> +
>> le32_to_cpu(gpt_h->num_partition_entries) *
>> +
>> le32_to_cpu(gpt_h->sizeof_partition_entry));
>> +	if (gpt_h->partition_entry_array_crc32 !=
>> cpu_to_le32(calc_crc32)) {
>> +		printf("%s: 'partition_entry_array_crc32' is
>> invalid\n",
>> +		       __func__);
>> +		rc += 1;
>> +	}
>> +	return rc;
>> +}
>>   #endif
>>
>>   /*
>> diff --git a/include/part.h b/include/part.h
>> index a496a4a..9cfa4c9 100644
>> --- a/include/part.h
>> +++ b/include/part.h
>> @@ -244,6 +244,16 @@ int gpt_fill_header(block_dev_desc_t *dev_desc,
>> gpt_header *gpt_h, */
>>   int gpt_restore(block_dev_desc_t *dev_desc, char *str_disk_guid,
>>   		disk_partition_t *partitions, const int parts_count);
>> +
>> +/**
>> + * is_valid_gpt_primary() - Ensure that the Primary GPT information
>> is valid
>> + *
>> + * @param buf - buffer which contains the Primary GPT info
>> + * @param blksz -  block size (in bytes)
>> + *
>> + * @return - '0' on success, otherwise error
>> + */
>> +int is_valid_gpt_primary(void *buf, unsigned long blksz);
>>   #endif
>>
>>   #endif /* _PART_H */
>
>
>

[U-Boot] [PATCH v2] fastboot: handle flash write to GPT partition

[Lukasz Majewski]

Hi Steve,

> Hi Lukasz,
> 
> On 14-12-09 01:05 AM, Lukasz Majewski wrote:
> > Hi Steve,
> >
> >> Implement a feature to allow fastboot to write the downloaded image
> >> to the space reserved for the Protective MBR and the Primary GUID
> >> Partition Table.
> >>
> >> Signed-off-by: Steve Rae <srae@broadcom.com>
> >> ---
> >>
> >> Changes in v2:
> >> add validation of the GPT before writing to flash
> >> (suggested by: Lukasz Majewski <l.majewski@samsung.com>)
> >>
> >>   README          |  7 +++++++
> >>   common/fb_mmc.c | 26 +++++++++++++++++++++++---
> >>   disk/part_efi.c | 37 +++++++++++++++++++++++++++++++++++++
> >>   include/part.h  | 10 ++++++++++
> >>   4 files changed, 77 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/README b/README
> >> index 4ca04d0..5f50cdd 100644
> >> --- a/README
> >> +++ b/README
> >> @@ -1773,6 +1773,13 @@ The following options need to be configured:
> >>   		regarding the non-volatile storage device. Define
> >> this to the eMMC device that fastboot should use to store the
> >> image.
> >>
> >> +		CONFIG_FASTBOOT_GPT_NAME
> >> +		The fastboot "flash" command supports writing the
> >> downloaded
> >> +		image to the Protective MBR and the Primary GUID
> >> Partition
> >> +		Table. This occurs when the specified "partition
> >> name" on the
> >> +		"fastboot flash" command line matches this value.
> >> +		Default is GPT_ENTRY_NAME (currently "gpt") if
> >> undefined. +
> >>   - Journaling Flash filesystem support:
> >>   		CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF,
> >> CONFIG_JFFS2_NAND_SIZE, CONFIG_JFFS2_NAND_DEV
> >> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> >> index fb06d8a..a646a7b 100644
> >> --- a/common/fb_mmc.c
> >> +++ b/common/fb_mmc.c
> >> @@ -4,12 +4,17 @@
> >>    * SPDX-License-Identifier:	GPL-2.0+
> >>    */
> >>
> >> +#include <config.h>
> >>   #include <common.h>
> >>   #include <fb_mmc.h>
> >>   #include <part.h>
> >>   #include <aboot.h>
> >>   #include <sparse_format.h>
> >>
> >> +#ifndef CONFIG_FASTBOOT_GPT_NAME
> >> +#define CONFIG_FASTBOOT_GPT_NAME GPT_ENTRY_NAME
> >> +#endif
> >> +
> >>   /* The 64 defined bytes plus the '\0' */
> >>   #define RESPONSE_LEN	(64 + 1)
> >>
> >> @@ -62,9 +67,9 @@ static void write_raw_image(block_dev_desc_t
> >> *dev_desc, disk_partition_t *info, void fb_mmc_flash_write(const
> >> char *cmd, void *download_buffer, unsigned int download_bytes, char
> >> *response) {
> >> -	int ret;
> >>   	block_dev_desc_t *dev_desc;
> >>   	disk_partition_t info;
> >> +	lbaint_t blksz;
> >>
> >>   	/* initialize the response buffer */
> >>   	response_str = response;
> >> @@ -76,8 +81,23 @@ void fb_mmc_flash_write(const char *cmd, void
> >> *download_buffer, return;
> >>   	}
> >>
> >> -	ret = get_partition_info_efi_by_name(dev_desc, cmd,
> >> &info);
> >> -	if (ret) {
> >> +	if (strcmp(cmd, CONFIG_FASTBOOT_GPT_NAME) == 0) {
> >> +		printf("%s: updating GUID Partition Table
> >> (including MBR)\n",
> >> +		       __func__);
> >> +		/* start at Protective MBR */
> >> +		info.start = (GPT_PRIMARY_PARTITION_TABLE_LBA -
> >> 1);
> >> +		blksz = dev_desc->blksz;
> >> +		info.blksz = blksz;
> >> +		/* assume that the Partition Entry Array starts in
> >> LBA 2 */
> >> +		info.size = (2 + (GPT_ENTRY_NUMBERS *
> >> GPT_ENTRY_SIZE) / blksz); +
> >> +		if (is_valid_gpt_primary(download_buffer,
> >> dev_desc->blksz)) {
> >> +			printf("%s: invalid GPT - refusing to
> >> write to flash\n",
> >> +			       __func__);
> >> +			fastboot_fail("invalid GPT partition");
> >> +			return;
> >> +		}
> >
> > IMHO some extra code to update secondary (backup) GPT is needed
> > here.
> >
> > A good example may be at part_efi.c - print_part_efi() function [1].
> >
> 
> OK -- added "prepare and write Backup GPT" in v3
> 
> >> +	} else if (get_partition_info_efi_by_name(dev_desc, cmd,
> >> &info)) { error("cannot find partition: '%s'\n", cmd);
> >>   		fastboot_fail("cannot find partition");
> >>   		return;
> >> diff --git a/disk/part_efi.c b/disk/part_efi.c
> >> index efed58f..86cb160 100644
> >> --- a/disk/part_efi.c
> >> +++ b/disk/part_efi.c
> >> @@ -455,6 +455,43 @@ err:
> >>   	free(gpt_h);
> >>   	return ret;
> >>   }
> >> +
> >> +int is_valid_gpt_primary(void *buf, unsigned long blksz)
> >
> > The name is a bit misleading - I rather though about something like
> > is_valid_gpt_buf() and write it in a similar way to is_valid_gpt()
> > which would allow using it for both primary and secondary GPTs
> > (like in [1]).
> 
> OK - I changed the name, however, it still can only handle a buffer 
> which contains:
> - 512 bytes of MBR, followed by
> - 512 bytes of GPT Header, followed by
> the GPT Entries at the specified offset (typically 2 * 512 bytes)
> Thus, it cannot be used for the Backup GPT anyway....

I think that it should be possible to reuse the code:

1. There is "is_pmbr_valid()" function @ part_efi.c - so you don't need
to check pmbr validity in the is_valid_gpt_buf function.

2. If 1. is done, then one function can be used to check CRC's for
primary and secondary GPT headers and entries (as is done in the
print_part_efi()).

> 
> >
> >> +{
> >> +	int rc = 0;
> >> +	gpt_header *gpt_h;
> >> +	gpt_entry *gpt_e;
> >> +	gpt_header gpt_hdr;
> >> +	uint32_t calc_crc32;
> >> +
> >> +	/* GPT Header starts at "LBA[1]" in the buffer */
> >> +	gpt_h = buf + blksz;
> >> +	if (gpt_h->signature !=
> >> cpu_to_le64(GPT_HEADER_SIGNATURE)) {
> >> +		printf("%s: 'signature' is invalid\n", __func__);
> >> +		rc += 1;
> >> +	}
> >> +
> >> +	memcpy(&gpt_hdr, gpt_h, sizeof(gpt_header));
> >> +	gpt_hdr.header_crc32 = 0;
> >> +	calc_crc32 = efi_crc32((const unsigned char *)&gpt_hdr,
> >> +			       le32_to_cpu(gpt_h->header_size));
> >> +	if (gpt_h->header_crc32 != cpu_to_le32(calc_crc32)) {
> >> +		printf("%s: 'header_crc32' is invalid\n",
> >> __func__);
> >> +		rc += 1;
> >> +	}
> >> +
> >> +	/* GPT Entries start at "LBA[2]" in the buffer */
> >> +	gpt_e = buf + (2 * blksz);
> >> +	calc_crc32 = efi_crc32((const unsigned char *)gpt_e,
> >> +
> >> le32_to_cpu(gpt_h->num_partition_entries) *
> >> +
> >> le32_to_cpu(gpt_h->sizeof_partition_entry));
> >> +	if (gpt_h->partition_entry_array_crc32 !=
> >> cpu_to_le32(calc_crc32)) {
> >> +		printf("%s: 'partition_entry_array_crc32' is
> >> invalid\n",
> >> +		       __func__);
> >> +		rc += 1;
> >> +	}
> >> +	return rc;
> >> +}
> >>   #endif
> >>
> >>   /*
> >> diff --git a/include/part.h b/include/part.h
> >> index a496a4a..9cfa4c9 100644
> >> --- a/include/part.h
> >> +++ b/include/part.h
> >> @@ -244,6 +244,16 @@ int gpt_fill_header(block_dev_desc_t
> >> *dev_desc, gpt_header *gpt_h, */
> >>   int gpt_restore(block_dev_desc_t *dev_desc, char *str_disk_guid,
> >>   		disk_partition_t *partitions, const int
> >> parts_count); +
> >> +/**
> >> + * is_valid_gpt_primary() - Ensure that the Primary GPT
> >> information is valid
> >> + *
> >> + * @param buf - buffer which contains the Primary GPT info
> >> + * @param blksz -  block size (in bytes)
> >> + *
> >> + * @return - '0' on success, otherwise error
> >> + */
> >> +int is_valid_gpt_primary(void *buf, unsigned long blksz);
> >>   #endif
> >>
> >>   #endif /* _PART_H */
> >
> >
> >



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group