Re: [PATCH 1/3] Bluetooth: Fix hci core device initialization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux