From nobody Fri Sep 19 05:32:46 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 557E3C4321E for ; Tue, 29 Nov 2022 12:36:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234507AbiK2MgY (ORCPT ); Tue, 29 Nov 2022 07:36:24 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58232 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234395AbiK2MgK (ORCPT ); Tue, 29 Nov 2022 07:36:10 -0500 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AEE195EF9C for ; Tue, 29 Nov 2022 04:36:04 -0800 (PST) Received: by mail-wm1-x332.google.com with SMTP id 5so10772448wmo.1 for ; Tue, 29 Nov 2022 04:36:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bgdev-pl.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=65EIRuC/f+qhbhpFeJUbpfhpS/MTh9F0f4z0kCtj+6M=; b=pBf5Tlxx5splyz4GvTksaYcu1rH9KNyVN77k5JKnOA8jDXPzeGn3NkLVaKYVYcoCJJ 0sXUGD0YeE8q7ViOfE4LcbJDLr1X7hUASLWTa2D0hXsDPGn+wM8C5uEtbe/bg9RTqJtX 8e0dWKm4Qhy9aBwAORdmPMrK6bhVHUKxXyWEkN02ql4logjL0ZWBYKyxRjwSqMAhFZ9U 0XB1FQF8zsiDwDr4ga/gLPpy76u7dSBIGFS+jYOY+yBrg+wAgM9LJjsisnUJm5uG5r8d H53busQIGhs+6IQ95nBVK9iugqXc917DqUqnhl63x2GTOLmUd2F9dlZqLsnFaQTo+lQy 983g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=65EIRuC/f+qhbhpFeJUbpfhpS/MTh9F0f4z0kCtj+6M=; b=d1+xj2yL/lPazEQGcHd9tDe2Q5DMpgDxfOjSF6liSXRfsB3O9KMi3iiMa4xFiL4wYK KJ2Y8cCH+VJrXx6P385Cpv0juLjw+zjrv983KgWd+iNUgRYmYhNEtHRq+jnBixBSHSo7 UV5PPFAUPjFvhNu8H+Fnkk5rpUXQ+tLJ7dtrtNGYERF/4VatX3CZTWxNjDCGXW3c364Q 7woFspEC41dZmRfKuIjlnLdA8OBNMSTnu5OBWKf3dk5v2b+FnF03neOlPemYeC2l8xct w0fsDJunzJSuxKrdrF4rxFkIqAICDkrhNfTjnpvCmjwRh0fxWvyhQuP/KtqgWCZ3g8KL 2qKA== X-Gm-Message-State: ANoB5pnCbYqoSZB9rwfgoiF9EWhf5Af+TIVoCMyYaW+SXt52zqdaDZGN J7w16yE7D/zMmG77uIlHY+U+Wg== X-Google-Smtp-Source: AA0mqf4Rmkyuf14DcCkE15BNJ8lbaRyhzGDW+5ZJ914isRojfXxYZx9nbJGYsBs1lztMxAvguCAvvg== X-Received: by 2002:a05:600c:511b:b0:3d0:128a:6d1e with SMTP id o27-20020a05600c511b00b003d0128a6d1emr26852165wms.108.1669725363173; Tue, 29 Nov 2022 04:36:03 -0800 (PST) Received: from brgl-uxlite.home ([2a01:cb1d:334:ac00:6b19:1d8e:fbca:fd02]) by smtp.gmail.com with ESMTPSA id k13-20020a05600c1c8d00b003c6bd91caa5sm2295306wms.17.2022.11.29.04.36.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Nov 2022 04:36:02 -0800 (PST) From: Bartosz Golaszewski To: Kent Gibson , Linus Walleij , Andy Shevchenko Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, Bartosz Golaszewski Subject: [PATCH v3 2/2] gpiolib: protect the GPIO device against being dropped while in use by user-space Date: Tue, 29 Nov 2022 13:35:53 +0100 Message-Id: <20221129123553.353410-3-brgl@bgdev.pl> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20221129123553.353410-1-brgl@bgdev.pl> References: <20221129123553.353410-1-brgl@bgdev.pl> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" From: Bartosz Golaszewski While any of the GPIO cdev syscalls is in progress, the kernel can call gpiochip_remove() (for instance, when a USB GPIO expander is disconnected) which will set gdev->chip to NULL after which any subsequent access will cause a crash. To avoid that: use an RW-semaphore in which the syscalls take it for reading (so that we don't needlessly prohibit the user-space from calling syscalls simultaneously) while gpiochip_remove() takes it for writing so that it can only happen once all syscalls return. Fixes: d7c51b47ac11 ("gpio: userspace ABI for reading/writing GPIO lines") Fixes: 3c0d9c635ae2 ("gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPI= O_V2_LINE_GET_VALUES_IOCTL") Fixes: aad955842d1c ("gpiolib: cdev: support GPIO_V2_GET_LINEINFO_IOCTL and= GPIO_V2_GET_LINEINFO_WATCH_IOCTL") Fixes: a54756cb24ea ("gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL") Fixes: 7b8e00d98168 ("gpiolib: cdev: support GPIO_V2_LINE_SET_VALUES_IOCTL") Signed-off-by: Bartosz Golaszewski Reviewed-by: Kent Gibson --- drivers/gpio/gpiolib-cdev.c | 163 +++++++++++++++++++++++++++++++----- drivers/gpio/gpiolib.c | 3 + drivers/gpio/gpiolib.h | 5 ++ 3 files changed, 149 insertions(+), 22 deletions(-) diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index 911d91668903..a075f44310e8 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -84,6 +84,50 @@ struct linehandle_state { GPIOHANDLE_REQUEST_OPEN_DRAIN | \ GPIOHANDLE_REQUEST_OPEN_SOURCE) =20 +static __poll_t call_poll_locked(struct file *file, + struct poll_table_struct *wait, + struct gpio_device *gdev, + __poll_t (*func)(struct file *, + struct poll_table_struct *)) +{ + __poll_t ret; + + down_read(&gdev->sem); + ret =3D func(file, wait); + up_read(&gdev->sem); + + return ret; +} + +static long call_ioctl_locked(struct file *file, unsigned int cmd, + unsigned long arg, struct gpio_device *gdev, + long (*func)(struct file *, unsigned int, + unsigned long)) +{ + long ret; + + down_read(&gdev->sem); + ret =3D func(file, cmd, arg); + up_read(&gdev->sem); + + return ret; +} + +static ssize_t call_read_locked(struct file *file, char __user *buf, + size_t count, loff_t *f_ps, + struct gpio_device *gdev, + ssize_t (*func)(struct file *, char __user *, + size_t, loff_t *)) +{ + ssize_t ret; + + down_read(&gdev->sem); + ret =3D func(file, buf, count, f_ps); + up_read(&gdev->sem); + + return ret; +} + static int linehandle_validate_flags(u32 flags) { /* Return an error if an unknown flag is set */ @@ -191,8 +235,8 @@ static long linehandle_set_config(struct linehandle_sta= te *lh, return 0; } =20 -static long linehandle_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) +static long linehandle_ioctl_unlocked(struct file *file, unsigned int cmd, + unsigned long arg) { struct linehandle_state *lh =3D file->private_data; void __user *ip =3D (void __user *)arg; @@ -250,6 +294,15 @@ static long linehandle_ioctl(struct file *file, unsign= ed int cmd, } } =20 +static long linehandle_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct linehandle_state *lh =3D file->private_data; + + return call_ioctl_locked(file, cmd, arg, lh->gdev, + linehandle_ioctl_unlocked); +} + #ifdef CONFIG_COMPAT static long linehandle_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg) @@ -1381,8 +1434,8 @@ static long linereq_set_config(struct linereq *lr, vo= id __user *ip) return ret; } =20 -static long linereq_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) +static long linereq_ioctl_unlocked(struct file *file, unsigned int cmd, + unsigned long arg) { struct linereq *lr =3D file->private_data; void __user *ip =3D (void __user *)arg; @@ -1402,6 +1455,15 @@ static long linereq_ioctl(struct file *file, unsigne= d int cmd, } } =20 +static long linereq_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct linereq *lr =3D file->private_data; + + return call_ioctl_locked(file, cmd, arg, lr->gdev, + linereq_ioctl_unlocked); +} + #ifdef CONFIG_COMPAT static long linereq_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg) @@ -1410,8 +1472,8 @@ static long linereq_ioctl_compat(struct file *file, u= nsigned int cmd, } #endif =20 -static __poll_t linereq_poll(struct file *file, - struct poll_table_struct *wait) +static __poll_t linereq_poll_unlocked(struct file *file, + struct poll_table_struct *wait) { struct linereq *lr =3D file->private_data; __poll_t events =3D 0; @@ -1428,10 +1490,16 @@ static __poll_t linereq_poll(struct file *file, return events; } =20 -static ssize_t linereq_read(struct file *file, - char __user *buf, - size_t count, - loff_t *f_ps) +static __poll_t linereq_poll(struct file *file, + struct poll_table_struct *wait) +{ + struct linereq *lr =3D file->private_data; + + return call_poll_locked(file, wait, lr->gdev, linereq_poll_unlocked); +} + +static ssize_t linereq_read_unlocked(struct file *file, char __user *buf, + size_t count, loff_t *f_ps) { struct linereq *lr =3D file->private_data; struct gpio_v2_line_event le; @@ -1485,6 +1553,15 @@ static ssize_t linereq_read(struct file *file, return bytes_read; } =20 +static ssize_t linereq_read(struct file *file, char __user *buf, + size_t count, loff_t *f_ps) +{ + struct linereq *lr =3D file->private_data; + + return call_read_locked(file, buf, count, f_ps, lr->gdev, + linereq_read_unlocked); +} + static void linereq_free(struct linereq *lr) { unsigned int i; @@ -1722,8 +1799,8 @@ struct lineevent_state { (GPIOEVENT_REQUEST_RISING_EDGE | \ GPIOEVENT_REQUEST_FALLING_EDGE) =20 -static __poll_t lineevent_poll(struct file *file, - struct poll_table_struct *wait) +static __poll_t lineevent_poll_unlocked(struct file *file, + struct poll_table_struct *wait) { struct lineevent_state *le =3D file->private_data; __poll_t events =3D 0; @@ -1739,15 +1816,21 @@ static __poll_t lineevent_poll(struct file *file, return events; } =20 +static __poll_t lineevent_poll(struct file *file, + struct poll_table_struct *wait) +{ + struct lineevent_state *le =3D file->private_data; + + return call_poll_locked(file, wait, le->gdev, lineevent_poll_unlocked); +} + struct compat_gpioeevent_data { compat_u64 timestamp; u32 id; }; =20 -static ssize_t lineevent_read(struct file *file, - char __user *buf, - size_t count, - loff_t *f_ps) +static ssize_t lineevent_read_unlocked(struct file *file, char __user *buf, + size_t count, loff_t *f_ps) { struct lineevent_state *le =3D file->private_data; struct gpioevent_data ge; @@ -1815,6 +1898,15 @@ static ssize_t lineevent_read(struct file *file, return bytes_read; } =20 +static ssize_t lineevent_read(struct file *file, char __user *buf, + size_t count, loff_t *f_ps) +{ + struct lineevent_state *le =3D file->private_data; + + return call_read_locked(file, buf, count, f_ps, le->gdev, + lineevent_read_unlocked); +} + static void lineevent_free(struct lineevent_state *le) { if (le->irq) @@ -1832,8 +1924,8 @@ static int lineevent_release(struct inode *inode, str= uct file *file) return 0; } =20 -static long lineevent_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) +static long lineevent_ioctl_unlocked(struct file *file, unsigned int cmd, + unsigned long arg) { struct lineevent_state *le =3D file->private_data; void __user *ip =3D (void __user *)arg; @@ -1864,6 +1956,15 @@ static long lineevent_ioctl(struct file *file, unsig= ned int cmd, return -EINVAL; } =20 +static long lineevent_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct lineevent_state *le =3D file->private_data; + + return call_ioctl_locked(file, cmd, arg, le->gdev, + lineevent_ioctl_unlocked); +} + #ifdef CONFIG_COMPAT static long lineevent_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg) @@ -2422,8 +2523,8 @@ static int lineinfo_changed_notify(struct notifier_bl= ock *nb, return NOTIFY_OK; } =20 -static __poll_t lineinfo_watch_poll(struct file *file, - struct poll_table_struct *pollt) +static __poll_t lineinfo_watch_poll_unlocked(struct file *file, + struct poll_table_struct *pollt) { struct gpio_chardev_data *cdev =3D file->private_data; __poll_t events =3D 0; @@ -2440,8 +2541,17 @@ static __poll_t lineinfo_watch_poll(struct file *fil= e, return events; } =20 -static ssize_t lineinfo_watch_read(struct file *file, char __user *buf, - size_t count, loff_t *off) +static __poll_t lineinfo_watch_poll(struct file *file, + struct poll_table_struct *pollt) +{ + struct gpio_chardev_data *cdev =3D file->private_data; + + return call_poll_locked(file, pollt, cdev->gdev, + lineinfo_watch_poll_unlocked); +} + +static ssize_t lineinfo_watch_read_unlocked(struct file *file, char __user= *buf, + size_t count, loff_t *off) { struct gpio_chardev_data *cdev =3D file->private_data; struct gpio_v2_line_info_changed event; @@ -2519,6 +2629,15 @@ static ssize_t lineinfo_watch_read(struct file *file= , char __user *buf, return bytes_read; } =20 +static ssize_t lineinfo_watch_read(struct file *file, char __user *buf, + size_t count, loff_t *off) +{ + struct gpio_chardev_data *cdev =3D file->private_data; + + return call_read_locked(file, buf, count, off, cdev->gdev, + lineinfo_watch_read_unlocked); +} + /** * gpio_chrdev_open() - open the chardev for ioctl operations * @inode: inode for this chardev diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 4756ea08894f..0d71f8a5a66e 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -731,6 +731,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, vo= id *data, spin_unlock_irqrestore(&gpio_lock, flags); =20 BLOCKING_INIT_NOTIFIER_HEAD(&gdev->notifier); + init_rwsem(&gdev->sem); =20 #ifdef CONFIG_PINCTRL INIT_LIST_HEAD(&gdev->pin_ranges); @@ -865,6 +866,7 @@ void gpiochip_remove(struct gpio_chip *gc) unsigned long flags; unsigned int i; =20 + down_write(&gdev->sem); /* FIXME: should the legacy sysfs handling be moved to gpio_device? */ gpiochip_sysfs_unregister(gdev); gpiochip_free_hogs(gc); @@ -899,6 +901,7 @@ void gpiochip_remove(struct gpio_chip *gc) * gone. */ gcdev_unregister(gdev); + up_write(&gdev->sem); put_device(&gdev->dev); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index d900ecdbac46..9ad68a0adf4a 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -15,6 +15,7 @@ #include #include #include +#include =20 #define GPIOCHIP_NAME "gpiochip" =20 @@ -39,6 +40,9 @@ * @list: links gpio_device:s together for traversal * @notifier: used to notify subscribers about lines being requested, rele= ased * or reconfigured + * @sem: protects the structure from a NULL-pointer dereference of @chip by + * user-space operations when the device gets unregistered during + * a hot-unplug event * @pin_ranges: range of pins served by the GPIO driver * * This state container holds most of the runtime variable data @@ -60,6 +64,7 @@ struct gpio_device { void *data; struct list_head list; struct blocking_notifier_head notifier; + struct rw_semaphore sem; =20 #ifdef CONFIG_PINCTRL /* --=20 2.37.2