[PATCH] dm: core: ofnode: safely fallback in ofnode_lookup_fdt

[Caleb Connolly]

Hi Simon,

On 27/11/2024 17:52, Simon Glass wrote:
> Hi Caleb,
> 
> On Tue, 12 Nov 2024 at 22:21, Caleb Connolly <caleb.connolly at linaro.org> 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 <caleb.connolly at linaro.org>
>> ---
>>  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)