Re: [PATCH v2] of/irq: Make sure to update out_irq->np to the new parent in of_irq_parse_raw

[Rob Herring]

On Mon, Jul 29, 2024 at 11:54 PM WHR <whr@xxxxxxxxxxx> wrote:
>
> Commit 935df1bd40d43c4ee91838c42a20e9af751885cc has removed an
> assignment statement for 'out_irq->np' right after label 'skiplevel',
> causing the new parent acquired from function of_irq_find_parent didn't
> being stored to 'out_irq->np' as it supposed to. Under some conditions
> this can resuit in multiple corruptions and leakages to device nodes.

Under what conditions? Please provide a specific platform and DT.

>
> Update 'out_irq->np' before jumping to label 'skiplevel'.
>
> Fixes: 935df1bd40d4 ("of/irq: Factor out parsing of interrupt-map parent
> phandle+args from of_irq_parse_raw()")
> Signed-off-by: WHR <whr@xxxxxxxxxxx>

Need a name here.

> ---
> v2:
> Add 'Fixes:' line, and update subject.
>
>  drivers/of/irq.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index c94203ce65bb..580b33ce60d2 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -263,6 +263,7 @@ int of_irq_parse_raw(const __be32 *addr, struct
> of_phandle_args *out_irq)
>                 if (imap == NULL) {
>                         pr_debug(" -> no map, getting parent\n");
>                         newpar = of_irq_find_parent(ipar);
> +                       out_irq->np = newpar;

Honestly, I think the DT is wrong if you get to this point. We'd have
to have the initial interrupt parent with #interrupt-cells, but not an
interrupt-controller nor interrupt-map property to get here. Maybe
that happens in some ancient platform, but if so, I want to know which
one and what exactly we need to handle.

Rob