[PATCH] disk: part: fix partition search boundaries

[Quentin Schulz]

Hi Mikhail,

On 1/6/26 6:14 AM, Mikhail Kshevetskiy wrote:
> GPT disk partition with max available number (ex: /dev/mmcblk128) can't
> be read/write from U-Boot using read/write command. Here is an example:
> 
>    => mmc part
> 
>    Partition Map for mmc device 0  --   Partition Type: EFI
> 
>    Part	Start LBA	End LBA		Name
> 	Attributes
> 	Type GUID
> 	Partition GUID
>    1	0x00001000	0x000013ff	"env1"
> 	attrs:	0x0000000000000000
> 	type:	0fc63daf-8483-4772-8e79-3d69d8477de4
> 	guid:	5452574f-2211-4433-5566-778899aabb02
>    2	0x00001400	0x000017ff	"env2"
> 	attrs:	0x0000000000000000
> 	type:	0fc63daf-8483-4772-8e79-3d69d8477de4
> 	guid:	5452574f-2211-4433-5566-778899aabb03
>    .................
>    8	0x00158000	0x0034bfff	"apps"
> 	attrs:	0x0000000000000000
> 	type:	0fc63daf-8483-4772-8e79-3d69d8477de4
> 	guid:	5452574f-2211-4433-5566-778899aabb09
>    128	0x00000420	0x00000fff	"fip"
> 	attrs:	0x0000000000000000
> 	type:	c12a7328-f81f-11d2-ba4b-00a0c93ec93b
> 	guid:	5452574f-2211-4433-5566-778899aabb01
> 
>    => read mmc 0#fip ${loadaddr} 0 4
>    Could not find "fip" partition
>    ** Bad device specification mmc 0#fip **
>    ** Bad device specification mmc 0#fip **
>    Couldn't find partition mmc 0#fip
> 
> The error is caused by invalid boundary checks. This patch fixes an
> issue.
> 
> Fixes: 43fd4bcefd4e ("disk: part: implement generic function part_get_info_by_uuid()")
> Fixes: 56670d6fb83f ("disk: part: use common api to lookup part driver")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu>
> ---
>   disk/part.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/disk/part.c b/disk/part.c
> index 49a0fca6b89..4923dc44593 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -674,7 +674,7 @@ int part_get_info_by_name(struct blk_desc *desc, const char *name,
>   	if (!part_drv)
>   		return -1;
>   
> -	for (i = 1; i < part_drv->max_entries; i++) {
> +	for (i = 1; i <= part_drv->max_entries; i++) {
>   		ret = part_driver_get_info(part_drv, desc, i, info);
>   		if (ret != 0) {
>   			/* -ENOSYS means no ->get_info method. */
> @@ -709,7 +709,7 @@ int part_get_info_by_uuid(struct blk_desc *desc, const char *uuid,
>   	if (!part_drv)
>   		return -1;
>   
> -	for (i = 1; i < part_drv->max_entries; i++) {
> +	for (i = 1; i <= part_drv->max_entries; i++) {

This *seems* to be fine but I'm wary of the change.

Are we sure **all** partition drivers understood this the right way and 
did store the maximum number of entries in that variable? This could 
open the door to out-of-band access/buffer overflow if we have some 
arrays of size max_entries for example (thus accessible 
array[max_entries] is an out-of-band access for example).

For example the AMIGA partition driver seems to be saying there can only 
be 8 partitions, while 
https://amitools.readthedocs.io/en/latest/tools/rdbtool.html#info-show-information-of-the-rdb-data-structures 
shows 9 partitions, starting at 0 instead of 1 even. We have max_entries 
set to AMIGA_ENTRY_NUMBERS (8) so I guess 1-7 is currently covered, but 
this would then cover 1-8 which hopefully is still fine (we would still 
be missing partition 0 if that one is actually possible).

So we need careful consideration on the impact of this patch across all 
types of partitions.

Finally, I'm pretty sure we may need something similar in cmd/gpt.c 
since the same check is made there?

Cheers,
Quentin