[patch -next] HID: unlock on error path in hid_device_probe()
Subsystems:
hid core layer , the rest
5488d
3 messages,
3 authors,
2011-08-24 · open the first message on its own page
We recently introduced locking into this function, but we missed an
error path which needs an unlock.
Signed-off-by: Dan Carpenter <redacted>
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 ;
}
On Wed, Aug 24, 2011 at 1:27 PM, Dan Carpenter [off-list ref] wrote: quoted hunk We recently introduced locking into this function, but we missed an
error path which needs an unlock.
Signed-off-by: Dan Carpenter <redacted>
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:
quoted hunk We recently introduced locking into this function, but we missed an
error path which needs an unlock.
Signed-off-by: Dan Carpenter <redacted>
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.
--
Jiri Kosina
SUSE Labs