On 07/12/2013 04:40 PM, Gianluca Anzolin wrote:
rfcomm_dev_add doesn't call module_put() in its error path when it took a
reference.
Good catch.
Fix by always calling __module_get() and module_put() in the error path.
Signed-off-by: Gianluca Anzolin <gianluca@xxxxxxxxxxxxxx>
---
net/bluetooth/rfcomm/tty.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 7bc603a..40288c3 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -357,11 +357,11 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
rfcomm_dlc_unlock(dlc);
+out:
/* It's safe to call __module_get() here because socket already
holds reference to this module. */
__module_get(THIS_MODULE);
-out:
spin_unlock(&rfcomm_dev_lock);
if (err < 0)
Is it possible to reverse the order-of-operations here:
spin_unlock(&rfcomm_dev_lock);
__module_get(THIS_MODULE);
Then the out: label could be moved to the function exit as noted below.
The rfcomm_dev_lock use here is what creates the mess in rfcomm_dev_state_change().
Anyone know why the rfcomm_dev_lock critical section is so ridiculously large
here? Does it really protect the module count??
@@ -386,6 +386,7 @@ out:
return dev->id;
free:
+ module_put(THIS_MODULE);
out:
spin_unlock(&rfcomm_dev_lock);
kfree(dev);
return err;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
