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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqKpTKv-fthwD9bFHiVESJyNP6uMg7Px7Rh+-k583oz76g@mail.gmail.com>
Date: Wed, 31 Jul 2024 15:58:38 -0600
From: Rob Herring <robh@...nel.org>
To: WHR <whr@...oreo.one>
Cc: Saravana Kannan <saravanak@...gle.com>, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] of/irq: Make sure to update out_irq->np to the new
 parent in of_irq_parse_raw

On Mon, Jul 29, 2024 at 11:54 PM WHR <whr@...oreo.one> 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@...oreo.one>

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