Linux-Bluetooth Archive on lore.kernel.org
 help / Atom feed
* Error setting UUIDs discovery filter on big endian systems
@ 2018-12-03 13:56 Matt
  2018-12-04 19:53 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Matt @ 2018-12-03 13:56 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

When setting a UUIDs discovery filter I am receiving 
"org.bluez.Error.InProgress" on starting a scan (which isn't in progress 
and will not start unless the discovery filter is cleared).

I am using bluez 5.50 (on openWRT). The problem seems to only appear on 
big endian hardware (tested a couple of recent kernel and bluez 
versions), the same software compiled for and tested on little endian 
hardware works as expected. The issue can be reproduced in the following 
way using bluetoothctl:

# /etc/init.d/bluetoothd restart
# bluetoothctl
[bluetooth]# power on
Changing power on succeeded
[CHG] Controller 00:1A:7D:DA:71:13 Powered: yes
[bluetooth]# menu scan
[bluetooth]# uuids 0000180f-0000-1000-8000-00805f9b34fb
[bluetooth]# back
[bluetooth]# scan on
SetDiscoveryFilter success
Failed to start discovery: org.bluez.Error.InProgress

Setting an rssi filter does work as expected on my big endian hardware, 
it is only the uuids filter that appears to show this problem. The issue 
appears whether set using bluetoothctl or directly using DBus.

Thank you for your help and attention,
Matt

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

* Re: Error setting UUIDs discovery filter on big endian systems
  2018-12-03 13:56 Error setting UUIDs discovery filter on big endian systems Matt
@ 2018-12-04 19:53 ` Luiz Augusto von Dentz
  2018-12-05  0:27   ` Matt
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2018-12-04 19:53 UTC (permalink / raw)
  To: mwtaylor; +Cc: linux-bluetooth

Hi Matt,
On Mon, Dec 3, 2018 at 3:59 PM Matt <mwtaylor@gmail.com> wrote:
>
> Hi,
>
> When setting a UUIDs discovery filter I am receiving
> "org.bluez.Error.InProgress" on starting a scan (which isn't in progress
> and will not start unless the discovery filter is cleared).
>
> I am using bluez 5.50 (on openWRT). The problem seems to only appear on
> big endian hardware (tested a couple of recent kernel and bluez
> versions), the same software compiled for and tested on little endian
> hardware works as expected. The issue can be reproduced in the following
> way using bluetoothctl:
>
> # /etc/init.d/bluetoothd restart
> # bluetoothctl
> [bluetooth]# power on
> Changing power on succeeded
> [CHG] Controller 00:1A:7D:DA:71:13 Powered: yes
> [bluetooth]# menu scan
> [bluetooth]# uuids 0000180f-0000-1000-8000-00805f9b34fb
> [bluetooth]# back
> [bluetooth]# scan on
> SetDiscoveryFilter success
> Failed to start discovery: org.bluez.Error.InProgress
>
> Setting an rssi filter does work as expected on my big endian hardware,
> it is only the uuids filter that appears to show this problem. The issue
> appears whether set using bluetoothctl or directly using DBus.

Do you have the bluetoothd logs when that happens? I wonder if it is
something with our string to UUID conversion.


-- 
Luiz Augusto von Dentz

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

* Re: Error setting UUIDs discovery filter on big endian systems
  2018-12-04 19:53 ` Luiz Augusto von Dentz
@ 2018-12-05  0:27   ` Matt
  2019-03-05 18:02     ` Matt
  0 siblings, 1 reply; 8+ messages in thread
From: Matt @ 2018-12-05  0:27 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On 04/12/2018 19:53, Luiz Augusto von Dentz wrote:
> Hi Matt,
> On Mon, Dec 3, 2018 at 3:59 PM Matt <mwtaylor@gmail.com> wrote:
>> Hi,
>>
>> When setting a UUIDs discovery filter I am receiving
>> "org.bluez.Error.InProgress" on starting a scan (which isn't in progress
>> and will not start unless the discovery filter is cleared).
>>
>> I am using bluez 5.50 (on openWRT). The problem seems to only appear on
>> big endian hardware (tested a couple of recent kernel and bluez
>> versions), the same software compiled for and tested on little endian
>> hardware works as expected. The issue can be reproduced in the following
>> way using bluetoothctl:
>>
>> # /etc/init.d/bluetoothd restart
>> # bluetoothctl
>> [bluetooth]# power on
>> Changing power on succeeded
>> [CHG] Controller 00:1A:7D:DA:71:13 Powered: yes
>> [bluetooth]# menu scan
>> [bluetooth]# uuids 0000180f-0000-1000-8000-00805f9b34fb
>> [bluetooth]# back
>> [bluetooth]# scan on
>> SetDiscoveryFilter success
>> Failed to start discovery: org.bluez.Error.InProgress
>>
>> Setting an rssi filter does work as expected on my big endian hardware,
>> it is only the uuids filter that appears to show this problem. The issue
>> appears whether set using bluetoothctl or directly using DBus.
> Do you have the bluetoothd logs when that happens? I wonder if it is
> something with our string to UUID conversion.
>
>
When performing the above sequence in bluetoothctl with 'bluetoothd -d' 
I get the following in my log:

daemon.debug bluetoothd[22991]: src/agent.c:agent_ref() 0xb03e60: ref=1
daemon.debug bluetoothd[22991]: src/agent.c:register_agent() agent :1.29
daemon.debug bluetoothd[22991]: src/adapter.c:property_set_mode() 
sending Set Powered command for index 0
daemon.debug bluetoothd[22991]: 
src/adapter.c:property_set_mode_complete() Success (0x00)
daemon.debug bluetoothd[22991]: src/adapter.c:new_settings_callback() 
Settings: 0x00000ad1
daemon.debug bluetoothd[22991]: src/adapter.c:settings_changed() Changed 
settings: 0x00000001
daemon.debug bluetoothd[22991]: src/adapter.c:adapter_start() adapter 
/org/bluez/hci0 has been enabled
daemon.debug bluetoothd[22991]: src/adapter.c:trigger_passive_scanning()
daemon.debug bluetoothd[22991]: src/adapter.c:set_discovery_filter() 
sender :1.29
daemon.debug bluetoothd[22991]: 
src/adapter.c:parse_discovery_filter_dict() filtered discovery params: 
transport: 7 rssi: 32767 pathloss: 32767  duplicate data: false
daemon.debug bluetoothd[22991]: src/adapter.c:set_discovery_filter() 
successfully pre-set filter
daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery() sender :1.29
daemon.debug bluetoothd[22991]: src/adapter.c:update_discovery_filter()
daemon.debug bluetoothd[22991]: src/adapter.c:discovery_filter_to_mgmt_cp()
daemon.debug bluetoothd[22991]: src/adapter.c:trigger_start_discovery()
daemon.debug bluetoothd[22991]: src/adapter.c:cancel_passive_scanning()
daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery_timeout()
daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery_timeout() 
adapter->current_discovery_filter == 1
daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery_timeout() 
sending MGMT_OP_START_SERVICE_DISCOVERY 127, 7, 1
daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery_complete() 
status 0x0d
kern.err kernel: [709588.482104] Bluetooth: service_discovery: expected 
4100 bytes, got 20 bytes
daemon.debug bluetoothd[22991]: src/agent.c:agent_disconnect() Agent 
:1.29 disconnected
daemon.debug bluetoothd[22991]: src/agent.c:agent_destroy() agent :1.29
daemon.debug bluetoothd[22991]: src/agent.c:agent_unref() 0xb03e60: ref=0

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

* Re: Error setting UUIDs discovery filter on big endian systems
  2018-12-05  0:27   ` Matt
@ 2019-03-05 18:02     ` Matt
  2019-03-06 11:10       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Matt @ 2019-03-05 18:02 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

On 05/12/2018 00:27, Matt wrote:
> Hi Luiz,
>
> On 04/12/2018 19:53, Luiz Augusto von Dentz wrote:
>> Hi Matt,
>> On Mon, Dec 3, 2018 at 3:59 PM Matt <mwtaylor@gmail.com> wrote:
>>> Hi,
>>>
>>> When setting a UUIDs discovery filter I am receiving
>>> "org.bluez.Error.InProgress" on starting a scan (which isn't in 
>>> progress
>>> and will not start unless the discovery filter is cleared).
>>>
>>> I am using bluez 5.50 (on openWRT). The problem seems to only appear on
>>> big endian hardware (tested a couple of recent kernel and bluez
>>> versions), the same software compiled for and tested on little endian
>>> hardware works as expected. The issue can be reproduced in the 
>>> following
>>> way using bluetoothctl:
>>>
>>> # /etc/init.d/bluetoothd restart
>>> # bluetoothctl
>>> [bluetooth]# power on
>>> Changing power on succeeded
>>> [CHG] Controller 00:1A:7D:DA:71:13 Powered: yes
>>> [bluetooth]# menu scan
>>> [bluetooth]# uuids 0000180f-0000-1000-8000-00805f9b34fb
>>> [bluetooth]# back
>>> [bluetooth]# scan on
>>> SetDiscoveryFilter success
>>> Failed to start discovery: org.bluez.Error.InProgress
>>>
>>> Setting an rssi filter does work as expected on my big endian hardware,
>>> it is only the uuids filter that appears to show this problem. The 
>>> issue
>>> appears whether set using bluetoothctl or directly using DBus.
>> Do you have the bluetoothd logs when that happens? I wonder if it is
>> something with our string to UUID conversion.
>>
>>
> When performing the above sequence in bluetoothctl with 'bluetoothd 
> -d' I get the following in my log:
>
> daemon.debug bluetoothd[22991]: src/agent.c:agent_ref() 0xb03e60: ref=1
> daemon.debug bluetoothd[22991]: src/agent.c:register_agent() agent :1.29
> daemon.debug bluetoothd[22991]: src/adapter.c:property_set_mode() 
> sending Set Powered command for index 0
> daemon.debug bluetoothd[22991]: 
> src/adapter.c:property_set_mode_complete() Success (0x00)
> daemon.debug bluetoothd[22991]: src/adapter.c:new_settings_callback() 
> Settings: 0x00000ad1
> daemon.debug bluetoothd[22991]: src/adapter.c:settings_changed() 
> Changed settings: 0x00000001
> daemon.debug bluetoothd[22991]: src/adapter.c:adapter_start() adapter 
> /org/bluez/hci0 has been enabled
> daemon.debug bluetoothd[22991]: src/adapter.c:trigger_passive_scanning()
> daemon.debug bluetoothd[22991]: src/adapter.c:set_discovery_filter() 
> sender :1.29
> daemon.debug bluetoothd[22991]: 
> src/adapter.c:parse_discovery_filter_dict() filtered discovery params: 
> transport: 7 rssi: 32767 pathloss: 32767  duplicate data: false
> daemon.debug bluetoothd[22991]: src/adapter.c:set_discovery_filter() 
> successfully pre-set filter
> daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery() sender 
> :1.29
> daemon.debug bluetoothd[22991]: src/adapter.c:update_discovery_filter()
> daemon.debug bluetoothd[22991]: 
> src/adapter.c:discovery_filter_to_mgmt_cp()
> daemon.debug bluetoothd[22991]: src/adapter.c:trigger_start_discovery()
> daemon.debug bluetoothd[22991]: src/adapter.c:cancel_passive_scanning()
> daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery_timeout()
> daemon.debug bluetoothd[22991]: 
> src/adapter.c:start_discovery_timeout() 
> adapter->current_discovery_filter == 1
> daemon.debug bluetoothd[22991]: 
> src/adapter.c:start_discovery_timeout() sending 
> MGMT_OP_START_SERVICE_DISCOVERY 127, 7, 1
> daemon.debug bluetoothd[22991]: 
> src/adapter.c:start_discovery_complete() status 0x0d
> kern.err kernel: [709588.482104] Bluetooth: service_discovery: 
> expected 4100 bytes, got 20 bytes
> daemon.debug bluetoothd[22991]: src/agent.c:agent_disconnect() Agent 
> :1.29 disconnected
> daemon.debug bluetoothd[22991]: src/agent.c:agent_destroy() agent :1.29
> daemon.debug bluetoothd[22991]: src/agent.c:agent_unref() 0xb03e60: ref=0

If I instead try a uuid filter of 3 uuids in length I get this in the log:

bluetoothd[550]: src/adapter.c:start_discovery_timeout() sending 
MGMT_OP_START_SERVICE_DISCOVERY 127, 7, 3
kernel: [1077807.129187] Bluetooth: service_discovery: expected 12292 
bytes, got 52 bytes
bluetoothd[550]: src/adapter.c:start_discovery_complete() status 0x0d

So something about the size calculation is going wrong on big endian, 
from adapter.c (line 1607) the command is:

|

mgmt_send(adapter->mgmt,  MGMT_OP_START_SERVICE_DISCOVERY,
		adapter->dev_id,  sizeof(*sd_cp)  +  sd_cp->uuid_count  *  16,
		sd_cp,  start_discovery_complete,  adapter,  NULL);|

With 1 uuid the bytes expected is being returned as 4100 (should be 4 + 
1*16 = 20), with 3 uuids it is 12292 (should be 4 + 3*16 = 52). These 
are the numbers that would be returned if the endian of the uuid_count 
(or the number 16) was switched, i.e. 1 becomes 256, 3 becomes 768. I'm 
not sure why this should be happening.

I have naively tried changing `cp->uuid_count = uuid_count;` to 
`cp->uuid_count = htobs(uuid_count);` (line 2079), just to see what 
would happen by switching the endian, this stops that kernel error 
appearing but just creates an error further along.

bluetoothd[4333]: src/adapter.c:start_discovery_timeout() sending 
MGMT_OP_START_SERVICE_DISCOVERY -45, 7, 256
bluetoothd[4333]: src/adapter.c:start_discovery_complete() status 0x03
bluetoothd[4333]: Wrong size of start discovery return parameters

Perhaps there is some place that is just missing some endian switching 
that would fix this? Sorry, I am not experienced with bluez or related 
code to know where this could be but I would be happy to test any 
suggestions or help in some other way with fixing this issue if someone 
could advise.

Thanks,

Matt

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

* Re: Error setting UUIDs discovery filter on big endian systems
  2019-03-05 18:02     ` Matt
@ 2019-03-06 11:10       ` Luiz Augusto von Dentz
  2019-03-06 13:48         ` Matt
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2019-03-06 11:10 UTC (permalink / raw)
  To: Matt; +Cc: linux-bluetooth

Hi Matt,

Looks like the kernel is indeed assuming the length is in LE:

https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/mgmt.c#n3958

Not sure about the error returned though, perhaps the hdr length is
not properly converted to LE.

On Tue, Mar 5, 2019 at 8:02 PM Matt <mwtaylor@gmail.com> wrote:
>
> On 05/12/2018 00:27, Matt wrote:
> > Hi Luiz,
> >
> > On 04/12/2018 19:53, Luiz Augusto von Dentz wrote:
> >> Hi Matt,
> >> On Mon, Dec 3, 2018 at 3:59 PM Matt <mwtaylor@gmail.com> wrote:
> >>> Hi,
> >>>
> >>> When setting a UUIDs discovery filter I am receiving
> >>> "org.bluez.Error.InProgress" on starting a scan (which isn't in
> >>> progress
> >>> and will not start unless the discovery filter is cleared).
> >>>
> >>> I am using bluez 5.50 (on openWRT). The problem seems to only appear on
> >>> big endian hardware (tested a couple of recent kernel and bluez
> >>> versions), the same software compiled for and tested on little endian
> >>> hardware works as expected. The issue can be reproduced in the
> >>> following
> >>> way using bluetoothctl:
> >>>
> >>> # /etc/init.d/bluetoothd restart
> >>> # bluetoothctl
> >>> [bluetooth]# power on
> >>> Changing power on succeeded
> >>> [CHG] Controller 00:1A:7D:DA:71:13 Powered: yes
> >>> [bluetooth]# menu scan
> >>> [bluetooth]# uuids 0000180f-0000-1000-8000-00805f9b34fb
> >>> [bluetooth]# back
> >>> [bluetooth]# scan on
> >>> SetDiscoveryFilter success
> >>> Failed to start discovery: org.bluez.Error.InProgress
> >>>
> >>> Setting an rssi filter does work as expected on my big endian hardware,
> >>> it is only the uuids filter that appears to show this problem. The
> >>> issue
> >>> appears whether set using bluetoothctl or directly using DBus.
> >> Do you have the bluetoothd logs when that happens? I wonder if it is
> >> something with our string to UUID conversion.
> >>
> >>
> > When performing the above sequence in bluetoothctl with 'bluetoothd
> > -d' I get the following in my log:
> >
> > daemon.debug bluetoothd[22991]: src/agent.c:agent_ref() 0xb03e60: ref=1
> > daemon.debug bluetoothd[22991]: src/agent.c:register_agent() agent :1.29
> > daemon.debug bluetoothd[22991]: src/adapter.c:property_set_mode()
> > sending Set Powered command for index 0
> > daemon.debug bluetoothd[22991]:
> > src/adapter.c:property_set_mode_complete() Success (0x00)
> > daemon.debug bluetoothd[22991]: src/adapter.c:new_settings_callback()
> > Settings: 0x00000ad1
> > daemon.debug bluetoothd[22991]: src/adapter.c:settings_changed()
> > Changed settings: 0x00000001
> > daemon.debug bluetoothd[22991]: src/adapter.c:adapter_start() adapter
> > /org/bluez/hci0 has been enabled
> > daemon.debug bluetoothd[22991]: src/adapter.c:trigger_passive_scanning()
> > daemon.debug bluetoothd[22991]: src/adapter.c:set_discovery_filter()
> > sender :1.29
> > daemon.debug bluetoothd[22991]:
> > src/adapter.c:parse_discovery_filter_dict() filtered discovery params:
> > transport: 7 rssi: 32767 pathloss: 32767  duplicate data: false
> > daemon.debug bluetoothd[22991]: src/adapter.c:set_discovery_filter()
> > successfully pre-set filter
> > daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery() sender
> > :1.29
> > daemon.debug bluetoothd[22991]: src/adapter.c:update_discovery_filter()
> > daemon.debug bluetoothd[22991]:
> > src/adapter.c:discovery_filter_to_mgmt_cp()
> > daemon.debug bluetoothd[22991]: src/adapter.c:trigger_start_discovery()
> > daemon.debug bluetoothd[22991]: src/adapter.c:cancel_passive_scanning()
> > daemon.debug bluetoothd[22991]: src/adapter.c:start_discovery_timeout()
> > daemon.debug bluetoothd[22991]:
> > src/adapter.c:start_discovery_timeout()
> > adapter->current_discovery_filter == 1
> > daemon.debug bluetoothd[22991]:
> > src/adapter.c:start_discovery_timeout() sending
> > MGMT_OP_START_SERVICE_DISCOVERY 127, 7, 1
> > daemon.debug bluetoothd[22991]:
> > src/adapter.c:start_discovery_complete() status 0x0d
> > kern.err kernel: [709588.482104] Bluetooth: service_discovery:
> > expected 4100 bytes, got 20 bytes
> > daemon.debug bluetoothd[22991]: src/agent.c:agent_disconnect() Agent
> > :1.29 disconnected
> > daemon.debug bluetoothd[22991]: src/agent.c:agent_destroy() agent :1.29
> > daemon.debug bluetoothd[22991]: src/agent.c:agent_unref() 0xb03e60: ref=0
>
> If I instead try a uuid filter of 3 uuids in length I get this in the log:
>
> bluetoothd[550]: src/adapter.c:start_discovery_timeout() sending
> MGMT_OP_START_SERVICE_DISCOVERY 127, 7, 3
> kernel: [1077807.129187] Bluetooth: service_discovery: expected 12292
> bytes, got 52 bytes
> bluetoothd[550]: src/adapter.c:start_discovery_complete() status 0x0d
>
> So something about the size calculation is going wrong on big endian,
> from adapter.c (line 1607) the command is:
>
> |
>
> mgmt_send(adapter->mgmt,  MGMT_OP_START_SERVICE_DISCOVERY,
>                 adapter->dev_id,  sizeof(*sd_cp)  +  sd_cp->uuid_count  *  16,
>                 sd_cp,  start_discovery_complete,  adapter,  NULL);|
>
> With 1 uuid the bytes expected is being returned as 4100 (should be 4 +
> 1*16 = 20), with 3 uuids it is 12292 (should be 4 + 3*16 = 52). These
> are the numbers that would be returned if the endian of the uuid_count
> (or the number 16) was switched, i.e. 1 becomes 256, 3 becomes 768. I'm
> not sure why this should be happening.
>
> I have naively tried changing `cp->uuid_count = uuid_count;` to
> `cp->uuid_count = htobs(uuid_count);` (line 2079), just to see what
> would happen by switching the endian, this stops that kernel error
> appearing but just creates an error further along.
>
> bluetoothd[4333]: src/adapter.c:start_discovery_timeout() sending
> MGMT_OP_START_SERVICE_DISCOVERY -45, 7, 256
> bluetoothd[4333]: src/adapter.c:start_discovery_complete() status 0x03
> bluetoothd[4333]: Wrong size of start discovery return parameters
>
> Perhaps there is some place that is just missing some endian switching
> that would fix this? Sorry, I am not experienced with bluez or related
> code to know where this could be but I would be happy to test any
> suggestions or help in some other way with fixing this issue if someone
> could advise.
>
> Thanks,
>
> Matt



-- 
Luiz Augusto von Dentz

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

* Re: Error setting UUIDs discovery filter on big endian systems
  2019-03-06 11:10       ` Luiz Augusto von Dentz
@ 2019-03-06 13:48         ` Matt
  2019-03-06 14:14           ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Matt @ 2019-03-06 13:48 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

On 06/03/2019 11:10, Luiz Augusto von Dentz wrote:
> Looks like the kernel is indeed assuming the length is in LE:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/mgmt.c#n3958

Thanks for pointing me to this line, removing the __le16_to_cpu() does 
seem to fix the UUIDs filter scanning on my big endian hardware, I'm not 
sure why it is needed, perhaps removing it would break LE hardware or 
some other case but I would imagine __le16_to_cpu() would do nothing in 
the LE case anyway. I have made this patch to my kernel (4.9) that seems 
to be all is needed for me to fix the issue (no changes to bluez):

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ba24f61..507d996 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3599,7 +3599,7 @@ static int start_service_discovery(struct sock 
*sk, struct hci_dev *hdev,
          goto failed;
      }

-    uuid_count = __le16_to_cpu(cp->uuid_count);
+    uuid_count = cp->uuid_count;
      if (uuid_count > max_uuid_count) {
          BT_ERR("service_discovery: too big uuid_count value %u",
                 uuid_count);



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

* Re: Error setting UUIDs discovery filter on big endian systems
  2019-03-06 13:48         ` Matt
@ 2019-03-06 14:14           ` Luiz Augusto von Dentz
  2019-03-06 15:14             ` Matt
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2019-03-06 14:14 UTC (permalink / raw)
  To: Matt; +Cc: linux-bluetooth

Hi Matt,
On Wed, Mar 6, 2019 at 3:48 PM Matt <mwtaylor@gmail.com> wrote:
>
> On 06/03/2019 11:10, Luiz Augusto von Dentz wrote:
> > Looks like the kernel is indeed assuming the length is in LE:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/mgmt.c#n3958
>
> Thanks for pointing me to this line, removing the __le16_to_cpu() does
> seem to fix the UUIDs filter scanning on my big endian hardware, I'm not
> sure why it is needed, perhaps removing it would break LE hardware or
> some other case but I would imagine __le16_to_cpu() would do nothing in
> the LE case anyway. I have made this patch to my kernel (4.9) that seems
> to be all is needed for me to fix the issue (no changes to bluez):

Check the patch Ive just sent, your initial fix is actually correct
but since you change it to little endian you had to convert it back
when calculating the length of message otherwise you end up with an
invalid size.

The kernel is actually assuming Little Endian as this is a convention
for Bluetooth protocols.

> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index ba24f61..507d996 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3599,7 +3599,7 @@ static int start_service_discovery(struct sock
> *sk, struct hci_dev *hdev,
>           goto failed;
>       }
>
> -    uuid_count = __le16_to_cpu(cp->uuid_count);
> +    uuid_count = cp->uuid_count;
>       if (uuid_count > max_uuid_count) {
>           BT_ERR("service_discovery: too big uuid_count value %u",
>                  uuid_count);
>
>


-- 
Luiz Augusto von Dentz

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

* Re: Error setting UUIDs discovery filter on big endian systems
  2019-03-06 14:14           ` Luiz Augusto von Dentz
@ 2019-03-06 15:14             ` Matt
  0 siblings, 0 replies; 8+ messages in thread
From: Matt @ 2019-03-06 15:14 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

On 06/03/2019 14:14, Luiz Augusto von Dentz wrote:
> Hi Matt,
> On Wed, Mar 6, 2019 at 3:48 PM Matt <mwtaylor@gmail.com> wrote:
>> On 06/03/2019 11:10, Luiz Augusto von Dentz wrote:
>>> Looks like the kernel is indeed assuming the length is in LE:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/mgmt.c#n3958
>> Thanks for pointing me to this line, removing the __le16_to_cpu() does
>> seem to fix the UUIDs filter scanning on my big endian hardware, I'm not
>> sure why it is needed, perhaps removing it would break LE hardware or
>> some other case but I would imagine __le16_to_cpu() would do nothing in
>> the LE case anyway. I have made this patch to my kernel (4.9) that seems
>> to be all is needed for me to fix the issue (no changes to bluez):
> Check the patch Ive just sent, your initial fix is actually correct
> but since you change it to little endian you had to convert it back
> when calculating the length of message otherwise you end up with an
> invalid size.

The patch you sent works, thanks. (Sorry, I missed it as I wasn't 
subscribed to the mailing list but looking at the archive.)

>
> The kernel is actually assuming Little Endian as this is a convention
> for Bluetooth protocols.

Ok, that would explain it then.

Many thanks for your help,

Matt


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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-03 13:56 Error setting UUIDs discovery filter on big endian systems Matt
2018-12-04 19:53 ` Luiz Augusto von Dentz
2018-12-05  0:27   ` Matt
2019-03-05 18:02     ` Matt
2019-03-06 11:10       ` Luiz Augusto von Dentz
2019-03-06 13:48         ` Matt
2019-03-06 14:14           ` Luiz Augusto von Dentz
2019-03-06 15:14             ` Matt

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org linux-bluetooth@archiver.kernel.org
	public-inbox-index linux-bluetooth


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/ public-inbox