[PATCH] boot: Ensure that ranges is not used uninitialised

[PATCH] boot: Ensure that ranges is not used uninitialised

[Andrew Goodbody]

The local variable ranges may be used uninitialised if the function
fdt_get_dma_range is called with node < 0. Ensure this does not
happen by initialising ranges when declared.

This issue was found with Smatch.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 boot/fdt_support.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index 92f2f534ee0..4f63ce9a763 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -1547,7 +1547,7 @@ int fdt_get_dma_range(const void *blob, int node, phys_addr_t *cpu,
 {
 	bool found_dma_ranges = false;
 	struct of_bus *bus_node;
-	const fdt32_t *ranges;
+	const fdt32_t *ranges = NULL;
 	int na, ns, pna, pns;
 	int parent = node;
 	int ret = 0;

---
base-commit: 359e3012921f2fc2d43f3c4e320a752173f82b82
change-id: 20250627-boot_fdt_fix-e4e56d956c8a

Best regards,
-- 
Andrew Goodbody <andrew.goodbody@linaro.org>

Re: [PATCH] boot: Ensure that ranges is not used uninitialised

[Yao Zi]

On Fri, Jun 27, 2025 at 12:51:02PM +0100, Andrew Goodbody wrote:
> The local variable ranges may be used uninitialised if the function
> fdt_get_dma_range is called with node < 0. Ensure this does not
> happen by initialising ranges when declared.

It doesn't harm. When node < 0, parent is less than zero as well, and
the first while loop is skipped. The control flow just fails into the
error path where parent < 0 stays true.

Furthermore, it doesn't seem valid to call fdt_get_dma_range() with a
negative node parameter, does it?

> /**
>  * fdt_get_dma_range() - get DMA ranges to perform bus/cpu translations
>  *
>  * @blob: pointer to device tree blob
>  * @node_offset: node DT offset
>  * @cpu: pointer to variable storing the range's cpu address
>  * @bus: pointer to variable storing the range's bus address
>  * @size: pointer to variable storing the range's size
>  *
>  * Get DMA ranges for a specifc node, this is useful to perform bus->cpu and
>  * cpu->bus address translations
>  *
>  * Return: translated DMA address or OF_BAD_ADDR on error
>  */
> int fdt_get_dma_range(const void *blob, int node_offset, phys_addr_t *cpu,
>                       dma_addr_t *bus, u64 *size);

Regards,
Yao Zi

> This issue was found with Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> ---
>  boot/fdt_support.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/boot/fdt_support.c b/boot/fdt_support.c
> index 92f2f534ee0..4f63ce9a763 100644
> --- a/boot/fdt_support.c
> +++ b/boot/fdt_support.c
> @@ -1547,7 +1547,7 @@ int fdt_get_dma_range(const void *blob, int node, phys_addr_t *cpu,
>  {
>  	bool found_dma_ranges = false;
>  	struct of_bus *bus_node;
> -	const fdt32_t *ranges;
> +	const fdt32_t *ranges = NULL;
>  	int na, ns, pna, pns;
>  	int parent = node;
>  	int ret = 0;
> 
> ---
> base-commit: 359e3012921f2fc2d43f3c4e320a752173f82b82
> change-id: 20250627-boot_fdt_fix-e4e56d956c8a
> 
> Best regards,
> -- 
> Andrew Goodbody <andrew.goodbody@linaro.org>
> 

Re: [PATCH] boot: Ensure that ranges is not used uninitialised

[Andrew Goodbody]

On 27/06/2025 13:23, Yao Zi wrote:
> On Fri, Jun 27, 2025 at 12:51:02PM +0100, Andrew Goodbody wrote:
>> The local variable ranges may be used uninitialised if the function
>> fdt_get_dma_range is called with node < 0. Ensure this does not
>> happen by initialising ranges when declared.
> 
> It doesn't harm. When node < 0, parent is less than zero as well, and
> the first while loop is skipped. The control flow just fails into the
> error path where parent < 0 stays true.

OK, I will withdraw the patch. It just does not seem a great idea to 
have an uninitialised variable evaluated even if the result of that 
evaluation is irrelevant due to the second term in the logical OR being 
true.

> Furthermore, it doesn't seem valid to call fdt_get_dma_range() with a
> negative node parameter, does it?

No it does not, but that is a different issue. Just because it is not 
valid does not mean it will never happen.

Andrew

>> /**
>>   * fdt_get_dma_range() - get DMA ranges to perform bus/cpu translations
>>   *
>>   * @blob: pointer to device tree blob
>>   * @node_offset: node DT offset
>>   * @cpu: pointer to variable storing the range's cpu address
>>   * @bus: pointer to variable storing the range's bus address
>>   * @size: pointer to variable storing the range's size
>>   *
>>   * Get DMA ranges for a specifc node, this is useful to perform bus->cpu and
>>   * cpu->bus address translations
>>   *
>>   * Return: translated DMA address or OF_BAD_ADDR on error
>>   */
>> int fdt_get_dma_range(const void *blob, int node_offset, phys_addr_t *cpu,
>>                        dma_addr_t *bus, u64 *size);
> 
> Regards,
> Yao Zi
> 
>> This issue was found with Smatch.
>>
>> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
>> ---
>>   boot/fdt_support.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/boot/fdt_support.c b/boot/fdt_support.c
>> index 92f2f534ee0..4f63ce9a763 100644
>> --- a/boot/fdt_support.c
>> +++ b/boot/fdt_support.c
>> @@ -1547,7 +1547,7 @@ int fdt_get_dma_range(const void *blob, int node, phys_addr_t *cpu,
>>   {
>>   	bool found_dma_ranges = false;
>>   	struct of_bus *bus_node;
>> -	const fdt32_t *ranges;
>> +	const fdt32_t *ranges = NULL;
>>   	int na, ns, pna, pns;
>>   	int parent = node;
>>   	int ret = 0;
>>
>> ---
>> base-commit: 359e3012921f2fc2d43f3c4e320a752173f82b82
>> change-id: 20250627-boot_fdt_fix-e4e56d956c8a
>>
>> Best regards,
>> -- 
>> Andrew Goodbody <andrew.goodbody@linaro.org>
>>