Hi Samuel,
On Thu, 17 Nov 2022 at 21:00, Samuel Holland samuel@sholland.org wrote:
Hi Simon,
On 11/7/22 17:35, Simon Glass wrote:
Hi Samuel,
On Mon, 31 Oct 2022 at 13:27, Simon Glass sjg@chromium.org wrote:
On Sun, 30 Oct 2022 at 21:41, Samuel Holland samuel@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@sholland.org
drivers/core/fdtaddr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@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
We should put the dm tag in the device tree node instead.
};
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);
That is slow, best avoided.
fdt_addr_t addr; if (CONFIG_IS_ENABLED(OF_TRANSLATE)) {
Regards, Simon