[PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info

[Anshul Dalal]

Hi Rasmus,

On Tue Nov 11, 2025 at 2:24 AM IST, Rasmus Villemoes wrote:
> Not all ->get_info implementations necessarily populate all the string
> members of struct disk_partition.
>
> Currently, only part_get_info_by_type() (and thereby part_get_info)
> ensure that the uuid strings are initialized; part_get_info_by_type()
> and part_get_info_by_uuid() do not. In fact, the latter could lead to
> a false positive match - if the ->get_info backend does not populate
> info->uuid, stale contents in info could cause the strncasecmp() to
> succeed.
>
> None of the functions currently ensure that the ->name and ->type
> strings are initialized.
>
> Instead of forcing all callers of any of these functions to
> pre-initialize info, or all implementations of the ->get_info method
> to ensure there are valid C strings in all four fields, create a small
> helper function and factor all invocations of ->get_info through that.
>
> This also consolidates the -ENOSYS check and standardizes on using
> log_debug() for reporting absence, instead of the current mix of
> PRINTF and log_debug(). It does mean we have to special-case -ENOSYS
> in the error cases inside the loops in the _by_uuid() and _by_name()
> functions, but it's still a net win in #LOC.
>
> Acked-by: Quentin Schulz <quentin.schulz at cherry.de>
> Signed-off-by: Rasmus Villemoes <ravi at prevas.dk>
> ---
>  cmd/gpt.c      |  4 ++--
>  disk/part.c    | 63 +++++++++++++++++++++++++++++---------------------
>  include/part.h | 16 +++++++++++++
>  3 files changed, 55 insertions(+), 28 deletions(-)
>
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index e18e5036a06..84221881c39 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -724,7 +724,7 @@ static int gpt_enumerate(struct blk_desc *desc)
>  			continue;
>  
>  		for (i = 1; i < part_drv->max_entries; i++) {
> -			ret = part_drv->get_info(desc, i, &pinfo);
> +			ret = part_driver_get_info(part_drv, desc, i, &pinfo);
>  			if (ret)
>  				continue;
>  
> @@ -820,7 +820,7 @@ static int gpt_setenv(struct blk_desc *desc, const char *name)
>  		int i;
>  
>  		for (i = 1; i < part_drv->max_entries; i++) {
> -			ret = part_drv->get_info(desc, i, &pinfo);
> +			ret = part_driver_get_info(part_drv, desc, i, &pinfo);
>  			if (ret)
>  				continue;
>  
> diff --git a/disk/part.c b/disk/part.c
> index be2b45d5a29..49a0fca6b89 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -72,6 +72,28 @@ struct part_driver *part_driver_lookup_type(struct blk_desc *desc)
>  	return NULL;
>  }
>  
> +static void disk_partition_clr(struct disk_partition *info)
> +{
> +	/* The common case is no UUID support */
> +	disk_partition_clr_uuid(info);
> +	disk_partition_clr_type_guid(info);
> +	info->name[0] = '\0';
> +	info->type[0] = '\0';
> +}
> +
> +int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part,
> +			 struct disk_partition *info)
> +{
> +	if (!drv->get_info) {
> +		log_debug("## Driver %s does not have the get_info() method\n",
> +			  drv->name);
> +		return -ENOSYS;
> +	}
> +
> +	disk_partition_clr(info);
> +	return drv->get_info(desc, part, info);
> +}
> +
>  int part_get_type_by_name(const char *name)
>  {
>  	struct part_driver *drv =
> @@ -322,12 +344,9 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
>  			  struct disk_partition *info)
>  {
>  	struct part_driver *drv;
> +	int ret = -ENOENT;
>  
>  	if (blk_enabled()) {
> -		/* The common case is no UUID support */
> -		disk_partition_clr_uuid(info);
> -		disk_partition_clr_type_guid(info);
> -
>  		if (part_type == PART_TYPE_UNKNOWN) {
>  			drv = part_driver_lookup_type(desc);
>  		} else {
> @@ -339,18 +358,16 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
>  			      desc->part_type);
>  			return -EPROTONOSUPPORT;
>  		}
> -		if (!drv->get_info) {
> -			PRINTF("## Driver %s does not have the get_info() method\n",
> -			       drv->name);
> -			return -ENOSYS;
> -		}
> -		if (drv->get_info(desc, part, info) == 0) {
> +
> +		ret = part_driver_get_info(drv, desc, part, info);
> +		if (ret && ret != -ENOSYS) {
> +			ret = -ENOENT;

Why are we overwriting the err code from part_driver_get_info here?

Regards,
Anshul

> +		} else {
>  			PRINTF("## Valid %s partition found ##\n", drv->name);
> -			return 0;
>  		}
>  	}
>  
> -	return -ENOENT;
> +	return ret;
>  }
>  
>  int part_get_info(struct blk_desc *desc, int part,
> @@ -657,15 +674,12 @@ int part_get_info_by_name(struct blk_desc *desc, const char *name,
>  	if (!part_drv)
>  		return -1;
>  
> -	if (!part_drv->get_info) {
> -		log_debug("## Driver %s does not have the get_info() method\n",
> -			  part_drv->name);
> -		return -ENOSYS;
> -	}
> -
>  	for (i = 1; i < part_drv->max_entries; i++) {
> -		ret = part_drv->get_info(desc, i, info);
> +		ret = part_driver_get_info(part_drv, desc, i, info);
>  		if (ret != 0) {
> +			/* -ENOSYS means no ->get_info method. */
> +			if (ret == -ENOSYS)
> +				return ret;
>  			/*
>  			 * Partition with this index can't be obtained, but
>  			 * further partitions might be, so keep checking.
> @@ -695,15 +709,12 @@ int part_get_info_by_uuid(struct blk_desc *desc, const char *uuid,
>  	if (!part_drv)
>  		return -1;
>  
> -	if (!part_drv->get_info) {
> -		log_debug("## Driver %s does not have the get_info() method\n",
> -			  part_drv->name);
> -		return -ENOSYS;
> -	}
> -
>  	for (i = 1; i < part_drv->max_entries; i++) {
> -		ret = part_drv->get_info(desc, i, info);
> +		ret = part_driver_get_info(part_drv, desc, i, info);
>  		if (ret != 0) {
> +			/* -ENOSYS means no ->get_info method. */
> +			if (ret == -ENOSYS)
> +				return ret;
>  			/*
>  			 * Partition with this index can't be obtained, but
>  			 * further partitions might be, so keep checking.
> diff --git a/include/part.h b/include/part.h
> index 6caaa6526aa..d940f8b5d0e 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -509,6 +509,22 @@ struct part_driver {
>  	int (*test)(struct blk_desc *desc);
>  };
>  
> +/**
> + * part_driver_get_info() - Call the part_driver's get_info method
> + *
> + * On success, string members of info are guaranteed to be properly
> + * initialized, though they may be empty.
> + *
> + * @drv:	Partition driver
> + * @desc:	Block device descriptor
> + * @part:	Partition number to read
> + * @info:	Returned partition information
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part,
> +			 struct disk_partition *info);
> +
>  /* Declare a new U-Boot partition 'driver' */
>  #define U_BOOT_PART_TYPE(__name)					\
>  	ll_entry_declare(struct part_driver, __name, part_driver)