[PATCH] gpiolib: clear the event clock flags when a line is released

[PATCH] gpiolib: clear the event clock flags when a line is released

[Adrian Ambrozewicz]

gpiod_free_commit() clears the flags that a request sets on a line, but
not GPIOD_FLAG_EVENT_CLOCK_REALTIME and GPIOD_FLAG_EVENT_CLOCK_HTE,
which gpio_v2_line_config_flags_to_desc_flags() sets from the uAPI v2
line config. Once a line requested with
GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME or
GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE is released, GPIO_V2_GET_LINEINFO_IOCTL
and the line info changed events, the "released" one included, keep
reporting that event clock for the free line:

  GPIO_V2_GET_LINE_IOCTL, line 1, input, event clock realtime:  ok
  close()
  GPIO_V2_GET_LINEINFO_IOCTL, line 1:  input, event clock realtime

The flags are set before edge detection is set up, so a request that
fails in hte_ts_get(), for instance because no HTE provider handles the
line, leaves GPIOD_FLAG_EVENT_CLOCK_HTE behind as well. The next uAPI v2
request rewrites both flags, but requests through uAPI v1, sysfs exports
and in-kernel consumers do not, and the line info then shows the event
clock of an earlier user for them too.

Clear both flags together with the other flags of the request.

Reproduced under QEMU with a test GPIO chip: after a released request
with the realtime event clock, and on a kernel with CONFIG_HTE=y but no
HTE provider after a failed request with the HTE event clock, the line
info of the free line still reports that event clock. With this change
it reports none.

Fixes: 26d060e47e25 ("gpiolib: cdev: allow edge event timestamps to be configured as REALTIME")
Fixes: 2068339a6c35 ("gpiolib: cdev: Add hardware timestamp clock type")
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
 drivers/gpio/gpiolib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 772c6f04f489..c6cee872e4ae 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2679,6 +2679,8 @@ void gpiod_free_commit(struct gpio_desc *desc)
 		clear_bit(GPIOD_FLAG_BIAS_DISABLE, &flags);
 		clear_bit(GPIOD_FLAG_EDGE_RISING, &flags);
 		clear_bit(GPIOD_FLAG_EDGE_FALLING, &flags);
+		clear_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &flags);
+		clear_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &flags);
 		clear_bit(GPIOD_FLAG_IS_HOGGED, &flags);
 #ifdef CONFIG_OF_DYNAMIC
 		WRITE_ONCE(desc->hog, NULL);

base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
2.43.0

Re: [PATCH] gpiolib: clear the event clock flags when a line is released

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: data race in gpiod_free_commit() causes debounce corruption
--

commit 9516d6cf1c2a072b1bec8e797ac66548d0323b06
Author: Adrian Ambrozewicz <aambrozewicz@nvidia.com>

gpiolib: clear the event clock flags when a line is released

This commit ensures that the GPIOD_FLAG_EVENT_CLOCK_REALTIME and
GPIOD_FLAG_EVENT_CLOCK_HTE flags are cleared when a GPIO line is released.
It modifies gpiod_free_commit() to clear these flags alongside others so
that they do not leak into the line info for a freed line.

> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 772c6f04f489b..c6cee872e4ae5 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2679,6 +2679,8 @@ void gpiod_free_commit(struct gpio_desc *desc)
>  		clear_bit(GPIOD_FLAG_BIAS_DISABLE, &flags);
>  		clear_bit(GPIOD_FLAG_EDGE_RISING, &flags);
>  		clear_bit(GPIOD_FLAG_EDGE_FALLING, &flags);
> +		clear_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &flags);
> +		clear_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &flags);
>  		clear_bit(GPIOD_FLAG_IS_HOGGED, &flags);
>  #ifdef CONFIG_OF_DYNAMIC
>  		WRITE_ONCE(desc->hog, NULL);

[Severity: High]
Does this code cause debounce corruption by clearing ownership too early?

In gpiod_free_commit(), the ownership synchronization token
(GPIOD_FLAG_REQUESTED) is cleared in desc->flags via a WRITE_ONCE() before
the function finishes cleaning up ancillary state (debounce_period_us) and
emitting the GPIO_V2_LINE_CHANGED_RELEASED notification:

drivers/gpio/gpiolib.c:gpiod_free_commit() {
    ...
		WRITE_ONCE(desc->flags, flags);
#ifdef CONFIG_GPIO_CDEV
		WRITE_ONCE(desc->debounce_period_us, 0);
#endif
		gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_RELEASED);
    ...
}

This allows a concurrent request (e.g., from GPIO_V2_GET_LINE_IOCTL via
linereq_create()) to successfully acquire the GPIO line:

drivers/gpio/gpiolib.c:gpiod_request_commit() {
    ...
	if (test_and_set_bit(GPIOD_FLAG_REQUESTED, &desc->flags))
		return -EBUSY;
    ...
}

It can then configure a new debounce_period_us:

drivers/gpio/gpiolib-cdev.c:edge_detector_setup() {
    ...
		WRITE_ONCE(line->desc->debounce_period_us, debounce_period_us);
    ...
}

The original freeing thread then resumes and overwrites the newly requested
line's debounce_period_us with 0. This completely breaks software debouncing,
causing a 0-delay software debounce that will trigger its delayed work
immediately on every hardware edge. This could lead to an interrupt storm
and CPU soft lockup.

Additionally, this sends an out-of-order RELEASED notification to userspace
for a line that is currently fully requested and active.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-gpiolib-event-clock-v1-1-aambrozewicz@nvidia.com?part=1