[PATCH v2 5/5] gpio: cdev: don't overwrite the whole flags field when configuring lines
From: Bartosz Golaszewski
Date: Mon Nov 09 2026 - 05:30:34 EST
linehandle_flags_to_desc_flags() and
gpio_v2_line_config_flags_to_desc_flags() read the flags word with
READ_ONCE(), modify the local copy and write it back with WRITE_ONCE().
The per-request config_mutex does not exclude sysfs, the irq core or
drivers touching the same descriptor, so bits they set or clear
concurrently can be lost.
Build the mask of bits owned by the line configuration and their new
values locally and apply them with gpiod_update_flags(). The whole
group changes in a single atomic step.
While at it: rename the functions whose semantics changed from
"conversion" to "flags update" accordingly.
Cc: stable@xxxxxxxxxxxxxxx
Fixes: b7adfb6076ff ("gpio: cdev: update flags at once when reconfiguring from user-space")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
---
drivers/gpio/gpiolib-cdev.c | 111 +++++++++++++++++++++++++-------------------
1 file changed, 62 insertions(+), 49 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index f68e33cbd7d4cd5153e82ad77768cf52a90601d6..50baee455283c3aeb25ed92f3936c5f6a5b215bf 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -3,6 +3,7 @@
#include <linux/anon_inodes.h>
#include <linux/atomic.h>
#include <linux/bitmap.h>
+#include <linux/bitops.h>
#include <linux/build_bug.h>
#include <linux/cdev.h>
#include <linux/cleanup.h>
@@ -140,24 +141,30 @@ static int linehandle_validate_flags(u32 flags)
return 0;
}
-static void linehandle_flags_to_desc_flags(u32 lflags, unsigned long *flagsp)
+static void linehandle_update_desc_flags(u32 lflags, struct gpio_desc *desc)
{
- unsigned long flags = READ_ONCE(*flagsp);
+ const unsigned long mask = BIT(GPIOD_FLAG_ACTIVE_LOW) |
+ BIT(GPIOD_FLAG_OPEN_DRAIN) |
+ BIT(GPIOD_FLAG_OPEN_SOURCE) |
+ BIT(GPIOD_FLAG_PULL_UP) |
+ BIT(GPIOD_FLAG_PULL_DOWN) |
+ BIT(GPIOD_FLAG_BIAS_DISABLE);
+ unsigned long flags = 0;
- assign_bit(GPIOD_FLAG_ACTIVE_LOW, &flags,
- lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW);
- assign_bit(GPIOD_FLAG_OPEN_DRAIN, &flags,
- lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN);
- assign_bit(GPIOD_FLAG_OPEN_SOURCE, &flags,
- lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE);
- assign_bit(GPIOD_FLAG_PULL_UP, &flags,
- lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP);
- assign_bit(GPIOD_FLAG_PULL_DOWN, &flags,
- lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN);
- assign_bit(GPIOD_FLAG_BIAS_DISABLE, &flags,
- lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE);
+ __assign_bit(GPIOD_FLAG_ACTIVE_LOW, &flags,
+ lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW);
+ __assign_bit(GPIOD_FLAG_OPEN_DRAIN, &flags,
+ lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN);
+ __assign_bit(GPIOD_FLAG_OPEN_SOURCE, &flags,
+ lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE);
+ __assign_bit(GPIOD_FLAG_PULL_UP, &flags,
+ lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP);
+ __assign_bit(GPIOD_FLAG_PULL_DOWN, &flags,
+ lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN);
+ __assign_bit(GPIOD_FLAG_BIAS_DISABLE, &flags,
+ lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE);
- WRITE_ONCE(*flagsp, flags);
+ set_mask_bits(&desc->flags, mask, flags);
}
static long linehandle_set_config(struct linehandle_state *lh,
@@ -182,7 +189,7 @@ static long linehandle_set_config(struct linehandle_state *lh,
for (i = 0; i < lh->num_descs; i++) {
desc = lh->descs[i];
- linehandle_flags_to_desc_flags(lflags, &desc->flags);
+ linehandle_update_desc_flags(lflags, desc);
if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
int val = !!gcnf.default_values[i];
@@ -339,7 +346,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
if (ret)
return ret;
lh->descs[i] = desc;
- linehandle_flags_to_desc_flags(handlereq.flags, &desc->flags);
+ linehandle_update_desc_flags(handlereq.flags, desc);
ret = gpiod_set_transitory(desc, false);
if (ret < 0)
@@ -1216,37 +1223,43 @@ static int gpio_v2_line_config_validate(struct gpio_v2_line_config *lc,
return 0;
}
-static void gpio_v2_line_config_flags_to_desc_flags(u64 lflags,
- unsigned long *flagsp)
+static void linereq_update_desc_flags(u64 lflags,
+ struct gpio_desc *desc)
{
- unsigned long flags = READ_ONCE(*flagsp);
-
- assign_bit(GPIOD_FLAG_ACTIVE_LOW, &flags,
- lflags & GPIO_V2_LINE_FLAG_ACTIVE_LOW);
-
- assign_bit(GPIOD_FLAG_EDGE_RISING, &flags,
- lflags & GPIO_V2_LINE_FLAG_EDGE_RISING);
- assign_bit(GPIOD_FLAG_EDGE_FALLING, &flags,
- lflags & GPIO_V2_LINE_FLAG_EDGE_FALLING);
-
- assign_bit(GPIOD_FLAG_OPEN_DRAIN, &flags,
- lflags & GPIO_V2_LINE_FLAG_OPEN_DRAIN);
- assign_bit(GPIOD_FLAG_OPEN_SOURCE, &flags,
- lflags & GPIO_V2_LINE_FLAG_OPEN_SOURCE);
-
- assign_bit(GPIOD_FLAG_PULL_UP, &flags,
- lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP);
- assign_bit(GPIOD_FLAG_PULL_DOWN, &flags,
- lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN);
- assign_bit(GPIOD_FLAG_BIAS_DISABLE, &flags,
- lflags & GPIO_V2_LINE_FLAG_BIAS_DISABLED);
-
- assign_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &flags,
- lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME);
- assign_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &flags,
- lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE);
+ const unsigned long mask = BIT(GPIOD_FLAG_ACTIVE_LOW) |
+ BIT(GPIOD_FLAG_EDGE_RISING) |
+ BIT(GPIOD_FLAG_EDGE_FALLING) |
+ BIT(GPIOD_FLAG_OPEN_DRAIN) |
+ BIT(GPIOD_FLAG_OPEN_SOURCE) |
+ BIT(GPIOD_FLAG_PULL_UP) |
+ BIT(GPIOD_FLAG_PULL_DOWN) |
+ BIT(GPIOD_FLAG_BIAS_DISABLE) |
+ BIT(GPIOD_FLAG_EVENT_CLOCK_REALTIME) |
+ BIT(GPIOD_FLAG_EVENT_CLOCK_HTE);
+ unsigned long flags = 0;
- WRITE_ONCE(*flagsp, flags);
+ __assign_bit(GPIOD_FLAG_ACTIVE_LOW, &flags,
+ lflags & GPIO_V2_LINE_FLAG_ACTIVE_LOW);
+ __assign_bit(GPIOD_FLAG_EDGE_RISING, &flags,
+ lflags & GPIO_V2_LINE_FLAG_EDGE_RISING);
+ __assign_bit(GPIOD_FLAG_EDGE_FALLING, &flags,
+ lflags & GPIO_V2_LINE_FLAG_EDGE_FALLING);
+ __assign_bit(GPIOD_FLAG_OPEN_DRAIN, &flags,
+ lflags & GPIO_V2_LINE_FLAG_OPEN_DRAIN);
+ __assign_bit(GPIOD_FLAG_OPEN_SOURCE, &flags,
+ lflags & GPIO_V2_LINE_FLAG_OPEN_SOURCE);
+ __assign_bit(GPIOD_FLAG_PULL_UP, &flags,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP);
+ __assign_bit(GPIOD_FLAG_PULL_DOWN, &flags,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN);
+ __assign_bit(GPIOD_FLAG_BIAS_DISABLE, &flags,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_DISABLED);
+ __assign_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &flags,
+ lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME);
+ __assign_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &flags,
+ lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE);
+
+ set_mask_bits(&desc->flags, mask, flags);
}
static long linereq_get_values(struct linereq *lr, void __user *ip)
@@ -1404,7 +1417,7 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
*/
if (!(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
continue;
- gpio_v2_line_config_flags_to_desc_flags(flags, &desc->flags);
+ linereq_update_desc_flags(flags, desc);
edflags = flags & GPIO_V2_LINE_EDGE_DETECTOR_FLAGS;
if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
int val = gpio_v2_line_config_output_value(&lc, i);
@@ -1660,7 +1673,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
lr->lines[i].desc = desc;
flags = gpio_v2_line_config_flags(lc, i);
- gpio_v2_line_config_flags_to_desc_flags(flags, &desc->flags);
+ linereq_update_desc_flags(flags, desc);
ret = gpiod_set_transitory(desc, false);
if (ret < 0)
@@ -2055,7 +2068,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
le->desc = desc;
le->eflags = eflags;
- linehandle_flags_to_desc_flags(lflags, &desc->flags);
+ linehandle_update_desc_flags(lflags, desc);
ret = gpiod_direction_input(desc);
if (ret)
--
2.47.3