| Message ID | 20210505205319.201563-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: hid-core: Don't fail to remove a device on a signal | expand |
Hello, On Wed, May 05, 2021 at 10:53:19PM +0200, Uwe Kleine-König wrote: > The driver core ignores the return code of a remove function and > considers a device unbound unconditionally after .remove() returns. So > don't sleep interruptible while waiting on the mutex, as there is no > sane way to handle an interrupt. Just returning -EINTR as was done up to > now results in leaking resources because the hid driver's remove > callback (or hid_hw_stop()) isn't called. > > Fixes: 4ea5454203d9 ("HID: Fix race condition between driver core and ll-driver") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/hid/hid-core.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 0ae9f6df59d1..4095a4db623d 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -2303,12 +2303,9 @@ static int hid_device_remove(struct device *dev) > { > struct hid_device *hdev = to_hid_device(dev); > struct hid_driver *hdrv; > - int ret = 0; > > - if (down_interruptible(&hdev->driver_input_lock)) { > - ret = -EINTR; > - goto end; > - } > + down(&hdev->driver_input_lock); > + > hdev->io_started = false; > > hdrv = hdev->driver; > @@ -2323,8 +2320,8 @@ static int hid_device_remove(struct device *dev) > > if (!hdev->io_started) > up(&hdev->driver_input_lock); > -end: > - return ret; > + > + return 0; > } > > static ssize_t modalias_show(struct device *dev, struct device_attribute *a, apart from a whitespace difference and a different commti log this patch is the same as Dmitry's patch that became commit f2145f8dc566 ("HID: do not use down_interruptible() when unbinding devices"). So my patch can be disregarded. Best regards Uwe
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 0ae9f6df59d1..4095a4db623d 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2303,12 +2303,9 @@ static int hid_device_remove(struct device *dev) { struct hid_device *hdev = to_hid_device(dev); struct hid_driver *hdrv; - int ret = 0; - if (down_interruptible(&hdev->driver_input_lock)) { - ret = -EINTR; - goto end; - } + down(&hdev->driver_input_lock); + hdev->io_started = false; hdrv = hdev->driver; @@ -2323,8 +2320,8 @@ static int hid_device_remove(struct device *dev) if (!hdev->io_started) up(&hdev->driver_input_lock); -end: - return ret; + + return 0; } static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
The driver core ignores the return code of a remove function and considers a device unbound unconditionally after .remove() returns. So don't sleep interruptible while waiting on the mutex, as there is no sane way to handle an interrupt. Just returning -EINTR as was done up to now results in leaking resources because the hid driver's remove callback (or hid_hw_stop()) isn't called. Fixes: 4ea5454203d9 ("HID: Fix race condition between driver core and ll-driver") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/hid/hid-core.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)