[PATCH 5/7] efi_loader: PARTITION_UUIDS should be optional

[Heinrich Schuchardt]

On 4/19/22 03:01, AKASHI Takahiro wrote:
> In the current implementation, partition table support (either GPT or DOS)
> is not mandatory. So CONFIG_PARTITION_UUIDS should not be enabled
> (selected) unconditionally.
>
> Fixes: commit 17f8cda505e3 ("efi_loader: set partition GUID in device path for SIG_TYPE_GUID")
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>   lib/efi_loader/Kconfig           |  2 +-
>   lib/efi_loader/efi_device_path.c | 11 ++++++++++-
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index d50cd2563d3d..bc518d7a413b 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -15,7 +15,7 @@ config EFI_LOADER
>   	depends on !EFI_APP
>   	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
>   	select LIB_UUID
> -	select PARTITION_UUIDS
> +	imply PARTITION_UUIDS
>   	select HAVE_BLOCK_DEVICE
>   	select REGEX
>   	imply FAT
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 0542aaae16c7..01fb53ae8c40 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -864,11 +864,20 @@ static void *dp_part_node(void *buf, struct blk_desc *desc, int part)
>   			break;
>   		case SIG_TYPE_GUID:

This can only be reached for CONFIG_EFI_PARTITION=y.

>   			hddp->signature_type = 2;
> +#if CONFIG_IS_ENABLED(PARTITION_UUIDS)
> +			/* info.uuid exists only with PARTITION_UUIDS */
>   			if (uuid_str_to_bin(info.uuid,
> -					    hddp->partition_signature, 1))
> +					    hddp->partition_signature, 1)) {

uuid_guid_get_bin() does not write any byte if info.uuid is not valid.

>   				log_warning(
>   					"Partition no. %d: invalid guid: %s\n",
>   					part, info.uuid);

> +				memset(hddp->partition_signature, 0,
> +				       sizeof(hddp->partition_signature));

dp_alloc() already has zeroed out the memory. Please, remove the
superfluous code.

> +			}
> +#else
> +			memset(hddp->partition_signature, 0,
> +			       sizeof(hddp->partition_signature));

ditto.


This does not conform to the UEFI specification. Why should we support it?

Shouldn't we select CONFIG_PARTITION_UUIDS if CONFIG_EFI_LOADER=y and
CONFIG_EFI_PARTITION=y.

Best regards

Heinrich

> +#endif
>   			break;
>   		}
>