All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Bluetooth: Fix append max 11 bytes of name to scan rsp data
@ 2016-10-18 14:53 Michał Narajowski
  2016-10-18 17:58 ` Marcel Holtmann
  0 siblings, 1 reply; 4+ messages in thread
From: Michał Narajowski @ 2016-10-18 14:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michał Narajowski

Append maximum of 10 + 1 bytes of name to scan response data.
Complete name is appended only if exists and is <= 10 characters.
Else append short name if exists or shorten complete name if not.
This makes sure name is consistent across multiple advertising
instances.

Signed-off-by: Michał Narajowski <michal.narajowski@codecoup.pl>
---
 net/bluetooth/hci_request.c | 47 +++++++++++++++++++++------------------------
 net/bluetooth/mgmt.c        |  4 ++--
 2 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index e228842..cfdd2c8 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -971,39 +971,36 @@ void __hci_req_enable_advertising(struct hci_request *req)
 
 static u8 append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len)
 {
-	size_t complete_len;
 	size_t short_len;
-	int max_len;
-
-	max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
-	complete_len = strlen(hdev->dev_name);
-	short_len = strlen(hdev->short_name);
-
-	/* no space left for name */
-	if (max_len < 1)
-		return ad_len;
+	size_t complete_len;
 
-	/* no name set */
-	if (!complete_len)
+	/* no space left for name (+ NULL + type + len) */
+	if ((HCI_MAX_AD_LENGTH - ad_len) < HCI_MAX_SHORT_NAME_LENGTH + 3)
 		return ad_len;
 
-	/* complete name fits and is eq to max short name len or smaller */
-	if (complete_len <= max_len &&
-	    complete_len <= HCI_MAX_SHORT_NAME_LENGTH) {
+	/* use complete name if present and fits */
+	complete_len = strlen(hdev->dev_name);
+	if (complete_len && complete_len <= HCI_MAX_SHORT_NAME_LENGTH)
 		return eir_append_data(ptr, ad_len, EIR_NAME_COMPLETE,
-				       hdev->dev_name, complete_len);
-	}
+				       hdev->dev_name, complete_len + 1);
 
-	/* short name set and fits */
-	if (short_len && short_len <= max_len) {
+	/* use short name if present */
+	short_len = strlen(hdev->short_name);
+	if (short_len)
 		return eir_append_data(ptr, ad_len, EIR_NAME_SHORT,
-				       hdev->short_name, short_len);
-	}
+				       hdev->short_name, short_len + 1);
 
-	/* no short name set so shorten complete name */
-	if (!short_len) {
-		return eir_append_data(ptr, ad_len, EIR_NAME_SHORT,
-				       hdev->dev_name, max_len);
+	/* use shortened full name if present, we already know that name
+	 * is longer then HCI_MAX_SHORT_NAME_LENGTH
+	 */
+	if (complete_len) {
+		u8 name[HCI_MAX_SHORT_NAME_LENGTH + 1];
+
+		memcpy(name, hdev->dev_name, HCI_MAX_SHORT_NAME_LENGTH);
+		name[HCI_MAX_SHORT_NAME_LENGTH] = '\0';
+
+		return eir_append_data(ptr, ad_len, EIR_NAME_SHORT, name,
+				       sizeof(name));
 	}
 
 	return ad_len;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7360380..cdcadca 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6030,9 +6030,9 @@ static u8 tlv_data_max_len(u32 adv_flags, bool is_adv_data)
 		if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
 			max_len -= 3;
 	} else {
-		/* at least 1 byte of name should fit in */
+		/* max 11 bytes of name should fit in */
 		if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
-			max_len -= 3;
+			max_len -= (1 + 1 + 11);
 
 		if (adv_flags & (MGMT_ADV_FLAG_APPEARANCE))
 			max_len -= 4;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] Bluetooth: Fix append max 11 bytes of name to scan rsp data
  2016-10-18 14:53 [PATCH v2] Bluetooth: Fix append max 11 bytes of name to scan rsp data Michał Narajowski
@ 2016-10-18 17:58 ` Marcel Holtmann
  2016-10-18 19:27   ` Szymon Janc
  0 siblings, 1 reply; 4+ messages in thread
From: Marcel Holtmann @ 2016-10-18 17:58 UTC (permalink / raw)
  To: Michał Narajowski; +Cc: linux-bluetooth

Hi Michal,

> Append maximum of 10 + 1 bytes of name to scan response data.
> Complete name is appended only if exists and is <= 10 characters.
> Else append short name if exists or shorten complete name if not.
> This makes sure name is consistent across multiple advertising
> instances.
> 
> Signed-off-by: Michał Narajowski <michal.narajowski@codecoup.pl>
> ---
> net/bluetooth/hci_request.c | 47 +++++++++++++++++++++------------------------
> net/bluetooth/mgmt.c        |  4 ++--
> 2 files changed, 24 insertions(+), 27 deletions(-)
> 
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index e228842..cfdd2c8 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -971,39 +971,36 @@ void __hci_req_enable_advertising(struct hci_request *req)
> 
> static u8 append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len)
> {
> -	size_t complete_len;
> 	size_t short_len;
> -	int max_len;
> -
> -	max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
> -	complete_len = strlen(hdev->dev_name);
> -	short_len = strlen(hdev->short_name);
> -
> -	/* no space left for name */
> -	if (max_len < 1)
> -		return ad_len;
> +	size_t complete_len;
> 
> -	/* no name set */
> -	if (!complete_len)
> +	/* no space left for name (+ NULL + type + len) */
> +	if ((HCI_MAX_AD_LENGTH - ad_len) < HCI_MAX_SHORT_NAME_LENGTH + 3)
> 		return ad_len;
> 
> -	/* complete name fits and is eq to max short name len or smaller */
> -	if (complete_len <= max_len &&
> -	    complete_len <= HCI_MAX_SHORT_NAME_LENGTH) {
> +	/* use complete name if present and fits */
> +	complete_len = strlen(hdev->dev_name);
> +	if (complete_len && complete_len <= HCI_MAX_SHORT_NAME_LENGTH)
> 		return eir_append_data(ptr, ad_len, EIR_NAME_COMPLETE,
> -				       hdev->dev_name, complete_len);
> -	}
> +				       hdev->dev_name, complete_len + 1);
> 
> -	/* short name set and fits */
> -	if (short_len && short_len <= max_len) {
> +	/* use short name if present */
> +	short_len = strlen(hdev->short_name);
> +	if (short_len)
> 		return eir_append_data(ptr, ad_len, EIR_NAME_SHORT,
> -				       hdev->short_name, short_len);
> -	}
> +				       hdev->short_name, short_len + 1);
> 
> -	/* no short name set so shorten complete name */
> -	if (!short_len) {
> -		return eir_append_data(ptr, ad_len, EIR_NAME_SHORT,
> -				       hdev->dev_name, max_len);
> +	/* use shortened full name if present, we already know that name
> +	 * is longer then HCI_MAX_SHORT_NAME_LENGTH
> +	 */
> +	if (complete_len) {
> +		u8 name[HCI_MAX_SHORT_NAME_LENGTH + 1];
> +
> +		memcpy(name, hdev->dev_name, HCI_MAX_SHORT_NAME_LENGTH);
> +		name[HCI_MAX_SHORT_NAME_LENGTH] = '\0';
> +
> +		return eir_append_data(ptr, ad_len, EIR_NAME_SHORT, name,
> +				       sizeof(name));
> 	}
> 
> 	return ad_len;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 7360380..cdcadca 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -6030,9 +6030,9 @@ static u8 tlv_data_max_len(u32 adv_flags, bool is_adv_data)
> 		if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
> 			max_len -= 3;
> 	} else {
> -		/* at least 1 byte of name should fit in */
> +		/* max 11 bytes of name should fit in */
> 		if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
> -			max_len -= 3;
> +			max_len -= (1 + 1 + 11);

actually this one should also return correct values and not just max.

If either the full_name or short_name is shorter than 11 octets, then only that value should be returned instead of always 11 octets.

Regards

Marcel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] Bluetooth: Fix append max 11 bytes of name to scan rsp data
  2016-10-18 17:58 ` Marcel Holtmann
@ 2016-10-18 19:27   ` Szymon Janc
  2016-10-18 19:39     ` Marcel Holtmann
  0 siblings, 1 reply; 4+ messages in thread
From: Szymon Janc @ 2016-10-18 19:27 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Michał Narajowski, linux-bluetooth

Hi Marcel,

On Tue, Oct 18, 2016 at 7:58 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Michal,
>
>> Append maximum of 10 + 1 bytes of name to scan response data.
>> Complete name is appended only if exists and is <=3D 10 characters.
>> Else append short name if exists or shorten complete name if not.
>> This makes sure name is consistent across multiple advertising
>> instances.
>>
>> Signed-off-by: Micha=C5=82 Narajowski <michal.narajowski@codecoup.pl>
>> ---
>> net/bluetooth/hci_request.c | 47 +++++++++++++++++++++------------------=
------
>> net/bluetooth/mgmt.c        |  4 ++--
>> 2 files changed, 24 insertions(+), 27 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
>> index e228842..cfdd2c8 100644
>> --- a/net/bluetooth/hci_request.c
>> +++ b/net/bluetooth/hci_request.c
>> @@ -971,39 +971,36 @@ void __hci_req_enable_advertising(struct hci_reque=
st *req)
>>
>> static u8 append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len)
>> {
>> -     size_t complete_len;
>>       size_t short_len;
>> -     int max_len;
>> -
>> -     max_len =3D HCI_MAX_AD_LENGTH - ad_len - 2;
>> -     complete_len =3D strlen(hdev->dev_name);
>> -     short_len =3D strlen(hdev->short_name);
>> -
>> -     /* no space left for name */
>> -     if (max_len < 1)
>> -             return ad_len;
>> +     size_t complete_len;
>>
>> -     /* no name set */
>> -     if (!complete_len)
>> +     /* no space left for name (+ NULL + type + len) */
>> +     if ((HCI_MAX_AD_LENGTH - ad_len) < HCI_MAX_SHORT_NAME_LENGTH + 3)
>>               return ad_len;
>>
>> -     /* complete name fits and is eq to max short name len or smaller *=
/
>> -     if (complete_len <=3D max_len &&
>> -         complete_len <=3D HCI_MAX_SHORT_NAME_LENGTH) {
>> +     /* use complete name if present and fits */
>> +     complete_len =3D strlen(hdev->dev_name);
>> +     if (complete_len && complete_len <=3D HCI_MAX_SHORT_NAME_LENGTH)
>>               return eir_append_data(ptr, ad_len, EIR_NAME_COMPLETE,
>> -                                    hdev->dev_name, complete_len);
>> -     }
>> +                                    hdev->dev_name, complete_len + 1);
>>
>> -     /* short name set and fits */
>> -     if (short_len && short_len <=3D max_len) {
>> +     /* use short name if present */
>> +     short_len =3D strlen(hdev->short_name);
>> +     if (short_len)
>>               return eir_append_data(ptr, ad_len, EIR_NAME_SHORT,
>> -                                    hdev->short_name, short_len);
>> -     }
>> +                                    hdev->short_name, short_len + 1);
>>
>> -     /* no short name set so shorten complete name */
>> -     if (!short_len) {
>> -             return eir_append_data(ptr, ad_len, EIR_NAME_SHORT,
>> -                                    hdev->dev_name, max_len);
>> +     /* use shortened full name if present, we already know that name
>> +      * is longer then HCI_MAX_SHORT_NAME_LENGTH
>> +      */
>> +     if (complete_len) {
>> +             u8 name[HCI_MAX_SHORT_NAME_LENGTH + 1];
>> +
>> +             memcpy(name, hdev->dev_name, HCI_MAX_SHORT_NAME_LENGTH);
>> +             name[HCI_MAX_SHORT_NAME_LENGTH] =3D '\0';
>> +
>> +             return eir_append_data(ptr, ad_len, EIR_NAME_SHORT, name,
>> +                                    sizeof(name));
>>       }
>>
>>       return ad_len;
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 7360380..cdcadca 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -6030,9 +6030,9 @@ static u8 tlv_data_max_len(u32 adv_flags, bool is_=
adv_data)
>>               if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
>>                       max_len -=3D 3;
>>       } else {
>> -             /* at least 1 byte of name should fit in */
>> +             /* max 11 bytes of name should fit in */
>>               if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
>> -                     max_len -=3D 3;
>> +                     max_len -=3D (1 + 1 + 11);
>
> actually this one should also return correct values and not just max.
>
> If either the full_name or short_name is shorter than 11 octets, then onl=
y that value should be returned instead of always 11 octets.

This would mean that if name is changed userspace will have to query
this again (and track name changes from mgmt). If that is the case we
should probably document this behavior.

--=20
pozdrawiam
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] Bluetooth: Fix append max 11 bytes of name to scan rsp data
  2016-10-18 19:27   ` Szymon Janc
@ 2016-10-18 19:39     ` Marcel Holtmann
  0 siblings, 0 replies; 4+ messages in thread
From: Marcel Holtmann @ 2016-10-18 19:39 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Michał Narajowski, linux-bluetooth

Hi Szymon,

>>> Append maximum of 10 + 1 bytes of name to scan response data.
>>> Complete name is appended only if exists and is <= 10 characters.
>>> Else append short name if exists or shorten complete name if not.
>>> This makes sure name is consistent across multiple advertising
>>> instances.
>>> 
>>> Signed-off-by: Michał Narajowski <michal.narajowski@codecoup.pl>
>>> ---
>>> net/bluetooth/hci_request.c | 47 +++++++++++++++++++++------------------------
>>> net/bluetooth/mgmt.c        |  4 ++--
>>> 2 files changed, 24 insertions(+), 27 deletions(-)
>>> 
>>> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
>>> index e228842..cfdd2c8 100644
>>> --- a/net/bluetooth/hci_request.c
>>> +++ b/net/bluetooth/hci_request.c
>>> @@ -971,39 +971,36 @@ void __hci_req_enable_advertising(struct hci_request *req)
>>> 
>>> static u8 append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len)
>>> {
>>> -     size_t complete_len;
>>>      size_t short_len;
>>> -     int max_len;
>>> -
>>> -     max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
>>> -     complete_len = strlen(hdev->dev_name);
>>> -     short_len = strlen(hdev->short_name);
>>> -
>>> -     /* no space left for name */
>>> -     if (max_len < 1)
>>> -             return ad_len;
>>> +     size_t complete_len;
>>> 
>>> -     /* no name set */
>>> -     if (!complete_len)
>>> +     /* no space left for name (+ NULL + type + len) */
>>> +     if ((HCI_MAX_AD_LENGTH - ad_len) < HCI_MAX_SHORT_NAME_LENGTH + 3)
>>>              return ad_len;
>>> 
>>> -     /* complete name fits and is eq to max short name len or smaller */
>>> -     if (complete_len <= max_len &&
>>> -         complete_len <= HCI_MAX_SHORT_NAME_LENGTH) {
>>> +     /* use complete name if present and fits */
>>> +     complete_len = strlen(hdev->dev_name);
>>> +     if (complete_len && complete_len <= HCI_MAX_SHORT_NAME_LENGTH)
>>>              return eir_append_data(ptr, ad_len, EIR_NAME_COMPLETE,
>>> -                                    hdev->dev_name, complete_len);
>>> -     }
>>> +                                    hdev->dev_name, complete_len + 1);
>>> 
>>> -     /* short name set and fits */
>>> -     if (short_len && short_len <= max_len) {
>>> +     /* use short name if present */
>>> +     short_len = strlen(hdev->short_name);
>>> +     if (short_len)
>>>              return eir_append_data(ptr, ad_len, EIR_NAME_SHORT,
>>> -                                    hdev->short_name, short_len);
>>> -     }
>>> +                                    hdev->short_name, short_len + 1);
>>> 
>>> -     /* no short name set so shorten complete name */
>>> -     if (!short_len) {
>>> -             return eir_append_data(ptr, ad_len, EIR_NAME_SHORT,
>>> -                                    hdev->dev_name, max_len);
>>> +     /* use shortened full name if present, we already know that name
>>> +      * is longer then HCI_MAX_SHORT_NAME_LENGTH
>>> +      */
>>> +     if (complete_len) {
>>> +             u8 name[HCI_MAX_SHORT_NAME_LENGTH + 1];
>>> +
>>> +             memcpy(name, hdev->dev_name, HCI_MAX_SHORT_NAME_LENGTH);
>>> +             name[HCI_MAX_SHORT_NAME_LENGTH] = '\0';
>>> +
>>> +             return eir_append_data(ptr, ad_len, EIR_NAME_SHORT, name,
>>> +                                    sizeof(name));
>>>      }
>>> 
>>>      return ad_len;
>>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>>> index 7360380..cdcadca 100644
>>> --- a/net/bluetooth/mgmt.c
>>> +++ b/net/bluetooth/mgmt.c
>>> @@ -6030,9 +6030,9 @@ static u8 tlv_data_max_len(u32 adv_flags, bool is_adv_data)
>>>              if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
>>>                      max_len -= 3;
>>>      } else {
>>> -             /* at least 1 byte of name should fit in */
>>> +             /* max 11 bytes of name should fit in */
>>>              if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
>>> -                     max_len -= 3;
>>> +                     max_len -= (1 + 1 + 11);
>> 
>> actually this one should also return correct values and not just max.
>> 
>> If either the full_name or short_name is shorter than 11 octets, then only that value should be returned instead of always 11 octets.
> 
> This would mean that if name is changed userspace will have to query
> this again (and track name changes from mgmt). If that is the case we
> should probably document this behavior.

that is what I would expect. And yes, documenting this would be a good idea.

Regards

Marcel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-10-18 19:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-18 14:53 [PATCH v2] Bluetooth: Fix append max 11 bytes of name to scan rsp data Michał Narajowski
2016-10-18 17:58 ` Marcel Holtmann
2016-10-18 19:27   ` Szymon Janc
2016-10-18 19:39     ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.