linux-kernel - Re: [PATCH] genirq/irqdomain: Don't call ops->select for DOMAIN_BUS

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]

Date: Sun, 25 Feb 2024 17:23:22 +0000
From: Marc Zyngier <maz@...nel.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Biju Das
 <biju.das.jz@...renesas.com>, x86@...nel.org
Subject: Re: [PATCH] genirq/irqdomain: Don't call ops->select for
 DOMAIN_BUS_ANY tokens

On 2024-02-25 16:19, Thomas Gleixner wrote:
> On Tue, Feb 20 2024 at 11:47, Marc Zyngier wrote:
>> Users of the IRQCHIP_PLATFORM_DRIVER_{BEGIN,END} helpers rely
>> on a fwspec containing only the fwnode (and crucially a number
>> of parameters set to 0) together with a DOMAIN_BUS_ANY token
>> to check whether a parent irqchip has probed and registered
>> a domain.
>> 
>> Since de1ff306dcf4 ("genirq/irqdomain: Remove the param count
>> restriction from select()"), we call ops->select unconditionally,
>> meaning that irqchips implementing select now need to handle
>> ANY as a match.
>> 
>> Instead of adding more esoteric checks to the individual drivers,
>> add that condition to irq_find_matching_fwspec(), and let it
>> handle the corner case, as per the comment in the function.
>> 
>> This restores the functionnality of the above helpers.
>> 
>> Reported-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>> Reported-by: Biju Das <biju.das.jz@...renesas.com>
>> Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count 
>> restriction from select()")
>> Signed-off-by: Marc Zyngier <maz@...nel.org>
>> Link: 
>> https://lore.kernel.org/r/20240219-gic-fix-child-domain-v1-1-09f8fd2d9a8f@linaro.org
> 
> Bah. That breaks x86 because it uses DOMAIN_BUS_ANY to find the MSI
> parent for a fwspec (IOAPIC and HPET) which gets either picked up by 
> the
> interrupt remapping or by the root vector domain.
> 
> Fix below.
> 
> Thanks,
> 
>         tglx
> ---
> Subject: x86/apic/msi: Use DOMAIN_BUS_GENERIC_MSI for HPET/IO-APIC 
> domain search
> From: Thomas Gleixner <tglx@...utronix.de>
> Date: Sun, 25 Feb 2024 16:56:12 +0100
> 
> The recent restriction to invoke irqdomain_ops::select() only when the
> domain bus toke is DOMAIN_BUS_ANY breaks the search for the parent MSI
> domain of HPET and IO-APIC. The latter causes a full boot fail.
> 
> The restriction itself makes sense to avoid adding DOMAIN_BUS_ANY 
> matches
> into the various ARM specific select() callbacks. Reverting this change
> would obviously break ARM platforms again and require DOMAIN_BUS_ANY
> matches added to various places.
> 
> A simpler solution is to use the DOMAIN_BUS_GENERIC_MSI token for the 
> HPET
> and IO-APIC parent domain search. This works out of the box because the
> affected parent domains check only for the firmware specification 
> content
> and not for the bus token.
> 
> Fixes: 5aa3c0cf5bba ("genirq/irqdomain: Don't call ops->select for
> DOMAIN_BUS_ANY tokens")
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>

Looks good to me.

Reviewed-by: Marc Zyngier <maz@...nel.org>

         M.
-- 
Jazz is not dead. It just smells funny...