Message-ID: <5d605f7fc99456804911403102a4fe999a14cc85.camel@siemens.com>
Date: Thu, 13 Nov 2025 21:11:55 +0000
From: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
To: "warthog618@...il.com" <warthog618@...il.com>, "brgl@...ev.pl"
	<brgl@...ev.pl>, "linus.walleij@...aro.org" <linus.walleij@...aro.org>
CC: "bartosz.golaszewski@...aro.org" <bartosz.golaszewski@...aro.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 6/8] gpio: cdev: put emitting the line state events on
 a workqueue

Hello Bartosz,

On Fri, 2024-10-18 at 11:10 +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> 
> 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@...aro.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):

------------[ cut here ]------------
struct file::f_count incremented from zero; use-after-free condition present!
WARNING: CPU: 1 PID: 560 at git/include/linux/fs.h:1082 lineinfo_changed_notify+0x238/0x260
CPU: 1 UID: 966 PID: 560 Comm: gpio-manager Tainted: G           O       6.12.0-rc3+git8340ac8106a4 #1
Tainted: [O]=OOT_MODULE
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : lineinfo_changed_notify+0x238/0x260
lr : lineinfo_changed_notify+0x238/0x260
Call trace:
 lineinfo_changed_notify+0x238/0x260
 notifier_call_chain+0xd4/0x248
 blocking_notifier_call_chain+0x5c/0xa8
 gpiod_free_commit+0x2c8/0x4a0
 gpiod_free+0x2c/0x90
 linereq_free+0xcc/0x190
 linereq_release+0x2c/0x48
 __fput+0x1b4/0x498
 ____fput+0x1c/0x30
 task_work_run+0x120/0x1d0
 do_exit+0x49c/0xff0
 do_group_exit+0x60/0xf8
 get_signal+0xdb8/0xea0
 do_signal+0x138/0x1b80
 do_notify_resume+0x118/0x1c8
 el0_svc+0xbc/0xc0
 el0t_64_sync_handler+0x120/0x130
 el0t_64_sync+0x190/0x198
irq event stamp: 212610
hardirqs last  enabled at (212609): [<ffff8000800d13fc>] finish_task_switch.isra.0+0xfc/0x3e8
hardirqs last disabled at (212610): [<ffff800081630928>] el1_dbg+0x28/0x90
softirqs last  enabled at (212600): [<ffff800080076e58>] handle_softirqs+0x6b0/0x6d8
softirqs last disabled at (212463): [<ffff800080010254>] __do_softirq+0x1c/0x28
---[ end trace 0000000000000000 ]---
==================================================================
BUG: KASAN: use-after-free in gpio_chrdev_release+0x30/0x98
Read of size 8 at addr ffff000006b94000 by task gpio-manager/560

CPU: 1 UID: 966 PID: 560 Comm: gpio-manager Tainted: G        W  O       6.12.0-rc3+git8340ac8106a4 #1
Tainted: [W]=WARN, [O]=OOT_MODULE
Call trace:
 dump_backtrace+0xa0/0x128
 show_stack+0x20/0x38
 dump_stack_lvl+0x8c/0xd0
 print_report+0xfc/0x5e0
 kasan_report+0xc0/0x100
 __asan_load8+0x9c/0xc0
 gpio_chrdev_release+0x30/0x98
 __fput+0x1b4/0x498
 ____fput+0x1c/0x30
 task_work_run+0x120/0x1d0
 do_exit+0x49c/0xff0
 do_group_exit+0x60/0xf8
 get_signal+0xdb8/0xea0
 do_signal+0x138/0x1b80
 do_notify_resume+0x118/0x1c8
 el0_svc+0xbc/0xc0
 el0t_64_sync_handler+0x120/0x130
 el0t_64_sync+0x190/0x198

The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff000006b974c0 pfn:0x86b94
flags: 0x0(zone=0)
page_type: f0(buddy)
raw: 0000000000000000 fffffdffc03eaf08 ffff800083356e90 0000000000000000
raw: ffff000006b974c0 0000000000000002 00000000f0000000 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff000006b93f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff000006b93f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff000006b94000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
                   ^
 ffff000006b94080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff000006b94100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
------------[ cut here ]------------
WARNING: CPU: 1 PID: 30 at git/mm/slub.c:4689 free_large_kmalloc+0x100/0x140
CPU: 1 UID: 0 PID: 30 Comm: kworker/1:1 Tainted: G        W  O       6.12.0-rc3+git8340ac8106a4 #1
Disabling lock debugging due to kernel taint
Tainted: [W]=WARN, [O]=OOT_MODULE
object pointer: 0xffff000006b94000
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000028
Workqueue: events delayed_fput
pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
Mem abort info:
pc : free_large_kmalloc+0x100/0x140
lr : kfree+0x2d4/0x418
Call trace:
 free_large_kmalloc+0x100/0x140
 kfree+0x2d4/0x418
 gpio_chrdev_release+0x78/0x98
Data abort info:
 __fput+0x1b4/0x498
  ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
 delayed_fput+0x68/0x88
  CM = 0, WnR = 0, TnD = 0, TagAccess = 0
 process_one_work+0x454/0xa80
  GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
 worker_thread+0x2c0/0x528
 kthread+0x1cc/0x1e0
 ret_from_fork+0x10/0x20
irq event stamp: 113358
hardirqs last  enabled at (113357): [<ffff800081645784>] _raw_spin_unlock_irqrestore+0x7c/0x88
hardirqs last disabled at (113358): [<ffff800081630928>] el1_dbg+0x28/0x90
user pgtable: 4k pages, 48-bit VAs, pgdp=0000000087eb6000
softirqs last  enabled at (111764): [<ffff800080076e58>] handle_softirqs+0x6b0/0x6d8
[0000000000000028] pgd=0000000000000000
softirqs last disabled at (111759): [<ffff800080010254>] __do_softirq+0x1c/0x28
, p4d=0000000000000000
---[ end trace 0000000000000000 ]---
------------[ cut here ]------------
WARNING: CPU: 1 PID: 30 at git/fs/dcache.c:774 dput.part.0+0x43c/0x458
CPU: 1 UID: 0 PID: 30 Comm: kworker/1:1 Tainted: G    B   W  O       6.12.0-rc3+git8340ac8106a4 #1
Tainted: [B]=BAD_PAGE, [W]=WARN, [O]=OOT_MODULE
Workqueue: events delayed_fput
pstate: a0000005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : dput.part.0+0x43c/0x458
lr : dput.part.0+0x36c/0x458
Call trace:
 dput.part.0+0x43c/0x458
 dput+0x1c/0x38
 __fput+0x218/0x498
 delayed_fput+0x68/0x88
 process_one_work+0x454/0xa80
 worker_thread+0x2c0/0x528
 kthread+0x1cc/0x1e0
 ret_from_fork+0x10/0x20
irq event stamp: 113358
hardirqs last  enabled at (113357): [<ffff800081645784>] _raw_spin_unlock_irqrestore+0x7c/0x88
hardirqs last disabled at (113358): [<ffff800081630928>] el1_dbg+0x28/0x90
Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
softirqs last  enabled at (111764): [<ffff800080076e58>] handle_softirqs+0x6b0/0x6d8
softirqs last disabled at (111759): [<ffff800080010254>] __do_softirq+0x1c/0x28
---[ end trace 0000000000000000 ]---


> ---
>  drivers/gpio/gpiolib-cdev.c | 82 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 71 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index b242fdb1ad28..7759dca92f8b 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -2333,6 +2333,7 @@ struct gpio_chardev_data {
>  #ifdef CONFIG_GPIO_CDEV_V1
>  	atomic_t watch_abi_version;
>  #endif
> +	struct file *fp;
>  };
>  
>  static int chipinfo_get(struct gpio_chardev_data *cdev, void __user *ip)
> @@ -2502,28 +2503,86 @@ static long gpio_ioctl_compat(struct file *file, unsigned int cmd,
>  }
>  #endif
>  
> +struct lineinfo_changed_ctx {
> +	struct work_struct work;
> +	struct gpio_v2_line_info_changed chg;
> +	struct gpio_device *gdev;
> +	struct gpio_chardev_data *cdev;
> +};
> +
> +static void lineinfo_changed_func(struct work_struct *work)
> +{
> +	struct lineinfo_changed_ctx *ctx =
> +			container_of(work, struct lineinfo_changed_ctx, work);
> +	struct gpio_chip *gc;
> +	int ret;
> +
> +	if (!(ctx->chg.info.flags & GPIO_V2_LINE_FLAG_USED)) {
> +		/*
> +		 * If nobody set the USED flag earlier, let's see with pinctrl
> +		 * now. We're doing this late because it's a sleeping function.
> +		 * Pin functions are in general much more static and while it's
> +		 * not 100% bullet-proof, it's good enough for most cases.
> +		 */
> +		scoped_guard(srcu, &ctx->gdev->srcu) {
> +			gc = srcu_dereference(ctx->gdev->chip, &ctx->gdev->srcu);
> +			if (gc &&
> +			    !pinctrl_gpio_can_use_line(gc, ctx->chg.info.offset))
> +				ctx->chg.info.flags |= GPIO_V2_LINE_FLAG_USED;
> +		}
> +	}
> +
> +	ret = kfifo_in_spinlocked(&ctx->cdev->events, &ctx->chg, 1,
> +				  &ctx->cdev->wait.lock);
> +	if (ret)
> +		wake_up_poll(&ctx->cdev->wait, EPOLLIN);
> +	else
> +		pr_debug_ratelimited("lineinfo event FIFO is full - event dropped\n");
> +
> +	gpio_device_put(ctx->gdev);
> +	fput(ctx->cdev->fp);
> +	kfree(ctx);
> +}
> +
>  static int lineinfo_changed_notify(struct notifier_block *nb,
>  				   unsigned long action, void *data)
>  {
>  	struct gpio_chardev_data *cdev =
>  		container_of(nb, struct gpio_chardev_data, lineinfo_changed_nb);
> -	struct gpio_v2_line_info_changed chg;
> +	struct lineinfo_changed_ctx *ctx;
>  	struct gpio_desc *desc = data;
> -	int ret;
>  
>  	if (!test_bit(gpio_chip_hwgpio(desc), cdev->watched_lines))
>  		return NOTIFY_DONE;
>  
> -	memset(&chg, 0, sizeof(chg));
> -	chg.event_type = action;
> -	chg.timestamp_ns = ktime_get_ns();
> -	gpio_desc_to_lineinfo(desc, &chg.info, false);
> +	/*
> +	 * If this is called from atomic context (for instance: with a spinlock
> +	 * taken by the atomic notifier chain), any sleeping calls must be done
> +	 * outside of this function in process context of the dedicated
> +	 * workqueue.
> +	 *
> +	 * Let's gather as much info as possible from the descriptor and
> +	 * postpone just the call to pinctrl_gpio_can_use_line() until the work
> +	 * is executed.
> +	 */
>  
> -	ret = kfifo_in_spinlocked(&cdev->events, &chg, 1, &cdev->wait.lock);
> -	if (ret)
> -		wake_up_poll(&cdev->wait, EPOLLIN);
> -	else
> -		pr_debug_ratelimited("lineinfo event FIFO is full - event dropped\n");
> +	ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
> +	if (!ctx) {
> +		pr_err("Failed to allocate memory for line info notification\n");
> +		return NOTIFY_DONE;
> +	}
> +
> +	ctx->chg.event_type = action;
> +	ctx->chg.timestamp_ns = ktime_get_ns();
> +	gpio_desc_to_lineinfo(desc, &ctx->chg.info, true);
> +	/* 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);
>  
>  	return NOTIFY_OK;
>  }
> @@ -2683,6 +2742,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
>  		goto out_unregister_line_notifier;
>  
>  	file->private_data = cdev;
> +	cdev->fp = file;
>  
>  	ret = nonseekable_open(inode, file);
>  	if (ret)
> 

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Powered by blists - more mailing lists