Hi Andre,
* Andre Guedes <andre.guedes@xxxxxxxxxxxxx> [2011-07-11 18:11:51 -0300]:
> According to the Bluetooth spec, the inquiry cancel command should
> only be issued after the inquiry command has been issued, a command
> status event has been received for the inquiry command, and before
> the inquiry complete event occurs.
>
> As HCI_INQUIRY flag is only set just after an inquiry command status
> event occurs and it is cleared just after an inquiry complete event
> occurs, the inquiry cancel command should be issued only if HCI_INQUIRY
> flag is set.
>
> This spec constraint raises two possible race condition we must handle.
>
> 1. A MGMT_OP_STOP_DISCOVERY command is issued just after a
> MGMT_OP_START_DISCOVERY. The controller didn't send the inquiry
> command status yet therefore the HCI_INQUIRY flag is not set. Thus,
> stop_discovery() will send no inquiry cancel command and the
> discovery procedure won't be stopped. This race is addressed by
> checking for pending MGMT_OP_STOP_DISCOVERY command in inquiry
> command status event handler.
>
> 2. While the MGMT_OP_STOP_DISCOVERY is being processed the controller
> sends the inquiry complete event and the HCI_INQUIRY flag is
> cleared. stop_discovery() will add a pending MGMT_OP_STOP_DISCOVERY
> command which won't be removed since there is no ongoing discovery.
> This race is addressed by synchronizing stop_discovery and inquiry
> complete event handler threads.
>
> Signed-off-by: Andre Guedes <andre.guedes@xxxxxxxxxxxxx>
> ---
> include/net/bluetooth/hci_core.h | 2 ++
> net/bluetooth/hci_event.c | 10 ++++++++++
> net/bluetooth/mgmt.c | 25 ++++++++++++++++++++++++-
> 3 files changed, 36 insertions(+), 1 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0ccd724..1ff59f2 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -861,6 +861,8 @@ int mgmt_start_discovery_complete(u16 index);
> int mgmt_start_discovery_failed(u16 index, u8 status);
> int mgmt_stop_discovery_complete(u16 index);
> int mgmt_stop_discovery_failed(u16 index, u8 status);
> +int mgmt_has_pending_stop_discov(u16 index);
> +int mgmt_cancel_discovery(u16 index);
>
> /* HCI info for socket */
> #define hci_pi(sk) ((struct hci_pinfo *) sk)
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index ea9e105..c75211c 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -963,6 +963,11 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
>
> set_bit(HCI_INQUIRY, &hdev->flags);
>
> + if (mgmt_has_pending_stop_discov(hdev->id)) {
Isn't a new bit flag log HCI_INQUIRY_CANCEL better that this? First this has
read a list, second it is a really ugly function name.
> + mgmt_cancel_discovery(hdev->id);
Just call hci_send_cmd(HCI_OP_INQUIRY_CANCEL)
> + return;
> + }
> +
> mgmt_discovering(hdev->id, 1);
> }
>
> @@ -1356,7 +1361,12 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
> hci_req_complete(hdev, HCI_OP_INQUIRY, status);
>
> mgmt_discovering(hdev->id, 0);
> +
> + hci_dev_lock(hdev);
> +
> mgmt_start_discovery_complete(hdev->id);
> +
> + hci_dev_unlock(hdev);
> }
>
> static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index e43940e..bbb0daa 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1670,6 +1670,21 @@ static int cancel_inquiry(struct hci_dev *hdev)
> return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
> }
>
> +int mgmt_cancel_discovery(u16 index)
> +{
> + struct hci_dev *hdev;
> + int res = 0;
> +
> + hdev = hci_dev_get(index);
> +
> + if (test_bit(HCI_INQUIRY, &hdev->flags))
> + res = cancel_inquiry(hdev);
> +
> + hci_dev_put(hdev);
> +
> + return res;
> +}
> +
> static int stop_discovery(struct sock *sk, u16 index)
> {
> struct hci_dev *hdev;
> @@ -1701,7 +1716,7 @@ static int stop_discovery(struct sock *sk, u16 index)
> goto failed;
> }
You can check here with mgmt_pending_find() if the HCI_OP_INQUIRY_CANCEL was
already issued or not. This simplifies code a bit.
Gustavo
--
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
