[PATCH] fdt: Fix bounds check in devfdt_get_addr_index

[Samuel Holland]

Hi Simon,

On 11/7/22 17:35, Simon Glass wrote:
> Hi Samuel,
> 
> On Mon, 31 Oct 2022 at 13:27, Simon Glass <sjg at chromium.org> wrote:
>>
>> On Sun, 30 Oct 2022 at 21:41, Samuel Holland <samuel at sholland.org> wrote:
>>>
>>> reg must contain enough cells for the entire next address/size pair
>>> after skipping `index` pairs. The previous code allows an out-of-bounds
>>> read when na + ns > 1.
>>>
>>> Fixes: 69b41388ba45 ("dm: core: Add a new api to get indexed device address")
>>> Signed-off-by: Samuel Holland <samuel at sholland.org>
>>> ---
>>>
>>>  drivers/core/fdtaddr.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> Somehow this break PPC in CI:
> 
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/524776
> 
> Can you please take a look?

The cause is the call to lists_bind_fdt() inside serial_check_stdout(),
which sets the UART's parent device to gd->dm_root, when the real parent
should be a simple-bus node with a ranges property.

Then when we go to probe the UART, devfdt_get_addr_index() does:

	int parent = dev_of_offset(dev->parent);
	// ...
	na = fdt_address_cells(gd->fdt_blob, parent);

So devfdt_get_addr_index() sees the wrong number of address/size cells
(2 instead of 1) and the check fails. This only worked previously
because the check in devfdt_get_addr_index() would always succeed for
index == 0.

This patch fixes things, by setting the UART's parent device correctly:

--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -623,9 +623,7 @@
 	.priv_auto	= sizeof(struct ns16550),
 	.probe = ns16550_serial_probe,
 	.ops	= &ns16550_serial_ops,
-#if !CONFIG_IS_ENABLED(OF_CONTROL)
 	.flags	= DM_FLAG_PRE_RELOC,
-#endif
 };

 DM_DRIVER_ALIAS(ns16550_serial, ti_da830_uart)

Or maybe devfdt_get_addr_index() should be looking at the FDT node
parent, instead of the DM parent. This also fixes things:

--- a/drivers/core/fdtaddr.c
+++ b/drivers/core/fdtaddr.c
@@ -22,7 +22,7 @@
 {
 #if CONFIG_IS_ENABLED(OF_REAL)
 	int offset = dev_of_offset(dev);
-	int parent = dev_of_offset(dev->parent);
+	int parent = fdt_parent_offset(gd->fdt_blob, offset);
 	fdt_addr_t addr;

 	if (CONFIG_IS_ENABLED(OF_TRANSLATE)) {

Regards,
Samuel