[PATCH] boot: image-board: Mismatch a type between variable and return value

[PATCH] boot: image-board: Mismatch a type between variable and return value

[Jaehoon Chung]

phys_addr_t can be used unsigned long long type with CONFIG_PHYS_64BIT.
But hextoul() is always returning to unsigned long. It can be assigned
to unexpected value. To avoid wrong behavior, change from hextoul() to
simple_strtoull().

Fixes: a4df06e41fa2 ("boot: fdt: Change type of env_get_bootm_low() to phys_addr_t")

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 boot/image-board.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index 1757e5816d84..977343a8995c 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -547,7 +547,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start,
 		/* a value of "no" or a similar string will act like 0,
 		 * turning the "load high" feature off. This is intentional.
 		 */
-		initrd_high = hextoul(s, NULL);
+		initrd_high = simple_strtoull(s, NULL, 16);
 		if (initrd_high == ~0)
 			initrd_copy_to_ram = 0;
 	} else {
-- 
2.25.1

Re: [PATCH] boot: image-board: Mismatch a type between variable and return value

[Marek Vasut]

On 11/5/24 11:11 PM, Jaehoon Chung wrote:
> phys_addr_t can be used unsigned long long type with CONFIG_PHYS_64BIT.
> But hextoul() is always returning to unsigned long. It can be assigned
> to unexpected value. To avoid wrong behavior, change from hextoul() to
> simple_strtoull().
> 
> Fixes: a4df06e41fa2 ("boot: fdt: Change type of env_get_bootm_low() to phys_addr_t")

Looking at:

7e5f460ec457 ("global: Convert simple_strtoul() with hex to hextoul()")

do you have to update any of the other hextoul() calls too ?

Maybe we need hextoull() ?

> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>   boot/image-board.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/boot/image-board.c b/boot/image-board.c
> index 1757e5816d84..977343a8995c 100644
> --- a/boot/image-board.c
> +++ b/boot/image-board.c
> @@ -547,7 +547,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start,
>   		/* a value of "no" or a similar string will act like 0,
>   		 * turning the "load high" feature off. This is intentional.
>   		 */
> -		initrd_high = hextoul(s, NULL);
> +		initrd_high = simple_strtoull(s, NULL, 16);
>   		if (initrd_high == ~0)
>   			initrd_copy_to_ram = 0;
>   	} else {

RE: [PATCH] boot: image-board: Mismatch a type between variable and return value

[Jaehoon Chung]

> -----Original Message-----
> From: Marek Vasut <marek.vasut@mailbox.org>
> Sent: Wednesday, November 6, 2024 7:57 AM
> To: Jaehoon Chung <jh80.chung@samsung.com>; u-boot@lists.denx.de
> Cc: trini@konsulko.com; sjg@chromium.org; marek.vasut+renesas@mailbox.org;
> laurent.pinchart@ideasonboard.com; m.szyprowski@samsung.com
> Subject: Re: [PATCH] boot: image-board: Mismatch a type between variable and return value
>
> On 11/5/24 11:11 PM, Jaehoon Chung wrote:
> > phys_addr_t can be used unsigned long long type with CONFIG_PHYS_64BIT.
> > But hextoul() is always returning to unsigned long. It can be assigned
> > to unexpected value. To avoid wrong behavior, change from hextoul() to
> > simple_strtoull().
> >
> > Fixes: a4df06e41fa2 ("boot: fdt: Change type of env_get_bootm_low() to phys_addr_t")
>
> Looking at:
>
> 7e5f460ec457 ("global: Convert simple_strtoul() with hex to hextoul()")

Thanks for informing it.

>
> do you have to update any of the other hextoul() calls too ?

I didn't check other hextoul().
During booting with initramfs on RPi (32bit), I found the problem that is not working fine because of this.

>
> Maybe we need hextoull() ?

IMO, We need hextoull() for some cases.
If there is no objection, I will send the patch for adding hextoull().
And I will check entire cases.

Best Regards,
Jaehoon Chung

>
> > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> > ---
> >   boot/image-board.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/boot/image-board.c b/boot/image-board.c
> > index 1757e5816d84..977343a8995c 100644
> > --- a/boot/image-board.c
> > +++ b/boot/image-board.c
> > @@ -547,7 +547,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start,
> >   		/* a value of "no" or a similar string will act like 0,
> >   		 * turning the "load high" feature off. This is intentional.
> >   		 */
> > -		initrd_high = hextoul(s, NULL);
> > +		initrd_high = simple_strtoull(s, NULL, 16);
> >   		if (initrd_high == ~0)
> >   			initrd_copy_to_ram = 0;
> >   	} else {
>

Re: [PATCH] boot: image-board: Mismatch a type between variable and return value

[Marek Vasut]

On 11/6/24 12:47 AM, Jaehoon Chung wrote:
> 
>> -----Original Message-----
>> From: Marek Vasut <marek.vasut@mailbox.org>
>> Sent: Wednesday, November 6, 2024 7:57 AM
>> To: Jaehoon Chung <jh80.chung@samsung.com>; u-boot@lists.denx.de
>> Cc: trini@konsulko.com; sjg@chromium.org; marek.vasut+renesas@mailbox.org;
>> laurent.pinchart@ideasonboard.com; m.szyprowski@samsung.com
>> Subject: Re: [PATCH] boot: image-board: Mismatch a type between variable and return value
>>
>> On 11/5/24 11:11 PM, Jaehoon Chung wrote:
>>> phys_addr_t can be used unsigned long long type with CONFIG_PHYS_64BIT.
>>> But hextoul() is always returning to unsigned long. It can be assigned
>>> to unexpected value. To avoid wrong behavior, change from hextoul() to
>>> simple_strtoull().
>>>
>>> Fixes: a4df06e41fa2 ("boot: fdt: Change type of env_get_bootm_low() to phys_addr_t")
>>
>> Looking at:
>>
>> 7e5f460ec457 ("global: Convert simple_strtoul() with hex to hextoul()")
> 
> Thanks for informing it.
> 
>>
>> do you have to update any of the other hextoul() calls too ?
> 
> I didn't check other hextoul().
> During booting with initramfs on RPi (32bit), I found the problem that is not working fine because of this.

Please check the other sites too, maybe there are more of these issues 
lurking around.

>> Maybe we need hextoull() ?
> 
> IMO, We need hextoull() for some cases.
> If there is no objection, I will send the patch for adding hextoull().
> And I will check entire cases.
No objection from me. Thank you.