Re: [PATCH v5 6/8] gpio: cdev: put emitting the line state events on a workqueue

[Bartosz Golaszewski <brgl@bgdev.pl>]

On Fri, 14 Nov 2025 10:04:11 +0100, "Sverdlin, Alexander"
<alexander.sverdlin@siemens.com> said:
> Hi Bartosz,
>
> On Fri, 2025-11-14 at 09:21 +0100, Bartosz Golaszewski wrote:
>> > > In order to allow line state notifications to be emitted from atomic
>> > > context (for instance: from gpiod_direction_input/output()), we must
>> > > stop calling any sleeping functions in lineinfo_changed_notify(). To
>> > > that end let's use the new workqueue.
>> > >
>> > > Let's atomically allocate small structures containing the required data
>> > > and fill it with information immediately upon being notified about the
>> > > change except for the pinctrl state which will be retrieved later from
>> > > process context. We can pretty reliably do this as pin functions are
>> > > typically set once per boot.
>> > >
>> > > Let's make sure to bump the reference count of GPIO device and the GPIO
>> > > character device file descriptor to keep both alive until the event was
>> > > queued.
>> > >
>> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> >
>> > starting from this patch up to the current linux-next (v6.18-rcX)
>> > I see the following refcnt warnings + KASAN UAF reports on either reboot
>> > (when gpio-manager is being stopped) or
>> > `systemctl kill --signal=SIGKILL gpio-manager` (if some GPIOs are being
>> > requested from (owned by) gpio-manager prior to kill):
>
> []
>
>> Thanks for the bug report. I confirm it's reproducible on my side too.
>> It never occurred to me to try and SIGKILL gpio-manager. On normal
>> exit, nothing bad happens. Let me look into it.
>
> In my case it happens also on every reboot/shutdown, however if I
> `systemctl stop gpio-manager`, I don't see the issue... not sure yet,
> what is the difference... and I'm not telling SIGKILL is how one
> should stop gpio-manager, but I'm happy SIGKILL now allows you to
> reproduce the issue in the kernel code!
>

When the process is killed, it seems the character device's file struct is no
longer valid even though chrdev_release() was not yet called - it's called
after we try to send out this notification. I think it's best to just check
if the file is active and not do anything if it isn't.

Can you test the following change before I submit a formal patch?

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index e6a289fa0f8f..0f1103a679b5 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2548,10 +2548,17 @@ static int lineinfo_changed_notify(struct
notifier_block *nb,
 		container_of(nb, struct gpio_chardev_data, lineinfo_changed_nb);
 	struct lineinfo_changed_ctx *ctx;
 	struct gpio_desc *desc = data;
+	struct file *fp;

 	if (!test_bit(gpio_chip_hwgpio(desc), cdev->watched_lines))
 		return NOTIFY_DONE;

+	/* Keep the file descriptor alive for the duration of the notification. */
+	fp = get_file_active(&cdev->fp);
+	if (!fp)
+		/* Chardev file descriptor was already released. */
+		return NOTIFY_DONE;
+
 	/*
 	 * If this is called from atomic context (for instance: with a spinlock
 	 * taken by the atomic notifier chain), any sleeping calls must be done
@@ -2575,8 +2582,6 @@ static int lineinfo_changed_notify(struct
notifier_block *nb,
 	/* Keep the GPIO device alive until we emit the event. */
 	ctx->gdev = gpio_device_get(desc->gdev);
 	ctx->cdev = cdev;
-	/* Keep the file descriptor alive too. */
-	get_file(ctx->cdev->fp);

 	INIT_WORK(&ctx->work, lineinfo_changed_func);
 	queue_work(ctx->gdev->line_state_wq, &ctx->work);

>> Do you also go by ccpalex on github? I want to give you credit for
>> reporting the other bug in gpio-manager as well.
>
> That's me! I didn't know where to report an issue with user-space libgpiod,
> that's why I went with github ;-)
>

You can use this mailing list but github is fine too. Thanks.

Bartosz