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
