[PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t

[Marek Vasut]

On 3/17/24 12:18 PM, Laurent Pinchart wrote:
> Hi Marek,
> 
> Thank you for the patch.
> 
> On Sun, Mar 17, 2024 at 07:16:29AM +0100, Marek Vasut wrote:
>> Change type of ulong env_get_bootm_low() to phys_addr_t env_get_bootm_low().
>> The PPC/LS systems already treat env_get_bootm_low() result as phys_addr_t,
>> while the function itself still returns ulong. This is potentially dangerous
>> on 64bit systems, where ulong might not be large enough to hold the content
>> of "bootm_low" environment variable. Fix it by using phys_addr_t, similar to
>> what env_get_bootm_size() does, which returns phys_size_t .
>>
>> Reported-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
>> ---
>> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> Cc: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
>> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Tom Rini <trini at konsulko.com>
>> ---
>>   boot/bootm.c       |  2 +-
>>   boot/image-board.c | 11 ++++++-----
>>   boot/image-fdt.c   |  9 +++++----
>>   include/image.h    |  2 +-
>>   4 files changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/boot/bootm.c b/boot/bootm.c
>> index d071537d692..2e818264f5f 100644
>> --- a/boot/bootm.c
>> +++ b/boot/bootm.c
>> @@ -242,7 +242,7 @@ static int boot_get_kernel(const char *addr_fit, struct bootm_headers *images,
>>   #ifdef CONFIG_LMB
>>   static void boot_start_lmb(struct bootm_headers *images)
>>   {
>> -	ulong		mem_start;
>> +	phys_addr_t	mem_start;
>>   	phys_size_t	mem_size;
>>   
>>   	mem_start = env_get_bootm_low();
>> diff --git a/boot/image-board.c b/boot/image-board.c
>> index 75f6906cd56..447ced2678a 100644
>> --- a/boot/image-board.c
>> +++ b/boot/image-board.c
>> @@ -107,12 +107,13 @@ static int on_loadaddr(const char *name, const char *value, enum env_op op,
>>   }
>>   U_BOOT_ENV_CALLBACK(loadaddr, on_loadaddr);
>>   
>> -ulong env_get_bootm_low(void)
>> +phys_addr_t env_get_bootm_low(void)
>>   {
>>   	char *s = env_get("bootm_low");
>> +	phys_size_t tmp;
>>   
>>   	if (s) {
>> -		ulong tmp = hextoul(s, NULL);
>> +		tmp = (phys_addr_t)simple_strtoull(s, NULL, 16);
>>   		return tmp;
> 
> Can't you write
> 
> 		return (phys_addr_t)simple_strtoull(s, NULL, 16);
> 
> and avoid the temporary variable ?

Fixed in V2, thanks