Messages in this thread | | | Date | Wed, 30 Nov 2022 14:01:59 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v4 2/2] gpiolib: protect the GPIO device against being dropped while in use by user-space |
| |
On Wed, Nov 30, 2022 at 10:05:56AM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > 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.
Bikeshedding below and one question. (As per tag I'm fine with this version anyway)
...
> +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 *);
<bikeshedding> It's only 84 is on a single line.
Dunno if it's better to have typedef followed by wrapper pairs rather than all typedefs and wrappers grouped. </bikeshedding>
> +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);
Thinking more about this, wouldn't be better to actually
ret = down_read_trylock(&gdev->sem); if (ret) return ret;
?
> + ret = func(file, wait); > + up_read(&gdev->sem); > + > + return ret; > +}
-- With Best Regards, Andy Shevchenko
| |