Re: [for-4.18][PATCH] xen/arm: Check return code from recursive calls to scan_pfdt_node()

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Michal,

> On Oct 16, 2023, at 21:28, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> wrote:
> 
> Hi Michal,
> 
>> On 16 Oct 2023, at 14:45, Michal Orzel <michal.orzel@xxxxxxx> wrote:
>> 
>> At the moment, we do not check a return code from scan_pfdt_node()
>> called recursively. This means that any issue that may occur while
>> parsing and copying the passthrough nodes is hidden and Xen continues
>> to boot a domain despite errors. This may lead to incorrect device tree
>> generation and various guest issues (e.g. trap on attempt to access MMIO
>> not mapped in P2M). Fix it.
>> 
>> Fixes: 669ecdf8d6cd ("xen/arm: copy dtb fragment to guest dtb")
>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> 
> Good finding :-)
> 
> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Release-acked-by: Henry Wang <Henry.Wang@xxxxxxx>

Kind regards,
Henry

> 
> Cheers
> Bertrand
> 
>> ---
>> @Henry:
>> This is a bug fix, so I think we should have it in 4.18 given the possible
>> consequences I described in the commit msg. I don't see any risks as this 
>> change
>> only checks the return code for an error.
>> ---
>> xen/arch/arm/domain_build.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 24c9019cc43c..49792dd590ee 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2872,8 +2872,11 @@ static int __init scan_pfdt_node(struct kernel_info 
>> *kinfo, const void *pfdt,
>>    node_next = fdt_first_subnode(pfdt, nodeoff);
>>    while ( node_next > 0 )
>>    {
>> -        scan_pfdt_node(kinfo, pfdt, node_next, address_cells, size_cells,
>> -                       scan_passthrough_prop);
>> +        rc = scan_pfdt_node(kinfo, pfdt, node_next, address_cells, 
>> size_cells,
>> +                            scan_passthrough_prop);
>> +        if ( rc )
>> +            return rc;
>> +
>>        node_next = fdt_next_subnode(pfdt, node_next);
>>    }
>> 
>> --
>> 2.25.1
>> 
>