[PATCH] gpio: virtuser: fix interrupt leak and UAF

[PATCH] gpio: virtuser: fix interrupt leak and UAF

[Bartosz Golaszewski]

When configuring interrupt handlers for GPIOs requested by virtual
users, we never verify if an interrupt has been previously requested and
just leak it if it was.

We also never free the interrupts at device unbind. This is both a leak
and a potential UAF bug.

We use atomic_t for the irq field for now reason, it can't be modifying
or read from atomic context. Let's repurpose the existing consumer_lock
mutex and serialize access to the irq field. Let's add sanitization of
input and schedule a tear-down action.

Cc: stable@vger.kernel.org
Fixes: 91581c4b3f29 ("gpio: virtuser: new virtual testing driver for the GPIO API")
Closes: https://sashiko.dev/#/bug/linux-d8935133-0e80-487a-9e00-e748056f595e
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpio-virtuser.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpio-virtuser.c b/drivers/gpio/gpio-virtuser.c
index 185c1be39b27760b468011acee1c21e649e87f8f..21b4bc64d4d346c51811491ab1d92b4af36ec3c6 100644
--- a/drivers/gpio/gpio-virtuser.c
+++ b/drivers/gpio/gpio-virtuser.c
@@ -55,9 +55,9 @@ struct gpio_virtuser_line_array_data {
 struct gpio_virtuser_line_data {
 	struct gpio_virtuser_attr_data ad;
 	char consumer[GPIO_VIRTUSER_NAME_BUF_LEN];
-	struct mutex consumer_lock;
+	struct mutex lock;
 	unsigned int debounce;
-	atomic_t irq;
+	int irq;
 	atomic_t irq_count;
 };
 
@@ -607,7 +607,7 @@ static ssize_t gpio_virtuser_consumer_read(struct file *file,
 
 	memset(buf, 0x0, sizeof(buf));
 
-	scoped_guard(mutex, &data->consumer_lock)
+	scoped_guard(mutex, &data->lock)
 		ret = snprintf(buf, sizeof(buf), "%s\n", data->consumer);
 
 	return simple_read_from_buffer(user_buf, size, ppos, buf, ret);
@@ -635,7 +635,7 @@ static ssize_t gpio_virtuser_consumer_write(struct file *file,
 	if (ret)
 		return ret;
 
-	scoped_guard(mutex, &data->consumer_lock)
+	scoped_guard(mutex, &data->lock)
 		strscpy(data->consumer, buf, sizeof(data->consumer));
 
 	return count;
@@ -675,7 +675,12 @@ static int gpio_virtuser_interrupts_set(void *data, u64 val)
 	if (val > 1)
 		return -EINVAL;
 
+	guard(mutex)(&ld->lock);
+
 	if (val) {
+		if (ld->irq >= 0)
+			return -EBUSY;
+
 		irq = gpiod_to_irq(ld->ad.desc);
 		if (irq < 0)
 			return irq;
@@ -689,11 +694,13 @@ static int gpio_virtuser_interrupts_set(void *data, u64 val)
 		if (ret)
 			return ret;
 
-		atomic_set(&ld->irq, irq);
+		ld->irq = irq;
 	} else {
-		irq = atomic_xchg(&ld->irq, 0);
-		if (irq)
-			free_irq(irq, ld);
+		if (ld->irq < 0)
+			return -EBUSY;
+
+		free_irq(ld->irq, ld);
+		ld->irq = -1;
 	}
 
 	return 0;
@@ -793,6 +800,14 @@ static int gpio_virtuser_dbgfs_init_line_array_attrs(struct device *dev,
 			data->ad.dbgfs_dir, data);
 }
 
+static void gpio_virtuser_free_irq(void *data)
+{
+	struct gpio_virtuser_line_data *ld = data;
+
+	if (ld->irq >= 0)
+		free_irq(ld->irq, ld);
+}
+
 static int gpio_virtuser_dbgfs_init_line_attrs(struct device *dev,
 					       struct gpio_desc *desc,
 					       const char *id,
@@ -809,14 +824,18 @@ static int gpio_virtuser_dbgfs_init_line_attrs(struct device *dev,
 
 	data->ad.desc = desc;
 	strscpy(data->consumer, id);
-	atomic_set(&data->irq, 0);
+	data->irq = -1;
 	atomic_set(&data->irq_count, 0);
 
+	ret = devm_add_action_or_reset(dev, gpio_virtuser_free_irq, data);
+	if (ret)
+		return ret;
+
 	name = devm_kasprintf(dev, GFP_KERNEL, "gpiod:%s:%u", id, index);
 	if (!name)
 		return -ENOMEM;
 
-	ret = devm_mutex_init(dev, &data->consumer_lock);
+	ret = devm_mutex_init(dev, &data->lock);
 	if (ret)
 		return ret;
 

---
base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b
change-id: 20261106-gpio-virtuser-irq-uaf-leak-392b42bf2833

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH] gpio: virtuser: fix interrupt leak and UAF

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] gpiolib: race condition and double free in desc_set_label()
  [1]
--

commit 4879168b8f1f80dbe315c00e833ecc347918f4e3
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

gpio: virtuser: fix interrupt leak and UAF

This commit serializes access to the virtual user interrupt state using the
per-line mutex, prevents duplicate IRQ requests, and registers a devm action
to free the interrupt on device unbind.

> diff --git a/drivers/gpio/gpio-virtuser.c b/drivers/gpio/gpio-virtuser.c
> index 185c1be39b277..21b4bc64d4d34 100644
> --- a/drivers/gpio/gpio-virtuser.c
> +++ b/drivers/gpio/gpio-virtuser.c

[ ... ]

> @@ -635,7 +635,7 @@ static ssize_t gpio_virtuser_consumer_write(struct file *file,
>  	if (ret)
>  		return ret;
>  
> -	scoped_guard(mutex, &data->consumer_lock)
> +	scoped_guard(mutex, &data->lock)
>  		strscpy(data->consumer, buf, sizeof(data->consumer));
>  
>  	return count;

[Severity: Critical]
This problem wasn't introduced by this patch, but can concurrent writes in
gpio_virtuser_consumer_write() race in desc_set_label() [1] and double
enqueue old->rh via call_srcu()?

Just above this hunk, gpio_virtuser_consumer_write() calls
gpiod_set_consumer_name() -> desc_set_label() without holding data->lock, and
desc_set_label() replaces desc->label without any update-side lock:

drivers/gpio/gpiolib.c:desc_set_label() {
	...
	old = rcu_replace_pointer(desc->label, new, 1);
	if (old)
		call_srcu(&desc->gdev->desc_srcu, &old->rh, desc_free_label);

	return 0;
}

Because rcu_replace_pointer() is just a load followed by a store, two
concurrent callers can read the same old pointer:

CPU0                                CPU1
gpio_virtuser_consumer_write()      gpio_virtuser_consumer_write()
  gpiod_set_consumer_name()           gpiod_set_consumer_name()
    desc_set_label()                    desc_set_label()
      old = rcu_dereference(...)
                                          old = rcu_dereference(...)
      rcu_assign_pointer(..., new0)
                                          rcu_assign_pointer(..., new1)
      call_srcu(..., &old->rh, ...)
                                          call_srcu(..., &old->rh, ...)
                                            <-- double call_srcu() / double free

Queuing the same rcu_head twice corrupts the SRCU callback list and double
frees old after the grace period, while leaking new0.

[1] https://sashiko.dev/#/bug/linux-8fa6e742-7945-4de7-8786-8ad6e6c6ea4e

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-gpio-virtuser-irq-uaf-leak-v1-1-ce9772d8ad0d@oss.qualcomm.com?part=1

Re: [PATCH] gpio: virtuser: fix interrupt leak and UAF

[Bartosz Golaszewski]

On Fri, Nov 06, 2026 at 03:43:05PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found no new issues
> in this patch, but noticed 1 related pre-existing issue it does not
> introduce:
> - [Critical] gpiolib: race condition and double free in desc_set_label()
>   [1]
> --
> 
> commit 4879168b8f1f80dbe315c00e833ecc347918f4e3
> Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> 
> gpio: virtuser: fix interrupt leak and UAF
> 
> This commit serializes access to the virtual user interrupt state using the
> per-line mutex, prevents duplicate IRQ requests, and registers a devm action
> to free the interrupt on device unbind.
> 
> > diff --git a/drivers/gpio/gpio-virtuser.c b/drivers/gpio/gpio-virtuser.c
> > index 185c1be39b277..21b4bc64d4d34 100644
> > --- a/drivers/gpio/gpio-virtuser.c
> > +++ b/drivers/gpio/gpio-virtuser.c
> 
> [ ... ]
> 
> > @@ -635,7 +635,7 @@ static ssize_t gpio_virtuser_consumer_write(struct file *file,
> >  	if (ret)
> >  		return ret;
> >  
> > -	scoped_guard(mutex, &data->consumer_lock)
> > +	scoped_guard(mutex, &data->lock)
> >  		strscpy(data->consumer, buf, sizeof(data->consumer));
> >  
> >  	return count;
> 
> [Severity: Critical]
> This problem wasn't introduced by this patch, but can concurrent writes in
> gpio_virtuser_consumer_write() race in desc_set_label() [1] and double
> enqueue old->rh via call_srcu()?
> 
> Just above this hunk, gpio_virtuser_consumer_write() calls
> gpiod_set_consumer_name() -> desc_set_label() without holding data->lock, and
> desc_set_label() replaces desc->label without any update-side lock:
>

FYI: There's a patch[1] on the list that fixes that issue too.

Bart

[1] https://lore.kernel.org/all/20261105-gpiolib-set-desc-label-corruption-v1-1-fa84726ceac0@oss.qualcomm.com/

Re: [PATCH] gpio: virtuser: fix interrupt leak and UAF

[Bartosz Golaszewski]

On Fri, 06 Nov 2026 16:33:45 +0100, Bartosz Golaszewski wrote:
> When configuring interrupt handlers for GPIOs requested by virtual
> users, we never verify if an interrupt has been previously requested and
> just leak it if it was.
> 
> We also never free the interrupts at device unbind. This is both a leak
> and a potential UAF bug.
> 
> [...]

Applied, thanks!

[1/1] gpio: virtuser: fix interrupt leak and UAF
      https://git.kernel.org/brgl/c/24cafd9580627f9f066296f4a6ce1b282a3f425d

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>