Commit bdbbae24 authored by Bartosz Golaszewski's avatar Bartosz Golaszewski
Browse files

gpiolib: protect the GPIO device against being dropped while in use by user-space 



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: d7c51b47 ("gpio: userspace ABI for reading/writing GPIO lines")
Fixes: 3c0d9c63 ("gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPIO_V2_LINE_GET_VALUES_IOCTL")
Fixes: aad95584 ("gpiolib: cdev: support GPIO_V2_GET_LINEINFO_IOCTL and GPIO_V2_GET_LINEINFO_WATCH_IOCTL")
Fixes: a54756cb ("gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL")
Fixes: 7b8e00d9 ("gpiolib: cdev: support GPIO_V2_LINE_SET_VALUES_IOCTL")
Signed-off-by: default avatarBartosz Golaszewski <bartosz.golaszewski@linaro.org>
[Nick: fixed a build failure with CDEV_V1 disabled]
Co-authored-by: default avatarNick Hainke <vincent@systemli.org>
Reviewed-by: default avatarKent Gibson <warthog618@gmail.com>
Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: default avatarLinus Walleij <linus.walleij@linaro.org>
parent 533aae7c

drivers/gpio/gpiolib-cdev.c

+152 −25
Original line number Diff line number Diff line
@@ -55,6 +55,50 @@ static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_values), 8)); 
 * interface to gpiolib GPIOs via ioctl()s.

 */



typedef __poll_t (*poll_fn)(struct file *, struct poll_table_struct *);

typedef long (*ioctl_fn)(struct file *, unsigned int, unsigned long);

typedef ssize_t (*read_fn)(struct file *, char __user *,

			   size_t count, loff_t *);



static __poll_t call_poll_locked(struct file *file,

				 struct poll_table_struct *wait,

				 struct gpio_device *gdev, poll_fn func)

{

	__poll_t ret;



	down_read(&gdev->sem);

	ret = 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,

			      ioctl_fn func)

{

	long ret;



	down_read(&gdev->sem);

	ret = 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, read_fn func)

{

	ssize_t ret;



	down_read(&gdev->sem);

	ret = func(file, buf, count, f_ps);

	up_read(&gdev->sem);



	return ret;

}



/*

 * GPIO line handle management

 */
@@ -191,7 +235,7 @@ static long linehandle_set_config(struct linehandle_state *lh, 
	return 0;

}



static long linehandle_ioctl(struct file *file, unsigned int cmd,

static long linehandle_ioctl_unlocked(struct file *file, unsigned int cmd,

				      unsigned long arg)

{

	struct linehandle_state *lh = file->private_data;
@@ -250,6 +294,15 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd, 
	}

}



static long linehandle_ioctl(struct file *file, unsigned int cmd,

			     unsigned long arg)

{

	struct linehandle_state *lh = 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,7 +1434,7 @@ static long linereq_set_config(struct linereq *lr, void __user *ip) 
	return ret;

}



static long linereq_ioctl(struct file *file, unsigned int cmd,

static long linereq_ioctl_unlocked(struct file *file, unsigned int cmd,

				   unsigned long arg)

{

	struct linereq *lr = file->private_data;
@@ -1402,6 +1455,15 @@ static long linereq_ioctl(struct file *file, unsigned int cmd, 
	}

}



static long linereq_ioctl(struct file *file, unsigned int cmd,

			  unsigned long arg)

{

	struct linereq *lr = 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,7 +1472,7 @@ static long linereq_ioctl_compat(struct file *file, unsigned int cmd, 
}

#endif



static __poll_t linereq_poll(struct file *file,

static __poll_t linereq_poll_unlocked(struct file *file,

				      struct poll_table_struct *wait)

{

	struct linereq *lr = file->private_data;
@@ -1428,10 +1490,16 @@ static __poll_t linereq_poll(struct file *file, 
	return events;

}



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 = 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 = file->private_data;

	struct gpio_v2_line_event le;
@@ -1485,6 +1553,15 @@ static ssize_t linereq_read(struct file *file, 
	return bytes_read;

}



static ssize_t linereq_read(struct file *file, char __user *buf,

			    size_t count, loff_t *f_ps)

{

	struct linereq *lr = 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,7 +1799,7 @@ struct lineevent_state { 
	(GPIOEVENT_REQUEST_RISING_EDGE | \

	GPIOEVENT_REQUEST_FALLING_EDGE)



static __poll_t lineevent_poll(struct file *file,

static __poll_t lineevent_poll_unlocked(struct file *file,

					struct poll_table_struct *wait)

{

	struct lineevent_state *le = file->private_data;
@@ -1739,15 +1816,21 @@ static __poll_t lineevent_poll(struct file *file, 
	return events;

}



static __poll_t lineevent_poll(struct file *file,

			       struct poll_table_struct *wait)

{

	struct lineevent_state *le = file->private_data;



	return call_poll_locked(file, wait, le->gdev, lineevent_poll_unlocked);

}



struct compat_gpioeevent_data {

	compat_u64	timestamp;

	u32		id;

};



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 = file->private_data;

	struct gpioevent_data ge;
@@ -1815,6 +1898,15 @@ static ssize_t lineevent_read(struct file *file, 
	return bytes_read;

}



static ssize_t lineevent_read(struct file *file, char __user *buf,

			      size_t count, loff_t *f_ps)

{

	struct lineevent_state *le = 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,7 +1924,7 @@ static int lineevent_release(struct inode *inode, struct file *file) 
	return 0;

}



static long lineevent_ioctl(struct file *file, unsigned int cmd,

static long lineevent_ioctl_unlocked(struct file *file, unsigned int cmd,

				     unsigned long arg)

{

	struct lineevent_state *le = file->private_data;
@@ -1864,6 +1956,15 @@ static long lineevent_ioctl(struct file *file, unsigned int cmd, 
	return -EINVAL;

}



static long lineevent_ioctl(struct file *file, unsigned int cmd,

			    unsigned long arg)

{

	struct lineevent_state *le = 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,7 +2523,7 @@ static int lineinfo_changed_notify(struct notifier_block *nb, 
	return NOTIFY_OK;

}



static __poll_t lineinfo_watch_poll(struct file *file,

static __poll_t lineinfo_watch_poll_unlocked(struct file *file,

					     struct poll_table_struct *pollt)

{

	struct gpio_chardev_data *cdev = file->private_data;
@@ -2440,7 +2541,16 @@ static __poll_t lineinfo_watch_poll(struct file *file, 
	return events;

}



static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,

static __poll_t lineinfo_watch_poll(struct file *file,

				    struct poll_table_struct *pollt)

{

	struct gpio_chardev_data *cdev = 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 = file->private_data;
@@ -2519,6 +2629,15 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf, 
	return bytes_read;

}



static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,

				   size_t count, loff_t *off)

{

	struct gpio_chardev_data *cdev = 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
@@ -2532,13 +2651,17 @@ 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)

		return -ENODEV;

	if (!gdev->chip) {

		ret = -ENODEV;

		goto out_unlock;

	}



	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);

	if (!cdev)

		return -ENOMEM;

		goto out_unlock;



	cdev->watched_lines = bitmap_zalloc(gdev->chip->ngpio, GFP_KERNEL);

	if (!cdev->watched_lines)
@@ -2561,6 +2684,8 @@ 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:
@@ -2570,6 +2695,8 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file) 
	bitmap_free(cdev->watched_lines);

out_free_cdev:

	kfree(cdev);

out_unlock:

	up_read(&gdev->sem);

	return ret;

}

drivers/gpio/gpiolib.c

+4 −0
Original line number Diff line number Diff line
@@ -790,6 +790,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);
@@ -924,6 +925,8 @@ 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);
@@ -958,6 +961,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);

drivers/gpio/gpiolib.h

+5 −0
Original line number Diff line number Diff line
@@ -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

	/*