Hi Bartosz,
I love your patch! Yet something to improve:
[auto build test ERROR on brgl/gpio/for-next]
[also build test ERROR on linus/master v6.1-rc7 next-20221128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpiolib-don-t-allow-user-space-to-crash-the-kernel-with-hot-unplugs/20221129-021037
base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link: https://lore.kernel.org/r/20221128175214.602612-3-brgl%40bgdev.pl
patch subject: [PATCH v2 2/2] gpiolib: protect the GPIO device against being dropped while in use by user-space
config: arm-randconfig-r046-20221128
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/ccf2821b643c23a5ef6fac8a4785c2127d4125c7
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Bartosz-Golaszewski/gpiolib-don-t-allow-user-space-to-crash-the-kernel-with-hot-unplugs/20221129-021037
git checkout ccf2821b643c23a5ef6fac8a4785c2127d4125c7
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/gpio/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
drivers/gpio/gpiolib-cdev.c: In function 'lineinfo_watch_read':
>> drivers/gpio/gpiolib-cdev.c:2647:1: error: expected 'while' before 'static'
2647 | static int gpio_chrdev_open(struct inode *inode, struct file *file)
| ^~~~~~
>> drivers/gpio/gpiolib-cdev.c:2709:12: error: invalid storage class for function 'gpio_chrdev_release'
2709 | static int gpio_chrdev_release(struct inode *inode, struct file *file)
| ^~~~~~~~~~~~~~~~~~~
>> drivers/gpio/gpiolib-cdev.c:2709:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
2709 | static int gpio_chrdev_release(struct inode *inode, struct file *file)
| ^~~~~~
>> drivers/gpio/gpiolib-cdev.c:2724:20: error: initializer element is not constant
2724 | .release = gpio_chrdev_release,
| ^~~~~~~~~~~~~~~~~~~
drivers/gpio/gpiolib-cdev.c:2724:20: note: (near initialization for 'gpio_fileops.release')
>> drivers/gpio/gpiolib-cdev.c:2725:17: error: 'gpio_chrdev_open' undeclared (first use in this function); did you mean 'gpio_chardev_data'?
2725 | .open = gpio_chrdev_open,
| ^~~~~~~~~~~~~~~~
| gpio_chardev_data
drivers/gpio/gpiolib-cdev.c:2725:17: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpio/gpiolib-cdev.c:2757:1: error: expected declaration or statement at end of input
2757 | }
| ^
drivers/gpio/gpiolib-cdev.c: At top level:
drivers/gpio/gpiolib-cdev.c:2754:6: warning: 'gpiolib_cdev_unregister' defined but not used [-Wunused-function]
2754 | void gpiolib_cdev_unregister(struct gpio_device *gdev)
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/gpio/gpiolib-cdev.c:2736:5: warning: 'gpiolib_cdev_register' defined but not used [-Wunused-function]
2736 | int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
| ^~~~~~~~~~~~~~~~~~~~~
drivers/gpio/gpiolib-cdev.c:2543:16: warning: 'lineinfo_watch_read' defined but not used [-Wunused-function]
2543 | static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
| ^~~~~~~~~~~~~~~~~~~
drivers/gpio/gpiolib-cdev.c:2494:12: warning: 'lineinfo_changed_notify' defined but not used [-Wunused-function]
2494 | static int lineinfo_changed_notify(struct notifier_block *nb,
| ^~~~~~~~~~~~~~~~~~~~~~~
vim +2647 drivers/gpio/gpiolib-cdev.c
925ca36913fc7df Kent Gibson 2020-06-16 2640
925ca36913fc7df Kent Gibson 2020-06-16 2641 /**
925ca36913fc7df Kent Gibson 2020-06-16 2642 * gpio_chrdev_open() - open the chardev for ioctl operations
925ca36913fc7df Kent Gibson 2020-06-16 2643 * @inode: inode for this chardev
49bc52798d7bb66 Kent Gibson 2020-07-08 2644 * @file: file struct for storing private data
925ca36913fc7df Kent Gibson 2020-06-16 2645 * Returns 0 on success
925ca36913fc7df Kent Gibson 2020-06-16 2646 */
49bc52798d7bb66 Kent Gibson 2020-07-08 @2647 static int gpio_chrdev_open(struct inode *inode, struct file *file)
925ca36913fc7df Kent Gibson 2020-06-16 2648 {
925ca36913fc7df Kent Gibson 2020-06-16 2649 struct gpio_device *gdev = container_of(inode->i_cdev,
925ca36913fc7df Kent Gibson 2020-06-16 2650 struct gpio_device, chrdev);
e2b781c5f0dd45f Kent Gibson 2020-07-08 2651 struct gpio_chardev_data *cdev;
925ca36913fc7df Kent Gibson 2020-06-16 2652 int ret = -ENOMEM;
925ca36913fc7df Kent Gibson 2020-06-16 2653
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2654 down_read(&gdev->sem);
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2655
925ca36913fc7df Kent Gibson 2020-06-16 2656 /* Fail on open if the backing gpiochip is gone */
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2657 if (!gdev->chip) {
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2658 up_read(&gdev->sem);
925ca36913fc7df Kent Gibson 2020-06-16 2659 return -ENODEV;
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2660 }
925ca36913fc7df Kent Gibson 2020-06-16 2661
e2b781c5f0dd45f Kent Gibson 2020-07-08 2662 cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2663 if (!cdev) {
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2664 up_read(&gdev->sem);
925ca36913fc7df Kent Gibson 2020-06-16 2665 return -ENOMEM;
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2666 }
925ca36913fc7df Kent Gibson 2020-06-16 2667
e2b781c5f0dd45f Kent Gibson 2020-07-08 2668 cdev->watched_lines = bitmap_zalloc(gdev->chip->ngpio, GFP_KERNEL);
e2b781c5f0dd45f Kent Gibson 2020-07-08 2669 if (!cdev->watched_lines)
e2b781c5f0dd45f Kent Gibson 2020-07-08 2670 goto out_free_cdev;
925ca36913fc7df Kent Gibson 2020-06-16 2671
e2b781c5f0dd45f Kent Gibson 2020-07-08 2672 init_waitqueue_head(&cdev->wait);
e2b781c5f0dd45f Kent Gibson 2020-07-08 2673 INIT_KFIFO(cdev->events);
e2b781c5f0dd45f Kent Gibson 2020-07-08 2674 cdev->gdev = gdev;
925ca36913fc7df Kent Gibson 2020-06-16 2675
e2b781c5f0dd45f Kent Gibson 2020-07-08 2676 cdev->lineinfo_changed_nb.notifier_call = lineinfo_changed_notify;
6accc376a7482ec Kent Gibson 2020-07-08 2677 ret = blocking_notifier_chain_register(&gdev->notifier,
e2b781c5f0dd45f Kent Gibson 2020-07-08 2678 &cdev->lineinfo_changed_nb);
925ca36913fc7df Kent Gibson 2020-06-16 2679 if (ret)
925ca36913fc7df Kent Gibson 2020-06-16 2680 goto out_free_bitmap;
925ca36913fc7df Kent Gibson 2020-06-16 2681
925ca36913fc7df Kent Gibson 2020-06-16 2682 get_device(&gdev->dev);
e2b781c5f0dd45f Kent Gibson 2020-07-08 2683 file->private_data = cdev;
925ca36913fc7df Kent Gibson 2020-06-16 2684
49bc52798d7bb66 Kent Gibson 2020-07-08 2685 ret = nonseekable_open(inode, file);
925ca36913fc7df Kent Gibson 2020-06-16 2686 if (ret)
925ca36913fc7df Kent Gibson 2020-06-16 2687 goto out_unregister_notifier;
925ca36913fc7df Kent Gibson 2020-06-16 2688
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2689 up_read(&gdev->sem);
925ca36913fc7df Kent Gibson 2020-06-16 2690 return ret;
925ca36913fc7df Kent Gibson 2020-06-16 2691
925ca36913fc7df Kent Gibson 2020-06-16 2692 out_unregister_notifier:
6accc376a7482ec Kent Gibson 2020-07-08 2693 blocking_notifier_chain_unregister(&gdev->notifier,
e2b781c5f0dd45f Kent Gibson 2020-07-08 2694 &cdev->lineinfo_changed_nb);
925ca36913fc7df Kent Gibson 2020-06-16 2695 out_free_bitmap:
e2b781c5f0dd45f Kent Gibson 2020-07-08 2696 bitmap_free(cdev->watched_lines);
e2b781c5f0dd45f Kent Gibson 2020-07-08 2697 out_free_cdev:
e2b781c5f0dd45f Kent Gibson 2020-07-08 2698 kfree(cdev);
ccf2821b643c23a Bartosz Golaszewski 2022-11-28 2699 up_read(&gdev->sem);
925ca36913fc7df Kent Gibson 2020-06-16 2700 return ret;
925ca36913fc7df Kent Gibson 2020-06-16 2701 }
925ca36913fc7df Kent Gibson 2020-06-16 2702
925ca36913fc7df Kent Gibson 2020-06-16 2703 /**
925ca36913fc7df Kent Gibson 2020-06-16 2704 * gpio_chrdev_release() - close chardev after ioctl operations
925ca36913fc7df Kent Gibson 2020-06-16 2705 * @inode: inode for this chardev
49bc52798d7bb66 Kent Gibson 2020-07-08 2706 * @file: file struct for storing private data
925ca36913fc7df Kent Gibson 2020-06-16 2707 * Returns 0 on success
925ca36913fc7df Kent Gibson 2020-06-16 2708 */
49bc52798d7bb66 Kent Gibson 2020-07-08 @2709 static int gpio_chrdev_release(struct inode *inode, struct file *file)
925ca36913fc7df Kent Gibson 2020-06-16 2710 {
e2b781c5f0dd45f Kent Gibson 2020-07-08 2711 struct gpio_chardev_data *cdev = file->private_data;
e2b781c5f0dd45f Kent Gibson 2020-07-08 2712 struct gpio_device *gdev = cdev->gdev;
925ca36913fc7df Kent Gibson 2020-06-16 2713
e2b781c5f0dd45f Kent Gibson 2020-07-08 2714 bitmap_free(cdev->watched_lines);
6accc376a7482ec Kent Gibson 2020-07-08 2715 blocking_notifier_chain_unregister(&gdev->notifier,
e2b781c5f0dd45f Kent Gibson 2020-07-08 2716 &cdev->lineinfo_changed_nb);
925ca36913fc7df Kent Gibson 2020-06-16 2717 put_device(&gdev->dev);
e2b781c5f0dd45f Kent Gibson 2020-07-08 2718 kfree(cdev);
925ca36913fc7df Kent Gibson 2020-06-16 2719
925ca36913fc7df Kent Gibson 2020-06-16 2720 return 0;
925ca36913fc7df Kent Gibson 2020-06-16 2721 }
925ca36913fc7df Kent Gibson 2020-06-16 2722
925ca36913fc7df Kent Gibson 2020-06-16 2723 static const struct file_operations gpio_fileops = {
925ca36913fc7df Kent Gibson 2020-06-16 @2724 .release = gpio_chrdev_release,
925ca36913fc7df Kent Gibson 2020-06-16 @2725 .open = gpio_chrdev_open,
925ca36913fc7df Kent Gibson 2020-06-16 2726 .poll = lineinfo_watch_poll,
925ca36913fc7df Kent Gibson 2020-06-16 2727 .read = lineinfo_watch_read,
925ca36913fc7df Kent Gibson 2020-06-16 2728 .owner = THIS_MODULE,
925ca36913fc7df Kent Gibson 2020-06-16 2729 .llseek = no_llseek,
925ca36913fc7df Kent Gibson 2020-06-16 2730 .unlocked_ioctl = gpio_ioctl,
925ca36913fc7df Kent Gibson 2020-06-16 2731 #ifdef CONFIG_COMPAT
925ca36913fc7df Kent Gibson 2020-06-16 2732 .compat_ioctl = gpio_ioctl_compat,
925ca36913fc7df Kent Gibson 2020-06-16 2733 #endif
925ca36913fc7df Kent Gibson 2020-06-16 2734 };
925ca36913fc7df Kent Gibson 2020-06-16 2735
925ca36913fc7df Kent Gibson 2020-06-16 2736 int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
925ca36913fc7df Kent Gibson 2020-06-16 2737 {
925ca36913fc7df Kent Gibson 2020-06-16 2738 int ret;
925ca36913fc7df Kent Gibson 2020-06-16 2739
925ca36913fc7df Kent Gibson 2020-06-16 2740 cdev_init(&gdev->chrdev, &gpio_fileops);
925ca36913fc7df Kent Gibson 2020-06-16 2741 gdev->chrdev.owner = THIS_MODULE;
925ca36913fc7df Kent Gibson 2020-06-16 2742 gdev->dev.devt = MKDEV(MAJOR(devt), gdev->id);
925ca36913fc7df Kent Gibson 2020-06-16 2743
925ca36913fc7df Kent Gibson 2020-06-16 2744 ret = cdev_device_add(&gdev->chrdev, &gdev->dev);
925ca36913fc7df Kent Gibson 2020-06-16 2745 if (ret)
925ca36913fc7df Kent Gibson 2020-06-16 2746 return ret;
925ca36913fc7df Kent Gibson 2020-06-16 2747
925ca36913fc7df Kent Gibson 2020-06-16 2748 chip_dbg(gdev->chip, "added GPIO chardev (%d:%d)\n",
925ca36913fc7df Kent Gibson 2020-06-16 2749 MAJOR(devt), gdev->id);
925ca36913fc7df Kent Gibson 2020-06-16 2750
925ca36913fc7df Kent Gibson 2020-06-16 2751 return 0;
925ca36913fc7df Kent Gibson 2020-06-16 2752 }
925ca36913fc7df Kent Gibson 2020-06-16 2753
925ca36913fc7df Kent Gibson 2020-06-16 2754 void gpiolib_cdev_unregister(struct gpio_device *gdev)
925ca36913fc7df Kent Gibson 2020-06-16 2755 {
925ca36913fc7df Kent Gibson 2020-06-16 2756 cdev_device_del(&gdev->chrdev, &gdev->dev);
925ca36913fc7df Kent Gibson 2020-06-16 @2757 }
@@ -200,10 +200,14 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
struct gpiohandle_data ghd;
DECLARE_BITMAP(vals, GPIOHANDLES_MAX);
unsigned int i;
- int ret;
+ int ret = 0;
+
+ down_read(&gdev->sem);
- if (!gdev->chip)
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return -ENODEV;
+ }
switch (cmd) {
case GPIOHANDLE_GET_LINE_VALUES_IOCTL:
@@ -212,43 +216,52 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
lh->num_descs, lh->descs,
NULL, vals);
if (ret)
- return ret;
+ break;
memset(&ghd, 0, sizeof(ghd));
for (i = 0; i < lh->num_descs; i++)
ghd.values[i] = test_bit(i, vals);
if (copy_to_user(ip, &ghd, sizeof(ghd)))
- return -EFAULT;
+ ret = -EFAULT;
- return 0;
+ break;
case GPIOHANDLE_SET_LINE_VALUES_IOCTL:
/*
* All line descriptors were created at once with the same
* flags so just check if the first one is really output.
*/
- if (!test_bit(FLAG_IS_OUT, &lh->descs[0]->flags))
- return -EPERM;
+ if (!test_bit(FLAG_IS_OUT, &lh->descs[0]->flags)) {
+ ret = -EPERM;
+ break;
+ }
- if (copy_from_user(&ghd, ip, sizeof(ghd)))
- return -EFAULT;
+ if (copy_from_user(&ghd, ip, sizeof(ghd))) {
+ ret = -EFAULT;
+ break;
+ }
/* Clamp all values to [0,1] */
for (i = 0; i < lh->num_descs; i++)
__assign_bit(i, vals, ghd.values[i]);
/* Reuse the array setting function */
- return gpiod_set_array_value_complex(false,
- true,
- lh->num_descs,
- lh->descs,
- NULL,
- vals);
+ ret = gpiod_set_array_value_complex(false,
+ true,
+ lh->num_descs,
+ lh->descs,
+ NULL,
+ vals);
+ break;
case GPIOHANDLE_SET_CONFIG_IOCTL:
- return linehandle_set_config(lh, ip);
+ ret = linehandle_set_config(lh, ip);
+ break;
default:
- return -EINVAL;
+ ret = -EINVAL;
}
+
+ up_read(&gdev->sem);
+ return ret;
}
#ifdef CONFIG_COMPAT
@@ -1388,20 +1401,31 @@ static long linereq_ioctl(struct file *file, unsigned int cmd,
struct linereq *lr = file->private_data;
struct gpio_device *gdev = lr->gdev;
void __user *ip = (void __user *)arg;
+ long ret;
+
+ down_read(&gdev->sem);
- if (!gdev->chip)
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return -ENODEV;
+ }
switch (cmd) {
case GPIO_V2_LINE_GET_VALUES_IOCTL:
- return linereq_get_values(lr, ip);
+ ret = linereq_get_values(lr, ip);
+ break;
case GPIO_V2_LINE_SET_VALUES_IOCTL:
- return linereq_set_values(lr, ip);
+ ret = linereq_set_values(lr, ip);
+ break;
case GPIO_V2_LINE_SET_CONFIG_IOCTL:
- return linereq_set_config(lr, ip);
+ ret = linereq_set_config(lr, ip);
+ break;
default:
- return -EINVAL;
+ ret = -EINVAL;
}
+
+ up_read(&gdev->sem);
+ return ret;
}
#ifdef CONFIG_COMPAT
@@ -1419,8 +1443,12 @@ static __poll_t linereq_poll(struct file *file,
struct gpio_device *gdev = lr->gdev;
__poll_t events = 0;
- if (!gdev->chip)
+ down_read(&gdev->sem);
+
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return 0;
+ }
poll_wait(file, &lr->wait, wait);
@@ -1428,6 +1456,7 @@ static __poll_t linereq_poll(struct file *file,
&lr->wait.lock))
events = EPOLLIN | EPOLLRDNORM;
+ up_read(&gdev->sem);
return events;
}
@@ -1442,22 +1471,30 @@ static ssize_t linereq_read(struct file *file,
ssize_t bytes_read = 0;
int ret;
- if (!gdev->chip)
+ down_read(&gdev->sem);
+
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return -ENODEV;
+ }
- if (count < sizeof(le))
+ if (count < sizeof(le)) {
+ up_read(&gdev->sem);
return -EINVAL;
+ }
do {
spin_lock(&lr->wait.lock);
if (kfifo_is_empty(&lr->events)) {
if (bytes_read) {
spin_unlock(&lr->wait.lock);
+ up_read(&gdev->sem);
return bytes_read;
}
if (file->f_flags & O_NONBLOCK) {
spin_unlock(&lr->wait.lock);
+ up_read(&gdev->sem);
return -EAGAIN;
}
@@ -1465,6 +1502,7 @@ static ssize_t linereq_read(struct file *file,
!kfifo_is_empty(&lr->events));
if (ret) {
spin_unlock(&lr->wait.lock);
+ up_read(&gdev->sem);
return ret;
}
}
@@ -1481,11 +1519,14 @@ static ssize_t linereq_read(struct file *file,
break;
}
- if (copy_to_user(buf + bytes_read, &le, sizeof(le)))
+ if (copy_to_user(buf + bytes_read, &le, sizeof(le))) {
+ up_read(&gdev->sem);
return -EFAULT;
+ }
bytes_read += sizeof(le);
} while (count >= bytes_read + sizeof(le));
+ up_read(&gdev->sem);
return bytes_read;
}
@@ -1733,14 +1774,19 @@ static __poll_t lineevent_poll(struct file *file,
struct gpio_device *gdev = le->gdev;
__poll_t events = 0;
- if (!gdev->chip)
+ down_read(&gdev->sem);
+
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return 0;
+ }
poll_wait(file, &le->wait, wait);
if (!kfifo_is_empty_spinlocked_noirqsave(&le->events, &le->wait.lock))
events = EPOLLIN | EPOLLRDNORM;
+ up_read(&gdev->sem);
return events;
}
@@ -1761,8 +1807,12 @@ static ssize_t lineevent_read(struct file *file,
ssize_t ge_size;
int ret;
- if (!gdev->chip)
+ down_read(&gdev->sem);
+
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return -ENODEV;
+ }
/*
* When compatible system call is being used the struct gpioevent_data,
@@ -1777,19 +1827,23 @@ static ssize_t lineevent_read(struct file *file,
ge_size = sizeof(struct compat_gpioeevent_data);
else
ge_size = sizeof(struct gpioevent_data);
- if (count < ge_size)
+ if (count < ge_size) {
+ up_read(&gdev->sem);
return -EINVAL;
+ }
do {
spin_lock(&le->wait.lock);
if (kfifo_is_empty(&le->events)) {
if (bytes_read) {
spin_unlock(&le->wait.lock);
+ up_read(&gdev->sem);
return bytes_read;
}
if (file->f_flags & O_NONBLOCK) {
spin_unlock(&le->wait.lock);
+ up_read(&gdev->sem);
return -EAGAIN;
}
@@ -1797,6 +1851,7 @@ static ssize_t lineevent_read(struct file *file,
!kfifo_is_empty(&le->events));
if (ret) {
spin_unlock(&le->wait.lock);
+ up_read(&gdev->sem);
return ret;
}
}
@@ -1813,11 +1868,14 @@ static ssize_t lineevent_read(struct file *file,
break;
}
- if (copy_to_user(buf + bytes_read, &ge, ge_size))
+ if (copy_to_user(buf + bytes_read, &ge, ge_size)) {
+ up_read(&gdev->sem);
return -EFAULT;
+ }
bytes_read += ge_size;
} while (count >= bytes_read + ge_size);
+ up_read(&gdev->sem);
return bytes_read;
}
@@ -1846,8 +1904,12 @@ static long lineevent_ioctl(struct file *file, unsigned int cmd,
void __user *ip = (void __user *)arg;
struct gpiohandle_data ghd;
- if (!gdev->chip)
+ down_read(&gdev->sem);
+
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return -ENODEV;
+ }
/*
* We can get the value for an event line but not set it,
@@ -1859,15 +1921,23 @@ static long lineevent_ioctl(struct file *file, unsigned int cmd,
memset(&ghd, 0, sizeof(ghd));
val = gpiod_get_value_cansleep(le->desc);
- if (val < 0)
+ if (val < 0) {
+ up_read(&gdev->sem);
return val;
+ }
+
ghd.values[0] = val;
- if (copy_to_user(ip, &ghd, sizeof(ghd)))
+ if (copy_to_user(ip, &ghd, sizeof(ghd))) {
+ up_read(&gdev->sem);
return -EFAULT;
+ }
+ up_read(&gdev->sem);
return 0;
}
+
+ up_read(&gdev->sem);
return -EINVAL;
}
@@ -2358,36 +2428,53 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct gpio_chardev_data *cdev = file->private_data;
struct gpio_device *gdev = cdev->gdev;
void __user *ip = (void __user *)arg;
+ long ret;
+
+ down_read(&gdev->sem);
/* We fail any subsequent ioctl():s when the chip is gone */
- if (!gdev->chip)
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return -ENODEV;
+ }
/* Fill in the struct and pass to userspace */
switch (cmd) {
case GPIO_GET_CHIPINFO_IOCTL:
- return chipinfo_get(cdev, ip);
+ ret = chipinfo_get(cdev, ip);
+ break;
#ifdef CONFIG_GPIO_CDEV_V1
case GPIO_GET_LINEHANDLE_IOCTL:
- return linehandle_create(gdev, ip);
+ ret = linehandle_create(gdev, ip);
+ break;
case GPIO_GET_LINEEVENT_IOCTL:
- return lineevent_create(gdev, ip);
+ ret = lineevent_create(gdev, ip);
+ break;
case GPIO_GET_LINEINFO_IOCTL:
- return lineinfo_get_v1(cdev, ip, false);
+ ret = lineinfo_get_v1(cdev, ip, false);
+ break;
case GPIO_GET_LINEINFO_WATCH_IOCTL:
- return lineinfo_get_v1(cdev, ip, true);
+ ret = lineinfo_get_v1(cdev, ip, true);
+ break;
#endif /* CONFIG_GPIO_CDEV_V1 */
case GPIO_V2_GET_LINEINFO_IOCTL:
- return lineinfo_get(cdev, ip, false);
+ ret = lineinfo_get(cdev, ip, false);
+ break;
case GPIO_V2_GET_LINEINFO_WATCH_IOCTL:
- return lineinfo_get(cdev, ip, true);
+ ret = lineinfo_get(cdev, ip, true);
+ break;
case GPIO_V2_GET_LINE_IOCTL:
- return linereq_create(gdev, ip);
+ ret = linereq_create(gdev, ip);
+ break;
case GPIO_GET_LINEINFO_UNWATCH_IOCTL:
- return lineinfo_unwatch(cdev, ip);
+ ret = lineinfo_unwatch(cdev, ip);
+ break;
default:
- return -EINVAL;
+ ret = -EINVAL;
}
+
+ up_read(&gdev->sem);
+ return ret;
}
#ifdef CONFIG_COMPAT
@@ -2436,8 +2523,12 @@ static __poll_t lineinfo_watch_poll(struct file *file,
struct gpio_device *gdev = cdev->gdev;
__poll_t events = 0;
- if (!gdev->chip)
+ down_read(&gdev->sem);
+
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return 0;
+ }
poll_wait(file, &cdev->wait, pollt);
@@ -2445,6 +2536,7 @@ static __poll_t lineinfo_watch_poll(struct file *file,
&cdev->wait.lock))
events = EPOLLIN | EPOLLRDNORM;
+ up_read(&gdev->sem);
return events;
}
@@ -2458,13 +2550,19 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
int ret;
size_t event_size;
- if (!gdev->chip)
+ down_read(&gdev->sem);
+
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return -ENODEV;
+ }
#ifndef CONFIG_GPIO_CDEV_V1
event_size = sizeof(struct gpio_v2_line_info_changed);
- if (count < event_size)
+ if (count < event_size) {
+ up_read(&gdev->sem);
return -EINVAL;
+ }
#endif
do {
@@ -2472,11 +2570,13 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
if (kfifo_is_empty(&cdev->events)) {
if (bytes_read) {
spin_unlock(&cdev->wait.lock);
+ up_read(&gdev->sem);
return bytes_read;
}
if (file->f_flags & O_NONBLOCK) {
spin_unlock(&cdev->wait.lock);
+ up_read(&gdev->sem);
return -EAGAIN;
}
@@ -2484,6 +2584,7 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
!kfifo_is_empty(&cdev->events));
if (ret) {
spin_unlock(&cdev->wait.lock);
+ up_read(&gdev->sem);
return ret;
}
}
@@ -2495,6 +2596,7 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
event_size = sizeof(struct gpioline_info_changed);
if (count < event_size) {
spin_unlock(&cdev->wait.lock);
+ up_read(&gdev->sem);
return -EINVAL;
}
#endif
@@ -2508,23 +2610,31 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
#ifdef CONFIG_GPIO_CDEV_V1
if (event_size == sizeof(struct gpio_v2_line_info_changed)) {
- if (copy_to_user(buf + bytes_read, &event, event_size))
+ if (copy_to_user(buf + bytes_read,
+ &event, event_size)) {
+ up_read(&gdev->sem);
return -EFAULT;
+ }
} else {
struct gpioline_info_changed event_v1;
gpio_v2_line_info_changed_to_v1(&event, &event_v1);
if (copy_to_user(buf + bytes_read, &event_v1,
- event_size))
+ event_size)) {
+ up_read(&gdev->sem);
return -EFAULT;
+ }
}
#else
- if (copy_to_user(buf + bytes_read, &event, event_size))
+ if (copy_to_user(buf + bytes_read, &event, event_size)) {
+ up_read(&gdev->sem);
return -EFAULT;
+
#endif
bytes_read += event_size;
} while (count >= bytes_read + sizeof(event));
+ up_read(&gdev->sem);
return bytes_read;
}
@@ -2541,13 +2651,19 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
struct gpio_chardev_data *cdev;
int ret = -ENOMEM;
+ down_read(&gdev->sem);
+
/* Fail on open if the backing gpiochip is gone */
- if (!gdev->chip)
+ if (!gdev->chip) {
+ up_read(&gdev->sem);
return -ENODEV;
+ }
cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
- if (!cdev)
+ if (!cdev) {
+ up_read(&gdev->sem);
return -ENOMEM;
+ }
cdev->watched_lines = bitmap_zalloc(gdev->chip->ngpio, GFP_KERNEL);
if (!cdev->watched_lines)
@@ -2570,6 +2686,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
if (ret)
goto out_unregister_notifier;
+ up_read(&gdev->sem);
return ret;
out_unregister_notifier:
@@ -2579,6 +2696,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
bitmap_free(cdev->watched_lines);
out_free_cdev:
kfree(cdev);
+ up_read(&gdev->sem);
return ret;
}
@@ -731,6 +731,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
spin_unlock_irqrestore(&gpio_lock, flags);
BLOCKING_INIT_NOTIFIER_HEAD(&gdev->notifier);
+ init_rwsem(&gdev->sem);
#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;
+ 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);
@@ -15,6 +15,7 @@
#include <linux/device.h>
#include <linux/module.h>
#include <linux/cdev.h>
+#include <linux/rwsem.h>
#define GPIOCHIP_NAME "gpiochip"
@@ -39,6 +40,9 @@
* @list: links gpio_device:s together for traversal
* @notifier: used to notify subscribers about lines being requested, released
* 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;
#ifdef CONFIG_PINCTRL
/*