[PATCH 2/2] part: efi: Treat unused partition type GUID as a valid case

[Heinrich Schuchardt]

On 3/28/24 23:29, Sam Protsenko wrote:
> Some platforms use the "unused" (all-zero) GUID as a partition type GUID
> to make some partitions hidden from the OS. For example, Samsung phones
> and other devices often have GPT partition tables like that, created by
> their "gpt_builder" tool [1]. All partitions with FILESYS=0 value
> (second column in [2] file) will be created in a way that:
>    1. Partition type GUID will be all-zero ("unused")
>    2. Attributes[48:49] bits will be set to 0 (whereas non-zero values
>       mean the partition is visible to the OS: 1=raw, 2=ext4, 3=f2fs)

The UEFI specification is defining what a GPT partition table has to
look like.

According the specification partition type GUID
00000000-0000-0000-0000-000000000000 marks an "unused entry" in the
partition table.

An unused partition table entry cannot define a partition. It is one of
the entries, that you skip over when enumerating via your patch 1/2.

With this patch 128 partition table entries are printed for an image
having a single partition.

=> part list host 0

Partition Map for HOST device 0  --   Partition Type: EFI

Part    Start LBA       End LBA         Name
         Attributes
         Type GUID
         Partition GUID
   1     0x00000800      0x0001f7ff      "EFI system partition"
         attrs:  0x0000000000000005
         type:   c12a7328-f81f-11d2-ba4b-00a0c93ec93b
                 (system)
         guid:   ee474198-4601-4d5c-81f0-54acf904dd40
   2     0x00000000      0x00000000      ""
         attrs:  0x0000000000000000
         type:   00000000-0000-0000-0000-000000000000
                 (00000000-0000-0000-0000-000000000000)
         guid:   00000000-0000-0000-0000-000000000000
...

128     0x00000000      0x00000000      ""
         attrs:  0x0000000000000000
         type:   00000000-0000-0000-0000-000000000000
                 (00000000-0000-0000-0000-000000000000)
         guid:   00000000-0000-0000-0000-000000000000

This is definitively wrong.

>
> Although it's true that Linux kernel verifies the partition type GUID to
> be non-zero (in block/partitions/efi.c, is_pte_valid() function), where
> its U-Boot counterpart code was borrowed from originally, in case of
> U-Boot we want to handle partitions with "unused" GUIDs the same way as
> any other valid partitions, to allow the user to interact with those
> (e.g. list partitions using "part list", be able to flash those via
> fastboot, etc).

You cannot interact with a non-existing partition.

You may create a new partition in any empty slot that Samsung's tool has
left in the partition table and then write to it. No patch is needed for
this.

Best regards

Heinrich

>
> [1] https://gitlab.com/Linaro/96boards/e850-96/tools/gpt/
> [2] https://gitlab.com/Linaro/96boards/e850-96/tools/gpt/-/blob/master/gpt_layout
>
> Fixes: 07f3d789b9be ("Add support for CONFIG_EFI_PARTITION (GUID Partition Table)")
> Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> ---
>   disk/part_efi.c | 21 +++++----------------
>   1 file changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index 4ce9243ef25c..6b138abae0a6 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -1166,28 +1166,17 @@ static gpt_entry *alloc_read_gpt_entries(struct blk_desc *desc,
>    */
>   static int is_pte_valid(gpt_entry * pte)
>   {
> -	efi_guid_t unused_guid;
> +	/*
> +	 * NOTE: Do not check unused (zero) GUIDs here, it's considered a valid
> +	 * case in U-Boot.
> +	 */
>
>   	if (!pte) {
>   		log_debug("Invalid Argument(s)\n");
>   		return 0;
>   	}
>
> -	/* Only one validation for now:
> -	 * The GUID Partition Type != Unused Entry (ALL-ZERO)
> -	 */
> -	memset(unused_guid.b, 0, sizeof(unused_guid.b));
> -
> -	if (memcmp(pte->partition_type_guid.b, unused_guid.b,
> -		sizeof(unused_guid.b)) == 0) {
> -
> -		log_debug("Found an unused PTE GUID at 0x%08X\n",
> -			  (unsigned int)(uintptr_t)pte);
> -
> -		return 0;
> -	} else {
> -		return 1;
> -	}
> +	return 1;
>   }
>
>   /*