Hi Simon, On 27/11/2024 17:52, Simon Glass wrote: > Hi Caleb, > > On Tue, 12 Nov 2024 at 22:21, Caleb Connolly <[email protected]> > wrote: >> >> Under some conditions it's possible to hit the null condition here like >> when running with OF_LIVE and using the ofnode API before >> initr_of_live() is called. There is very little null >> checking for this in the FDT framework, so returning null here can >> result in weird null pointer conditions. >> >> Instead let's return the control FDT in the fallback case, this is >> usually what the user was expecting. >> >> Signed-off-by: Caleb Connolly <[email protected]> >> --- >> drivers/core/ofnode.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c >> index 950895e72a99..7a7f25fc537c 100644 >> --- a/drivers/core/ofnode.c >> +++ b/drivers/core/ofnode.c >> @@ -152,9 +152,9 @@ void *ofnode_lookup_fdt(ofnode node) >> uint i = OFTREE_TREE_ID(node.of_offset); >> >> if (i >= oftree_count) { >> log_debug("Invalid tree ID %x\n", i); >> - return NULL; >> + return (void *)gd->fdt_blob; >> } >> >> return oftree_list[i]; >> } else { >> -- >> 2.47.0 >> > > Eek I really don't like that, since it will silently return an > unexpected value. What's the unexpected value? The only caller for this is ofnode_to_fdt() and the return value for that is never checked. It seems clear to me that in practise NULL is the unexpected value here.
> I think we should panic. Do you know what code path > gets you here? Hmm, I don't exactly recall. Should really have made a note when I wrote this patch :/ Guess I'll drop this from my dev branch and come back when I figure this out again. > > More work is needed in livetree to sort of the rough edges, sadly... > > Regards, > Simon -- // Caleb (they/them)
