Hi Gustavo On Mon, Oct 10, 2011 at 8:44 PM, Gustavo Padovan <padovan@xxxxxxxxxxxxxx> wrote: > Hi David, > > * David Herrmann <dh.herrmann@xxxxxxxxxxxxxx> [2011-10-08 14:58:47 +0200]: > >> We must not call device_del() if we didn't use device_add(). See module.c for >> comments on that. Therefore, we need to call device_initialize() when allocating >> the hci device and later device_add() instead of device_register(). >> >> This also fixes a bug when hci_register_dev() failed and we call hci_free_dev() >> without a valid core device. hci_free_dev() segfaults while calling put_device() >> on invalid memory. > > Please let me know if the following diff also fixes this problem. > It seems to fixes other issues like failing in usb_driver_claim_interface(). Could you elaborate more? I what way does may patch not fix this issue? > Gustavo > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index b84458d..ac446a7 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -922,9 +922,6 @@ EXPORT_SYMBOL(hci_alloc_dev); > void hci_free_dev(struct hci_dev *hdev) > { > skb_queue_purge(&hdev->driver_init); > - > - /* will free via device release */ > - put_device(&hdev->dev); This does not work. We need to drop a reference here, otherwise this function is totally useless except cleaning up the skb queue. In fact, this function will be called on an HCI device without having a reference. > } > EXPORT_SYMBOL(hci_free_dev); > > diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c > index 22f1a6c..1e5ccde 100644 > --- a/net/bluetooth/hci_sysfs.c > +++ b/net/bluetooth/hci_sysfs.c > @@ -587,7 +587,7 @@ void hci_unregister_sysfs(struct hci_dev *hdev) > > debugfs_remove_recursive(hdev->debugfs); > > - device_del(&hdev->dev); > + device_unregister(&hdev->dev); This drops the last reference in most cases and hence "hdev" will get deleted here and the caller (hci_unregister_dev) will fail cleaning up the device. The problem is, we actually have two reference counts. The device_get/device_put reference count (which actually cleans up the hdev device with kfree() when dropping the last reference) and the hdev->refcnt which is in charge of cleaning up the caller's data. We need to take a reference with device_get() in hci_alloc_dev and drop it in hci_free_dev, otherwise we have inconsistent behaviour. > } > > int __init bt_sysfs_init(void) > > Regards David -- 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