linux-kernel - Re: [PATCH v2 0/2] gpio-cdev: Release IRQ used by gpio-cdev on gpio chip removal

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: <CACRpkdZbGUd-u3ZcmmK1POSEq8z9J1aDhbGPAbcR6guKuEpEFQ@mail.gmail.com>
Date: Thu, 29 Feb 2024 15:09:38 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Herve Codina <herve.codina@...tlin.com>, Kent Gibson <warthog618@...il.com>, 
	Saravana Kannan <saravanak@...gle.com>, linux-gpio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Luca Ceresoli <luca.ceresoli@...tlin.com>, 
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v2 0/2] gpio-cdev: Release IRQ used by gpio-cdev on gpio
 chip removal

On Tue, Feb 27, 2024 at 8:31 PM Bartosz Golaszewski <brgl@...ev.pl> wrote:
> On Tue, Feb 27, 2024 at 12:34 PM Herve Codina <herve.codina@...tlin.com> wrote:

> > Herve Codina (2):
> >   gpiolib: call gcdev_unregister() sooner in the removal operations
> >   gpiolib: cdev: release IRQs when the gpio chip device is removed
(...)
> Sorry but this is just papering over the real issue. I'd say NAK for
> now as I'd really prefer to get to the root of the problem and fix it
> for all GPIO interrupt users.
>
> Kent, Linus: what do you think?

I'm not sure. What does "all GPIO interrupt users" mean in this context?

If you mean "also the kernel-internal" (such as some random driver
having performed gpiod_to_irq() and requested it or, taken it from a
phandle in the device tree) then I think these are slightly semantically
different.

The big difference is that users of the cdev are *expected* to *crash*
sometimes, releasing the file handle and then this cleanup needs to
happen. Also cdev is more likely to be used for hotplugged/unplugged
GPIOs.

The kernel-internal users are *not* expected to crash, but to clean up
their usage the right way. Also they are predominantly if not exclusively
used for fixed GPIOs such as those on an SoC that do not hot-unplug
and go away randomly.

Use case 1: you run gpio-mon on a random GPIO with IRQ on a board.
It is using a SoC-native GPIO. Suddenly gpio-mon crashes because
of OOM or whatever and releases the filehandle on the way down.
What to do?

Use case 2: you plug in a USB dongle with GPIOs on. Start gpio-mon
on one of the pins. Unplug the dongle. Then it is fair that the cdev cleans
up the irq, because I don't see any way that a kernel driver would
request any of these GPIOs (but I'm more uncertain here).

I just think it is necessary to think about the big picture here.

Yours,
Linus Walleij