[U-Boot] core: of_addr: Correct the size type of of_get_address to fdt_size_t
diff mbox series

Message ID 20190814102648.23082-1-j-keerthy@ti.com
State New
Delegated to: Simon Glass
Headers show
Series
  • [U-Boot] core: of_addr: Correct the size type of of_get_address to fdt_size_t
Related show

Commit Message

Keerthy Aug. 14, 2019, 10:26 a.m. UTC
Currently the size parameter is defined as u64 type.
Correct the size type of of_get_address to fdt_size_t
so that both 64 bit and 32 bit architectures are taken
care of.

The initial bug report:
https://patchwork.ozlabs.org/patch/1090094/#2212555

Fixes: e679d03b08fb ("core: ofnode: Add ofnode_get_addr_size_index")
Reported-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
Tested-by: Eugeniu Rosca <rosca.eugeniu@gmail.com> 
Signed-off-by: Keerthy <j-keerthy@ti.com>
---

Changes from RFT:

  * Fixed a typo in the commit log.
  * Added Reported-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
	  Tested-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>

 drivers/core/of_addr.c | 4 ++--
 drivers/core/ofnode.c  | 2 +-
 include/dm/of_addr.h   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

Comments

Eugeniu Rosca Aug. 19, 2019, 10:24 a.m. UTC | #1
Hi Keerthy,

On Wed, Aug 14, 2019 at 03:56:48PM +0530, Keerthy wrote:
> Currently the size parameter is defined as u64 type.
> Correct the size type of of_get_address to fdt_size_t
> so that both 64 bit and 32 bit architectures are taken
> care of.
> 
> The initial bug report:
> https://patchwork.ozlabs.org/patch/1090094/#2212555
> 
> Fixes: e679d03b08fb ("core: ofnode: Add ofnode_get_addr_size_index")
> Reported-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
> Tested-by: Eugeniu Rosca <rosca.eugeniu@gmail.com> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> 
> Changes from RFT:
> 
>   * Fixed a typo in the commit log.
>   * Added Reported-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
> 	  Tested-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
> 
>  drivers/core/of_addr.c | 4 ++--
>  drivers/core/ofnode.c  | 2 +-
>  include/dm/of_addr.h   | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c
> index 4e256d9926..812a400b19 100644
> --- a/drivers/core/of_addr.c
> +++ b/drivers/core/of_addr.c
> @@ -122,7 +122,7 @@ static void dev_count_cells(const struct device_node *np, int *nap, int *nsp)
>  }
>  
>  const __be32 *of_get_address(const struct device_node *dev, int index,
> -			     u64 *size, unsigned int *flags)
> +			     fdt_size_t *size, unsigned int *flags)

I took some time to also review the changes in addition to testing.

I can see that, since its inception in Linux [1], of_get_address() used
'u64*' type for its 'size' argument. That's still valid in v5.3-rc5.
So, it looks to me that with this patch we diverge from Linux.

I would barely think that the ASAN issue being fixed in this patch
exists in Linux, since the latter receives much more KASAN-enabled
testing on regular basis.

Do you foresee any alternative fix w/o diverging from Linux?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=22ae782f86b7
Keerthy Aug. 20, 2019, 4:09 a.m. UTC | #2
On 19/08/19 3:54 PM, Eugeniu Rosca wrote:
> Hi Keerthy,
> 
> On Wed, Aug 14, 2019 at 03:56:48PM +0530, Keerthy wrote:
>> Currently the size parameter is defined as u64 type.
>> Correct the size type of of_get_address to fdt_size_t
>> so that both 64 bit and 32 bit architectures are taken
>> care of.
>>
>> The initial bug report:
>> https://patchwork.ozlabs.org/patch/1090094/#2212555
>>
>> Fixes: e679d03b08fb ("core: ofnode: Add ofnode_get_addr_size_index")
>> Reported-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
>> Tested-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>
>> Changes from RFT:
>>
>>    * Fixed a typo in the commit log.
>>    * Added Reported-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
>> 	  Tested-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
>>
>>   drivers/core/of_addr.c | 4 ++--
>>   drivers/core/ofnode.c  | 2 +-
>>   include/dm/of_addr.h   | 2 +-
>>   3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c
>> index 4e256d9926..812a400b19 100644
>> --- a/drivers/core/of_addr.c
>> +++ b/drivers/core/of_addr.c
>> @@ -122,7 +122,7 @@ static void dev_count_cells(const struct device_node *np, int *nap, int *nsp)
>>   }
>>   
>>   const __be32 *of_get_address(const struct device_node *dev, int index,
>> -			     u64 *size, unsigned int *flags)
>> +			     fdt_size_t *size, unsigned int *flags)
> 
> I took some time to also review the changes in addition to testing.
> 
> I can see that, since its inception in Linux [1], of_get_address() used
> 'u64*' type for its 'size' argument. That's still valid in v5.3-rc5.
> So, it looks to me that with this patch we diverge from Linux.
> 
> I would barely think that the ASAN issue being fixed in this patch
> exists in Linux, since the latter receives much more KASAN-enabled
> testing on regular basis.
> 
> Do you foresee any alternative fix w/o diverging from Linux?

I am afraid No but isn't fdt_size_t also right type to represent size?

> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=22ae782f86b7
>

Patch
diff mbox series

diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c
index 4e256d9926..812a400b19 100644
--- a/drivers/core/of_addr.c
+++ b/drivers/core/of_addr.c
@@ -122,7 +122,7 @@  static void dev_count_cells(const struct device_node *np, int *nap, int *nsp)
 }
 
 const __be32 *of_get_address(const struct device_node *dev, int index,
-			     u64 *size, unsigned int *flags)
+			     fdt_size_t *size, unsigned int *flags)
 {
 	const __be32 *prop;
 	int psize;
@@ -347,7 +347,7 @@  int of_address_to_resource(const struct device_node *dev, int index,
 			   struct resource *r)
 {
 	const __be32	*addrp;
-	u64		size;
+	fdt_size_t	size;
 	unsigned int	flags;
 	const char	*name = NULL;
 
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 2ac73af934..b21b5183a3 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -260,7 +260,7 @@  fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)
 		uint flags;
 
 		prop_val = of_get_address(ofnode_to_np(node), index,
-					  (u64 *)size, &flags);
+					  size, &flags);
 		if (!prop_val)
 			return FDT_ADDR_T_NONE;
 
diff --git a/include/dm/of_addr.h b/include/dm/of_addr.h
index 3fa1ffce81..52443ef296 100644
--- a/include/dm/of_addr.h
+++ b/include/dm/of_addr.h
@@ -58,7 +58,7 @@  u64 of_translate_dma_address(const struct device_node *no, const __be32 *in_addr
  * @return pointer to address which can be read
  */
 const __be32 *of_get_address(const struct device_node *no, int index,
-			     u64 *size, unsigned int *flags);
+			     fdt_size_t *size, unsigned int *flags);
 
 struct resource;