linux-kernel - Re: [PATCH v2 2/4] irqdomain: Fix mapping-creation race

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] [day] [month] [year] [list]

Date:   Fri, 9 Dec 2022 15:18:33 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Johan Hovold <johan+linaro@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh@...nel.org>, linux-kernel@...r.kernel.org,
        Dmitry Torokhov <dtor@...omium.org>,
        Jon Hunter <jonathanh@...dia.com>
Subject: Re: [PATCH v2 2/4] irqdomain: Fix mapping-creation race

Hi Marc,

On Thu, Sep 15, 2022 at 09:54:25AM +0100, Marc Zyngier wrote:
> Johan,
> 
> On Thu, 01 Sep 2022 15:28:14 +0100,
> Johan Hovold <johan+linaro@...nel.org> wrote:
> > 
> > Parallel probing (e.g. due to asynchronous probing) of devices that share
> > interrupts can currently result in two mappings for the same hardware
> > interrupt to be created.
> > 
> > Add a serialising mapping mutex so that looking for an existing mapping
> > before creating a new one is done atomically.
> > 
> > Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers")
> > Fixes: b62b2cf5759b ("irqdomain: Fix handling of type settings for existing mappings")
> > Cc: Dmitry Torokhov <dtor@...omium.org>
> > Cc: Jon Hunter <jonathanh@...dia.com>
> > Link: https://lore.kernel.org/r/YuJXMHoT4ijUxnRb@hovoldconsulting.com
> > Signed-off-by: Johan Hovold <johan+linaro@...nel.org>

> I must confess I have a hard time figuring out the semantic difference
> between map_mutex and revmap_mutex. or rather, what is the use of
> revmap_mutex once map_mutex is taken. They fundamentally overlap, and
> I have the feeling one should eventually replace the other.
> 
> If anything, you should absolutely define/document how these two locks
> interact.

Sorry about the late follow-up on this. I meant to revisit this much
sooner, but couldn't seem to find the time until this week.

I just sent you a v3 which reworks the irqdomain locking and fixes the
race in the process. In the end the irq_domain_mutex is only used for
managing the irq_domain_list, while domain operations use per-domain
(hierarchy) locking.

Johan