linux-kernel - Re: [PATCH 2/2] genirq: proc: fix a procfs entry leak

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: <CAMRc=MdYteOxy87jdSEvBxnN7tx_J1X2aSsRzKZ6WKL31-ipmA@mail.gmail.com>
Date:   Fri, 25 Aug 2023 13:01:12 +0200
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
        Marc Zyngier <marc.zyngier@....com>,
        Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH 2/2] genirq: proc: fix a procfs entry leak

On Fri, Aug 25, 2023 at 10:11 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Fri, Aug 25 2023 at 00:36, brgl@...ev.pl wrote:
> > On Thu, 24 Aug 2023 22:12:41 +0200, Thomas Gleixner <tglx@...utronix.de> said:
> > Here a GPIO device - that is also an irq chip - is unbound (this is a testing
> > module unbound during a test-case but it can be anything else, like an I2C
> > expander for which the driver is unloaded) while some users called
> > request_irq() on its interrupts (this is orthogonal to gpiod_get() and doesn't
> > take a reference to the module, so nothing is stopping us from
> > unloading it)
>
> You just described the real problem in this sentence. So why are you
> trying to cure a symptom?
>

Cc: Linus Walleij

Honestly I'm not following. Even if we did have a way of taking the
reference to the underlying GPIO module (I'm 99% percent sure, it's
not possible: we're not using any of the GPIO interfaces for that,
just platform_get_irq() or similar and all the GPIO subsystem sees is
the call to .irq_map() but that doesn't look like a reliable place to
take that reference) - that wouldn't stop anyone from unbinding the
device elsewhere: from user-space over sysfs or maybe it's a GPIO
expander over USB that gets unplugged (I know that it would only be
described in DT if it was hard-wired but it's still within the realm
of possibility).

Because this situation (removing a device while it still has users) is
possible, we have a way of handling that in the GPIO subsystem, where
if a device you're using goes from under you, the GPIO descriptor (the
only interface consumers have to that device) is "numbed down" and no
longer functional but doesn't leak resources or crash.

I was under the impression that the whole irqnum-to-irq_desc mapping
was designed to handle this situation on purpose, hence a check for
!desc and a silent return in free_irq(). If a missing mapping was a
bug, then it would warrant at least a warning, right?

Bartosz