Hi Quentin,
On 1/14/26 16:41, Quentin Schulz wrote:
> 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 <[email protected]>
>> ---
>> 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.
There is no upper limit in AMIGA partition layout. U-Boot
uses AMIGA_ENTRY_NUMBERS just to restrict the number of available entries.
I study this and other partition drivers and fix all found issues (see
my next patch series)
>
> Finally, I'm pretty sure we may need something similar in cmd/gpt.c
> since the same check is made there?
Good catch! Fixed in my next patch series.
>
> Cheers,
> Quentin
Regards,
Mikhail Kshevetskiy
