[PATCH 1/2] Bluetooth: Do not allow LE connection if LE is not enabled

[PATCH 1/2] Bluetooth: Do not allow LE connection if LE is not enabled

[Lukasz Rymanowski]

Kernel gives possibility to enable/disable LE host support.
There is flag HCI_LE_ENABLED which is set when this support is enabled
and some parts of the code checks this flag e.g. SMP
However it is still possible to make LE connection if LE Host support is
disabled, what might be confused for remote device.
This patch makes sure that kernel will not send HCI LE Create Connection
if LE HOST support is not enabled.

Signed-off-by: Lukasz Rymanowski <lukasz.rymanowski@tieto.com>
---
 net/bluetooth/hci_conn.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index c9b8fa5..3a89d8f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -733,6 +733,10 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 	struct hci_request req;
 	int err;
 
+	/* Let's make sure that le is enabled */
+	if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
+		return ERR_PTR(-EOPNOTSUPP);
+
 	/* Some devices send ATT messages as soon as the physical link is
 	 * established. To be able to handle these ATT messages, the user-
 	 * space first establishes the connection and then starts the pairing
-- 
1.8.4

[PATCH 2/2] Bluetooth: Use reject error code in pair device method

[Lukasz Rymanowski]

If user space is trying to do LE pair but LE has not been enabled then
MGMT_STATUS_REJECT will be returned.

Same result code will be returned also in case of BREDR pairing if BREDR
is not enabled.

Having separate error code for that scenario might be useful for
debugging at least.

Signed-off-by: Lukasz Rymanowski <lukasz.rymanowski@tieto.com>
---
 net/bluetooth/mgmt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 41e3005..e03e63c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3246,6 +3246,8 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 		if (PTR_ERR(conn) == -EBUSY)
 			status = MGMT_STATUS_BUSY;
+		else if (PTR_ERR(conn) == -EOPNOTSUPP)
+			status = MGMT_STATUS_REJECTED;
 		else
 			status = MGMT_STATUS_CONNECT_FAILED;
 
-- 
1.8.4

Re: [PATCH 2/2] Bluetooth: Use reject error code in pair device method

[Johan Hedberg]

Hi Lukasz,

On Fri, Jan 23, 2015, Lukasz Rymanowski wrote:
> If user space is trying to do LE pair but LE has not been enabled then
> MGMT_STATUS_REJECT will be returned.
> 
> Same result code will be returned also in case of BREDR pairing if BREDR
> is not enabled.
> 
> Having separate error code for that scenario might be useful for
> debugging at least.
> 
> Signed-off-by: Lukasz Rymanowski <lukasz.rymanowski@tieto.com>
> ---
>  net/bluetooth/mgmt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 41e3005..e03e63c 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3246,6 +3246,8 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
>  
>  		if (PTR_ERR(conn) == -EBUSY)
>  			status = MGMT_STATUS_BUSY;
> +		else if (PTR_ERR(conn) == -EOPNOTSUPP)
> +			status = MGMT_STATUS_REJECTED;
>  		else
>  			status = MGMT_STATUS_CONNECT_FAILED;

Good catch, but I'd like to keep this consistent with our handling of LE
support elsewhere. Particularly, I'd like to follow the same semantics
as the mgmt_le_support() helper function, meaning if the HW doesn't
support LE we get NOT_SUPPORTED whereas if it does support LE but it's
simply not enabled we get REJECTED.

You could e.g. use EOPNOTSUPP for non-LE HW and ECONNREFUSED for LE not
enabled (feel free to propose a better errno to mgmt status mapping).

Johan

Re: [PATCH 2/2] Bluetooth: Use reject error code in pair device method

[Lukasz Rymanowski]

Hi Johan

On 23 January 2015 at 13:14, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lukasz,
>
> On Fri, Jan 23, 2015, Lukasz Rymanowski wrote:
>> If user space is trying to do LE pair but LE has not been enabled then
>> MGMT_STATUS_REJECT will be returned.
>>
>> Same result code will be returned also in case of BREDR pairing if BREDR
>> is not enabled.
>>
>> Having separate error code for that scenario might be useful for
>> debugging at least.
>>
>> Signed-off-by: Lukasz Rymanowski <lukasz.rymanowski@tieto.com>
>> ---
>>  net/bluetooth/mgmt.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 41e3005..e03e63c 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -3246,6 +3246,8 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
>>
>>               if (PTR_ERR(conn) == -EBUSY)
>>                       status = MGMT_STATUS_BUSY;
>> +             else if (PTR_ERR(conn) == -EOPNOTSUPP)
>> +                     status = MGMT_STATUS_REJECTED;
>>               else
>>                       status = MGMT_STATUS_CONNECT_FAILED;
>
> Good catch, but I'd like to keep this consistent with our handling of LE
> support elsewhere. Particularly, I'd like to follow the same semantics
> as the mgmt_le_support() helper function, meaning if the HW doesn't
> support LE we get NOT_SUPPORTED whereas if it does support LE but it's
> simply not enabled we get REJECTED.
>
> You could e.g. use EOPNOTSUPP for non-LE HW and ECONNREFUSED for LE not
> enabled (feel free to propose a better errno to mgmt status mapping).
>
Ok.
I could not found any better error code than  ECONNREFUSED so I will go for it.

Also going with your proposal I should probably change hci_connect_acl
in the same way.
Meaning return EOPNOTSUPP if it is LE only chip and ECONNREFUSED if it
is dual but
BREDR is disabled.

Do you agree?

> Johan

Re: [PATCH 2/2] Bluetooth: Use reject error code in pair device method

[Johan Hedberg]

Hi Lukasz,

On Fri, Jan 23, 2015, Lukasz Rymanowski wrote:
> >>               if (PTR_ERR(conn) == -EBUSY)
> >>                       status = MGMT_STATUS_BUSY;
> >> +             else if (PTR_ERR(conn) == -EOPNOTSUPP)
> >> +                     status = MGMT_STATUS_REJECTED;
> >>               else
> >>                       status = MGMT_STATUS_CONNECT_FAILED;
> >
> > Good catch, but I'd like to keep this consistent with our handling of LE
> > support elsewhere. Particularly, I'd like to follow the same semantics
> > as the mgmt_le_support() helper function, meaning if the HW doesn't
> > support LE we get NOT_SUPPORTED whereas if it does support LE but it's
> > simply not enabled we get REJECTED.
> >
> > You could e.g. use EOPNOTSUPP for non-LE HW and ECONNREFUSED for LE not
> > enabled (feel free to propose a better errno to mgmt status mapping).
> >
> Ok.
> I could not found any better error code than  ECONNREFUSED so I will go for it.
> 
> Also going with your proposal I should probably change hci_connect_acl
> in the same way.
> Meaning return EOPNOTSUPP if it is LE only chip and ECONNREFUSED if it
> is dual but
> BREDR is disabled.
> 
> Do you agree?

Yes, sounds reasonable.

Johan