| Message ID | 20110824112745.GB5975@shale.localdomain (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Wed, Aug 24, 2011 at 1:27 PM, Dan Carpenter <error27@gmail.com> wrote: > We recently introduced locking into this function, but we missed an > error path which needs an unlock. > > Signed-off-by: Dan Carpenter <error27@gmail.com> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 582be00..ed0cd09 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1643,8 +1643,10 @@ static int hid_device_probe(struct device *dev) > > if (!hdev->driver) { > id = hid_match_device(hdev, hdrv); > - if (id == NULL) > - return -ENODEV; > + if (id == NULL) { > + ret = -ENODEV; > + goto unlock; > + } > > hdev->driver = hdrv; > if (hdrv->probe) { > @@ -1657,7 +1659,7 @@ static int hid_device_probe(struct device *dev) > if (ret) > hdev->driver = NULL; > } > - > +unlock: > up(&hdev->driver_lock); > return ret; > } > How could I miss that... Thanks. I tested this locking patch only with one driver so this problem did not occur. However, using two HID devices will deadlock the drivers. Nice catch. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 24 Aug 2011, Dan Carpenter wrote: > We recently introduced locking into this function, but we missed an > error path which needs an unlock. > > Signed-off-by: Dan Carpenter <error27@gmail.com> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 582be00..ed0cd09 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1643,8 +1643,10 @@ static int hid_device_probe(struct device *dev) > > if (!hdev->driver) { > id = hid_match_device(hdev, hdrv); > - if (id == NULL) > - return -ENODEV; > + if (id == NULL) { > + ret = -ENODEV; > + goto unlock; > + } > > hdev->driver = hdrv; > if (hdrv->probe) { > @@ -1657,7 +1659,7 @@ static int hid_device_probe(struct device *dev) > if (ret) > hdev->driver = NULL; > } > - > +unlock: > up(&hdev->driver_lock); > return ret; > } Good catch Dan, thanks. Applied.
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 582be00..ed0cd09 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1643,8 +1643,10 @@ static int hid_device_probe(struct device *dev) if (!hdev->driver) { id = hid_match_device(hdev, hdrv); - if (id == NULL) - return -ENODEV; + if (id == NULL) { + ret = -ENODEV; + goto unlock; + } hdev->driver = hdrv; if (hdrv->probe) { @@ -1657,7 +1659,7 @@ static int hid_device_probe(struct device *dev) if (ret) hdev->driver = NULL; } - +unlock: up(&hdev->driver_lock); return ret; }
We recently introduced locking into this function, but we missed an error path which needs an unlock. Signed-off-by: Dan Carpenter <error27@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html