[PATCH 1/1] bluetooth: validate BLE connection interval updates

[PATCH 1/1] bluetooth: validate BLE connection interval updates

[csonsino]

Problem: The Linux bluetooth stack yields complete control over the BLE
connection interval to the remote device.

The Linux bluetooth stack provides access to the BLE connection interval
min and max values through /sys/kernel/debug/bluetooth/hci0/
conn_min_interval and /sys/kernel/debug/bluetooth/hci0/conn_max_interval.
These values are used for initial BLE connections, but the remote device
has the ability to request a connection parameter update. In the event
that the remote side requests to change the connection interval, the Linux
kernel currently only validates that the desired value is within the
acceptable range in the bluetooth specification (6 - 3200, corresponding to
7.5ms - 4000ms). There is currently no validation that the desired value
requested by the remote device is within the min/max limits specified in
the conn_min_interval/conn_max_interval configurations. This essentially
leads to Linux yielding complete control over the connection interval to
the remote device.

The proposed patch adds a verification step to the connection parameter
update mechanism, ensuring that the desired value is within the min/max
bounds of the current connection. If the desired value is outside of the
current connection min/max values, then the connection parameter update
request is rejected and the negative response is returned to the remote
device. Recall that the initial connection is established using the local
conn_min_interval/conn_max_interval values, so this allows the Linux
administrator to retain control over the BLE connection interval.

The one downside that I see is that the current default Linux values for
conn_min_interval and conn_max_interval typically correspond to 30ms and
50ms respectively. If this change were accepted, then it is feasible that
some devices would no longer be able to negotiate to their desired
connection interval values. This might be remedied by setting the default
Linux conn_min_interval and conn_max_interval values to the widest
supported range (6 - 3200 / 7.5ms - 4000ms). This could lead to the same
behavior as the current implementation, where the remote device could
request to change the connection interval value to any value that is
permitted by the bluetooth specification, and Linux would accept the
desired value.

Signed-off-by: Carey Sonsino <csonsino@gmail.com>

---

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ef9928d7b4fb..e5d69ae5eda1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5576,6 +5576,11 @@ static void hci_le_remote_conn_param_req_evt(struct hci_dev *hdev,
 		return send_conn_param_neg_reply(hdev, handle,
 						 HCI_ERROR_UNKNOWN_CONN_ID);
 
+	if (min < hcon->le_conn_min_interval ||
+	    max > hcon->le_conn_max_interval)
+		return send_conn_param_neg_reply(hdev, handle,
+						 HCI_ERROR_INVALID_LL_PARAMS);
+
 	if (hci_check_conn_params(min, max, latency, timeout))
 		return send_conn_param_neg_reply(hdev, handle,
 						 HCI_ERROR_INVALID_LL_PARAMS);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2146e0f3b6f8..1cea68108dba 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5246,7 +5246,14 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
 
 	memset(&rsp, 0, sizeof(rsp));
 
-	err = hci_check_conn_params(min, max, latency, to_multiplier);
+	if (min < hcon->le_conn_min_interval ||
+	    max > hcon->le_conn_max_interval) {
+		BT_DBG("requested connection interval exceeds current bounds.");
+		err = -EINVAL;
+	} else {
+		err = hci_check_conn_params(min, max, latency, to_multiplier);
+	}
+
 	if (err)
 		rsp.result = cpu_to_le16(L2CAP_CONN_PARAM_REJECTED);
 	else

Re: [PATCH 1/1] bluetooth: validate BLE connection interval updates

[Marcel Holtmann]

Hi Carey,

> Problem: The Linux bluetooth stack yields complete control over the BLE
> connection interval to the remote device.
> 
> The Linux bluetooth stack provides access to the BLE connection interval
> min and max values through /sys/kernel/debug/bluetooth/hci0/
> conn_min_interval and /sys/kernel/debug/bluetooth/hci0/conn_max_interval.
> These values are used for initial BLE connections, but the remote device
> has the ability to request a connection parameter update. In the event
> that the remote side requests to change the connection interval, the Linux
> kernel currently only validates that the desired value is within the
> acceptable range in the bluetooth specification (6 - 3200, corresponding to
> 7.5ms - 4000ms). There is currently no validation that the desired value
> requested by the remote device is within the min/max limits specified in
> the conn_min_interval/conn_max_interval configurations. This essentially
> leads to Linux yielding complete control over the connection interval to
> the remote device.
> 
> The proposed patch adds a verification step to the connection parameter
> update mechanism, ensuring that the desired value is within the min/max
> bounds of the current connection. If the desired value is outside of the
> current connection min/max values, then the connection parameter update
> request is rejected and the negative response is returned to the remote
> device. Recall that the initial connection is established using the local
> conn_min_interval/conn_max_interval values, so this allows the Linux
> administrator to retain control over the BLE connection interval.
> 
> The one downside that I see is that the current default Linux values for
> conn_min_interval and conn_max_interval typically correspond to 30ms and
> 50ms respectively. If this change were accepted, then it is feasible that
> some devices would no longer be able to negotiate to their desired
> connection interval values. This might be remedied by setting the default
> Linux conn_min_interval and conn_max_interval values to the widest
> supported range (6 - 3200 / 7.5ms - 4000ms). This could lead to the same
> behavior as the current implementation, where the remote device could
> request to change the connection interval value to any value that is
> permitted by the bluetooth specification, and Linux would accept the
> desired value.
> 
> Signed-off-by: Carey Sonsino <csonsino@gmail.com>

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH 1/1] bluetooth: validate BLE connection interval updates

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 3624 bytes --]

On Sat, 6 Jul 2019 15:34:34 +0200
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Carey,
> 
> > Problem: The Linux bluetooth stack yields complete control over the BLE
> > connection interval to the remote device.
> > 
> > The Linux bluetooth stack provides access to the BLE connection interval
> > min and max values through /sys/kernel/debug/bluetooth/hci0/
> > conn_min_interval and /sys/kernel/debug/bluetooth/hci0/conn_max_interval.
> > These values are used for initial BLE connections, but the remote device
> > has the ability to request a connection parameter update. In the event
> > that the remote side requests to change the connection interval, the Linux
> > kernel currently only validates that the desired value is within the
> > acceptable range in the bluetooth specification (6 - 3200, corresponding to
> > 7.5ms - 4000ms). There is currently no validation that the desired value
> > requested by the remote device is within the min/max limits specified in
> > the conn_min_interval/conn_max_interval configurations. This essentially
> > leads to Linux yielding complete control over the connection interval to
> > the remote device.
> > 
> > The proposed patch adds a verification step to the connection parameter
> > update mechanism, ensuring that the desired value is within the min/max
> > bounds of the current connection. If the desired value is outside of the
> > current connection min/max values, then the connection parameter update
> > request is rejected and the negative response is returned to the remote
> > device. Recall that the initial connection is established using the local
> > conn_min_interval/conn_max_interval values, so this allows the Linux
> > administrator to retain control over the BLE connection interval.
> > 
> > The one downside that I see is that the current default Linux values for
> > conn_min_interval and conn_max_interval typically correspond to 30ms and
> > 50ms respectively. If this change were accepted, then it is feasible that
> > some devices would no longer be able to negotiate to their desired
> > connection interval values. This might be remedied by setting the default
> > Linux conn_min_interval and conn_max_interval values to the widest
> > supported range (6 - 3200 / 7.5ms - 4000ms). This could lead to the same
> > behavior as the current implementation, where the remote device could
> > request to change the connection interval value to any value that is
> > permitted by the bluetooth specification, and Linux would accept the
> > desired value.
> > 
> > Signed-off-by: Carey Sonsino <csonsino@gmail.com>  
> 
> patch has been applied to bluetooth-next tree.
> 
There are devices which require low connection intervals for usable operation,
e.g. BLE smartcard readers. having 30ms instead of 7.5ms means speed four times
lower.

Other devices might want to set the connection interval  to high values to save
power.

So if the device is not allowed to set the connection interval to such values,
how should the driver sitting on top of the gatt dbus interface of bluetoothd
set such things?

The debugfs nodes are not necessarily available in distro kernels. Anything
sitting on top of gatt dbus interface does not have access to the connection handle
and cannot call hci_le_conn_update() to set things to nice values.

Using l2cap sockets instead of the dbus gatt interface is also not a viable altenative
because it interferes with bluetoothd.

So IMHO this patch causes regressions and should be reverted.

Sorry for stepping out this late.

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/1] bluetooth: validate BLE connection interval updates

[Carey Sonsino]

This seems like the exact "downside" situation that I described in the 
patch writeup.

I would still claim that as a Linux system administrator, I should have 
control over my system.  If I am operating in a low power environment, I 
do not want to allow a remote device to apply a setting which causes me 
to use more power without any say in the matter.

The connection min/max interval settings are configuration options that 
control how my bluetooth device operates.  Why are these down in debugfs 
anyway?  I think that a much more appropriate fix would be to migrate 
some of the BLE configuration options to sysconfdir (some place like 
/etc/bluetooth/ble.conf).  That would also help in the persistence of 
these configuration settings, which is kind of a pain with the debugfs 
mechanism that gets wiped out and recreated on bootup.

A quicker fix would be to simply set the default connection min interval 
and connection max interval values to the full range (6, 3200).  I 
*think* that this could be done by simply updating the values in 
hci_core.c, the hci_alloc_dev() function:

     hdev->le_conn_min_interval = 0x0018;
     hdev->le_conn_max_interval = 0x0028;

would become:

     hdev->le_conn_min_interval = 0x0006;
     hdev->le_conn_max_interval = 0x0c80;

This should allow all devices to negotiate whatever connection interval 
they want by default.  If I'm running on a device with debugfs (which I 
happen to be most of the time), then I can still override these defaults 
to control my system.

Please let me know if you would like me to do any further work towards 
resolving this issue.  I'd be happy to test and submit a patch that 
changes the default connection min/max interval values- I could probably 
get that done in the next day or two.  If you would like me to 
investigate migrating configuration settings to /etc then I'd be happy 
to do that as well, but it might take a bit more effort and time.

Regards,

Carey


On 8/15/19 2:44 AM, Andreas Kemnade wrote:
> On Sat, 6 Jul 2019 15:34:34 +0200
> Marcel Holtmann <marcel@holtmann.org> wrote:
>
>> Hi Carey,
>>
>>> Problem: The Linux bluetooth stack yields complete control over the BLE
>>> connection interval to the remote device.
>>>
>>> The Linux bluetooth stack provides access to the BLE connection interval
>>> min and max values through /sys/kernel/debug/bluetooth/hci0/
>>> conn_min_interval and /sys/kernel/debug/bluetooth/hci0/conn_max_interval.
>>> These values are used for initial BLE connections, but the remote device
>>> has the ability to request a connection parameter update. In the event
>>> that the remote side requests to change the connection interval, the Linux
>>> kernel currently only validates that the desired value is within the
>>> acceptable range in the bluetooth specification (6 - 3200, corresponding to
>>> 7.5ms - 4000ms). There is currently no validation that the desired value
>>> requested by the remote device is within the min/max limits specified in
>>> the conn_min_interval/conn_max_interval configurations. This essentially
>>> leads to Linux yielding complete control over the connection interval to
>>> the remote device.
>>>
>>> The proposed patch adds a verification step to the connection parameter
>>> update mechanism, ensuring that the desired value is within the min/max
>>> bounds of the current connection. If the desired value is outside of the
>>> current connection min/max values, then the connection parameter update
>>> request is rejected and the negative response is returned to the remote
>>> device. Recall that the initial connection is established using the local
>>> conn_min_interval/conn_max_interval values, so this allows the Linux
>>> administrator to retain control over the BLE connection interval.
>>>
>>> The one downside that I see is that the current default Linux values for
>>> conn_min_interval and conn_max_interval typically correspond to 30ms and
>>> 50ms respectively. If this change were accepted, then it is feasible that
>>> some devices would no longer be able to negotiate to their desired
>>> connection interval values. This might be remedied by setting the default
>>> Linux conn_min_interval and conn_max_interval values to the widest
>>> supported range (6 - 3200 / 7.5ms - 4000ms). This could lead to the same
>>> behavior as the current implementation, where the remote device could
>>> request to change the connection interval value to any value that is
>>> permitted by the bluetooth specification, and Linux would accept the
>>> desired value.
>>>
>>> Signed-off-by: Carey Sonsino <csonsino@gmail.com>
>> patch has been applied to bluetooth-next tree.
>>
> There are devices which require low connection intervals for usable operation,
> e.g. BLE smartcard readers. having 30ms instead of 7.5ms means speed four times
> lower.
>
> Other devices might want to set the connection interval  to high values to save
> power.
>
> So if the device is not allowed to set the connection interval to such values,
> how should the driver sitting on top of the gatt dbus interface of bluetoothd
> set such things?
>
> The debugfs nodes are not necessarily available in distro kernels. Anything
> sitting on top of gatt dbus interface does not have access to the connection handle
> and cannot call hci_le_conn_update() to set things to nice values.
>
> Using l2cap sockets instead of the dbus gatt interface is also not a viable altenative
> because it interferes with bluetoothd.
>
> So IMHO this patch causes regressions and should be reverted.
>
> Sorry for stepping out this late.
>
> Regards,
> Andreas

Re: [PATCH 1/1] bluetooth: validate BLE connection interval updates

[Andreas Kemnade]

Hi Carey,

On Mon, 19 Aug 2019 07:58:19 -0600
Carey Sonsino <csonsino@gmail.com> wrote:

> This seems like the exact "downside" situation that I described in the 
> patch writeup.
> 
> I would still claim that as a Linux system administrator, I should have 
> control over my system.  If I am operating in a low power environment, I 
> do not want to allow a remote device to apply a setting which causes me 
> to use more power without any say in the matter.
> 
In principle I agree here. High connection interval has its downsides,
low connection interval also. Just curios: What are the numbers about
power consumption here? A few mA? I have only compared these values on
peripherals running on low-power SoCs like e.g. the nrf stuff from nordic.
I see around 1 mA difference there with a power consumption besides of that 
usually measured in the µA range. Never tested these things on a linux machine.

The point here is that with this patch there is insufficient control
about this. Yes, there is the debugfs interface.

But if you want to provide a driver to a gatt service living on top of
bluetoothd dbus api? Ask people to not use distribution kernels?
What options do you have?
using the monitor interface to sniff the connection handle and then
call hci_le_conn_update() to set things?

> The connection min/max interval settings are configuration options that 
> control how my bluetooth device operates.  Why are these down in debugfs 
> anyway?  I think that a much more appropriate fix would be to migrate 
> some of the BLE configuration options to sysconfdir (some place like 
> /etc/bluetooth/ble.conf).  That would also help in the persistence of 
> these configuration settings, which is kind of a pain with the debugfs 
> mechanism that gets wiped out and recreated on bootup.
> 
I think that these things should be part of the  dbus api provided
by bluetoothd so that a driver could decide and having defaults outside
of such a dark corner like the debug fs.

> A quicker fix would be to simply set the default connection min interval 
> and connection max interval values to the full range (6, 3200).

Or just maybe a flag allowing such behavior?

> *think* that this could be done by simply updating the values in 
> hci_core.c, the hci_alloc_dev() function:
> 
>      hdev->le_conn_min_interval = 0x0018;
>      hdev->le_conn_max_interval = 0x0028;
> 
> would become:
> 
>      hdev->le_conn_min_interval = 0x0006;
>      hdev->le_conn_max_interval = 0x0c80;
> 
> This should allow all devices to negotiate whatever connection interval 
> they want by default.  If I'm running on a device with debugfs (which I 
> happen to be most of the time), then I can still override these defaults 
> to control my system.
> 
> Please let me know if you would like me to do any further work towards 
> resolving this issue.  I'd be happy to test and submit a patch that 
> changes the default connection min/max interval values- I could probably 
> get that done in the next day or two.  If you would like me to 
> investigate migrating configuration settings to /etc then I'd be happy 
> to do that as well, but it might take a bit more effort and time.
> 
Well, all these things are important, but are new features but there is a
problem:
The kernel patch has gone into the stable trees and from there into distributions,
so how can these new features flow down the same path.

Regards,
Andreas

Re: [PATCH 1/1] bluetooth: validate BLE connection interval updates

[Carey Sonsino]

Andreas,

Considering that this patch has already made it's way into stable trees, 
and the only options are to back it out or to patch it again, I would 
obviously prefer to keep it in and patch it again.  I think that it 
would be fairly quick and painless to just set the default min/max to 
6/3200, or add an additional flag as you suggested, which should resolve 
any problems in the near term.

Longer term, I think that there are several ways to allow the system 
admin to configure the allowable min/max, but it only matters if the 
system will check requests against the configured min/max and respond 
appropriately, which is what the patch in question does.  The best 
current way that I'm aware of to control the system-wide connection 
interval is through the existing debugfs mechanism, and in my opinion it 
didn't work correctly without this patch.

Regarding power consumption, I have not done any power measurement 
testing to determine the effect of connection interval on power 
consumption, so perhaps that was not the best use case.  Here is the 
exact real world use case that caused me to write the patch in the first 
place:

I was writing a Linux-based test system for an embedded 
BLE-communication based product.  The product embedded code was written 
to attempt to renegotiate the connection interval to a fairly low number 
after the initial connection was established. My test system had a 
requirement to be able to perform the tests using various pre-defined 
connection intervals so that I could gather data throughput and product 
performance metrics.  Every time my test system attempted to perform a 
test a the desired connection interval, the device would immediately 
request to renegiate to a lower value.  Linux would accept that request 
and return a successful response, and I had no way to stop it.  Let's 
not go down the "Why didn't you just change the embedded code?" / "Why 
doesn't Linux reject values outside of the configured bounds?" rabbit 
hole...  :)

Carey

On 8/19/19 10:08 AM, Andreas Kemnade wrote:
> Hi Carey,
>
> On Mon, 19 Aug 2019 07:58:19 -0600
> Carey Sonsino <csonsino@gmail.com> wrote:
>
>> This seems like the exact "downside" situation that I described in the
>> patch writeup.
>>
>> I would still claim that as a Linux system administrator, I should have
>> control over my system.  If I am operating in a low power environment, I
>> do not want to allow a remote device to apply a setting which causes me
>> to use more power without any say in the matter.
>>
> In principle I agree here. High connection interval has its downsides,
> low connection interval also. Just curios: What are the numbers about
> power consumption here? A few mA? I have only compared these values on
> peripherals running on low-power SoCs like e.g. the nrf stuff from nordic.
> I see around 1 mA difference there with a power consumption besides of that
> usually measured in the µA range. Never tested these things on a linux machine.
>
> The point here is that with this patch there is insufficient control
> about this. Yes, there is the debugfs interface.
>
> But if you want to provide a driver to a gatt service living on top of
> bluetoothd dbus api? Ask people to not use distribution kernels?
> What options do you have?
> using the monitor interface to sniff the connection handle and then
> call hci_le_conn_update() to set things?
>
>> The connection min/max interval settings are configuration options that
>> control how my bluetooth device operates.  Why are these down in debugfs
>> anyway?  I think that a much more appropriate fix would be to migrate
>> some of the BLE configuration options to sysconfdir (some place like
>> /etc/bluetooth/ble.conf).  That would also help in the persistence of
>> these configuration settings, which is kind of a pain with the debugfs
>> mechanism that gets wiped out and recreated on bootup.
>>
> I think that these things should be part of the  dbus api provided
> by bluetoothd so that a driver could decide and having defaults outside
> of such a dark corner like the debug fs.
>
>> A quicker fix would be to simply set the default connection min interval
>> and connection max interval values to the full range (6, 3200).
> Or just maybe a flag allowing such behavior?
>
>> *think* that this could be done by simply updating the values in
>> hci_core.c, the hci_alloc_dev() function:
>>
>>       hdev->le_conn_min_interval = 0x0018;
>>       hdev->le_conn_max_interval = 0x0028;
>>
>> would become:
>>
>>       hdev->le_conn_min_interval = 0x0006;
>>       hdev->le_conn_max_interval = 0x0c80;
>>
>> This should allow all devices to negotiate whatever connection interval
>> they want by default.  If I'm running on a device with debugfs (which I
>> happen to be most of the time), then I can still override these defaults
>> to control my system.
>>
>> Please let me know if you would like me to do any further work towards
>> resolving this issue.  I'd be happy to test and submit a patch that
>> changes the default connection min/max interval values- I could probably
>> get that done in the next day or two.  If you would like me to
>> investigate migrating configuration settings to /etc then I'd be happy
>> to do that as well, but it might take a bit more effort and time.
>>
> Well, all these things are important, but are new features but there is a
> problem:
> The kernel patch has gone into the stable trees and from there into distributions,
> so how can these new features flow down the same path.
>
> Regards,
> Andreas
>

RE: [PATCH 1/1] bluetooth: validate BLE connection interval updates

[Jamie Mccrae]

Hi,
From my many years' experience of embedded Bluetooth LE development I
would say that offering different connection intervals to different
devices is a must. If you have 3 devices connected to a laptop, for
example 1 device in DFU mode, 1 keyboard and 1 temperature sensor, you
would want vastly different connection interval settings for all of
these. To speed up a firmware update process on the DFU device you would
want the lowest connection interval (and I can speak from experience
having seen an embedded linux SoC try to update a device over DFU via
Bluetooth LE on Bluez 4.x and DBUS that it took in excess of 30 minutes
per device, compared to 1 minute on an android phone). They keyboard is
likely to negotiate a slave latency so that it can stay idle if no keys
are pressed for many connection intervals but if something is
pressed then it can respond fast, and the temperature sensor is going to
have a much larger connection interval, if the sensor itself gathers a
new reading every minute, then a connection interval of 30 seconds would
be fine. So to make things easy for both developers and users, how would
you go about doing that? Simply having a single 'minimum' and 'maximum'
connection interval in a file or hard-coded is not a workable solution
and does nothing to help user experience.
Thanks,
Jamie

> Andreas,
>
> Considering that this patch has already made it's way into stable trees,
> and the only options are to back it out or to patch it again, I would
> obviously prefer to keep it in and patch it again.  I think that it
> would be fairly quick and painless to just set the default min/max to
> 6/3200, or add an additional flag as you suggested, which should resolve
> any problems in the near term.
>
> Longer term, I think that there are several ways to allow the system
> admin to configure the allowable min/max, but it only matters if the
> system will check requests against the configured min/max and respond
> appropriately, which is what the patch in question does.  The best
> current way that I'm aware of to control the system-wide connection
> interval is through the existing debugfs mechanism, and in my opinion it
> didn't work correctly without this patch.
>
> Regarding power consumption, I have not done any power measurement
> testing to determine the effect of connection interval on power
> consumption, so perhaps that was not the best use case.  Here is the
> exact real world use case that caused me to write the patch in the first
> place:
>
> I was writing a Linux-based test system for an embedded
> BLE-communication based product.  The product embedded code was written
> to attempt to renegotiate the connection interval to a fairly low number
> after the initial connection was established. My test system had a
> requirement to be able to perform the tests using various pre-defined
> connection intervals so that I could gather data throughput and product
> performance metrics.  Every time my test system attempted to perform a
> test a the desired connection interval, the device would immediately
> request to renegiate to a lower value.  Linux would accept that request
> and return a successful response, and I had no way to stop it.  Let's
> not go down the "Why didn't you just change the embedded code?" / "Why
> doesn't Linux reject values outside of the configured bounds?" rabbit
> hole...  :)
>
> Carey
>
> On 8/19/19 10:08 AM, Andreas Kemnade wrote:
>> Hi Carey,
>>
>> On Mon, 19 Aug 2019 07:58:19 -0600
>> Carey Sonsino <csonsino@gmail.com> wrote:
>>
>>> This seems like the exact "downside" situation that I described in the
>>> patch writeup.
>>>
>>> I would still claim that as a Linux system administrator, I should have
>>> control over my system.  If I am operating in a low power environment, I
>>> do not want to allow a remote device to apply a setting which causes me
>>> to use more power without any say in the matter.
>>>
>> In principle I agree here. High connection interval has its downsides,
>> low connection interval also. Just curios: What are the numbers about
>> power consumption here? A few mA? I have only compared these values on
>> peripherals running on low-power SoCs like e.g. the nrf stuff from nordic.
>> I see around 1 mA difference there with a power consumption besides of that
>> usually measured in the µA range. Never tested these things on a linux machine.
>>
>> The point here is that with this patch there is insufficient control
>> about this. Yes, there is the debugfs interface.
>>
>> But if you want to provide a driver to a gatt service living on top of
>> bluetoothd dbus api? Ask people to not use distribution kernels?
>> What options do you have?
>> using the monitor interface to sniff the connection handle and then
>> call hci_le_conn_update() to set things?
>>
>>> The connection min/max interval settings are configuration options that
>>> control how my bluetooth device operates.  Why are these down in debugfs
>>> anyway?  I think that a much more appropriate fix would be to migrate
>>> some of the BLE configuration options to sysconfdir (some place like
>>> /etc/bluetooth/ble.conf).  That would also help in the persistence of
>>> these configuration settings, which is kind of a pain with the debugfs
>>> mechanism that gets wiped out and recreated on bootup.
>>>
>> I think that these things should be part of the  dbus api provided
>> by bluetoothd so that a driver could decide and having defaults outside
>> of such a dark corner like the debug fs.
>>
>>> A quicker fix would be to simply set the default connection min interval
>>> and connection max interval values to the full range (6, 3200).
>> Or just maybe a flag allowing such behavior?
>>
>>> *think* that this could be done by simply updating the values in
>>> hci_core.c, the hci_alloc_dev() function:
>>>
>>>       hdev->le_conn_min_interval = 0x0018;
>>>       hdev->le_conn_max_interval = 0x0028;
>>>
>>> would become:
>>>
>>>       hdev->le_conn_min_interval = 0x0006;
>>>       hdev->le_conn_max_interval = 0x0c80;
>>>
>>> This should allow all devices to negotiate whatever connection interval
>>> they want by default.  If I'm running on a device with debugfs (which I
>>> happen to be most of the time), then I can still override these defaults
>>> to control my system.
>>>
>>> Please let me know if you would like me to do any further work towards
>>> resolving this issue.  I'd be happy to test and submit a patch that
>>> changes the default connection min/max interval values- I could probably
>>> get that done in the next day or two.  If you would like me to
>>> investigate migrating configuration settings to /etc then I'd be happy
>>> to do that as well, but it might take a bit more effort and time.
>>>
>> Well, all these things are important, but are new features but there is a
>> problem:
>> The kernel patch has gone into the stable trees and from there into distributions,
>> so how can these new features flow down the same path.
>>
>> Regards,
>> Andreas
>>

Re: [PATCH 1/1] bluetooth: validate BLE connection interval updates

[Carey Sonsino]

Jamie,

I completely agree that different devices should (and need to) be able 
to negotiate their own desired connection interval... as long as the 
connection interval that they want to use is within the bounds that I as 
a Linux system administrator have allowed my system to operate.

Admittedly, the current situation is not good because the default values 
are 24/40, so without additional modification (which it turns out may 
not be easy or possible if you don't have debugfs) a device can only 
negotiate a connection interval between 30ms and 50ms otherwise it will 
receive a negative/rejection response.  But if we were to change the 
default values to 6/3200, then any device would be able to negotiate 
it's own desired connection interval between 7.5ms and 4000ms and 
additional configuration would be required to restrict that range.

I think that if multiple BLE devices were connected to my Linux machine, 
the BLE radio would have to "wake up" on the schedule of the shortest 
connection interval, so I'm not convinced that you would actually get 
any benefit from a min/max per client.  But I do agree that I like 
having that level of control.  If we did want to support a min/max per 
client, information is already stored per-client in 
/var/lib/bluetooth/<DEVICE_MAC>/ (for secure keys and other things) and 
we could probably add min/max configuration down there somewhere.  Maybe 
it could even just override the global min/max in the debugfs area.

But also please note that the patch in question does not know or care 
how the current connection's interval was configured.  It could be from 
a global setting, or it could be device-specific configuration.  All it 
knows is that the current connection has min/max values, and if the 
remote device requests to change the connection interval, the requested 
value should be checked against the min/max bounds of that connection.

If the desired behavior were for Linux to blindly accept and return a 
successful response for any connection interval change request value, 
then it makes no sense to have a min/max at all. The min/max would 
essentially be any value allowed by the bluetooth spec, and having the 
notion of a min/max connection interval is confusing and misleading.  I 
believe that if you have bounds defined, then values should be checked 
against those bounds.

Regards,

Carey


On 8/20/19 12:45 AM, Jamie Mccrae wrote:
> Hi,
>  From my many years' experience of embedded Bluetooth LE development I
> would say that offering different connection intervals to different
> devices is a must. If you have 3 devices connected to a laptop, for
> example 1 device in DFU mode, 1 keyboard and 1 temperature sensor, you
> would want vastly different connection interval settings for all of
> these. To speed up a firmware update process on the DFU device you would
> want the lowest connection interval (and I can speak from experience
> having seen an embedded linux SoC try to update a device over DFU via
> Bluetooth LE on Bluez 4.x and DBUS that it took in excess of 30 minutes
> per device, compared to 1 minute on an android phone). They keyboard is
> likely to negotiate a slave latency so that it can stay idle if no keys
> are pressed for many connection intervals but if something is
> pressed then it can respond fast, and the temperature sensor is going to
> have a much larger connection interval, if the sensor itself gathers a
> new reading every minute, then a connection interval of 30 seconds would
> be fine. So to make things easy for both developers and users, how would
> you go about doing that? Simply having a single 'minimum' and 'maximum'
> connection interval in a file or hard-coded is not a workable solution
> and does nothing to help user experience.
> Thanks,
> Jamie
>
>> Andreas,
>>
>> Considering that this patch has already made it's way into stable trees,
>> and the only options are to back it out or to patch it again, I would
>> obviously prefer to keep it in and patch it again.  I think that it
>> would be fairly quick and painless to just set the default min/max to
>> 6/3200, or add an additional flag as you suggested, which should resolve
>> any problems in the near term.
>>
>> Longer term, I think that there are several ways to allow the system
>> admin to configure the allowable min/max, but it only matters if the
>> system will check requests against the configured min/max and respond
>> appropriately, which is what the patch in question does.  The best
>> current way that I'm aware of to control the system-wide connection
>> interval is through the existing debugfs mechanism, and in my opinion it
>> didn't work correctly without this patch.
>>
>> Regarding power consumption, I have not done any power measurement
>> testing to determine the effect of connection interval on power
>> consumption, so perhaps that was not the best use case.  Here is the
>> exact real world use case that caused me to write the patch in the first
>> place:
>>
>> I was writing a Linux-based test system for an embedded
>> BLE-communication based product.  The product embedded code was written
>> to attempt to renegotiate the connection interval to a fairly low number
>> after the initial connection was established. My test system had a
>> requirement to be able to perform the tests using various pre-defined
>> connection intervals so that I could gather data throughput and product
>> performance metrics.  Every time my test system attempted to perform a
>> test a the desired connection interval, the device would immediately
>> request to renegiate to a lower value.  Linux would accept that request
>> and return a successful response, and I had no way to stop it.  Let's
>> not go down the "Why didn't you just change the embedded code?" / "Why
>> doesn't Linux reject values outside of the configured bounds?" rabbit
>> hole...  :)
>>
>> Carey
>>
>> On 8/19/19 10:08 AM, Andreas Kemnade wrote:
>>> Hi Carey,
>>>
>>> On Mon, 19 Aug 2019 07:58:19 -0600
>>> Carey Sonsino <csonsino@gmail.com> wrote:
>>>
>>>> This seems like the exact "downside" situation that I described in the
>>>> patch writeup.
>>>>
>>>> I would still claim that as a Linux system administrator, I should have
>>>> control over my system.  If I am operating in a low power environment, I
>>>> do not want to allow a remote device to apply a setting which causes me
>>>> to use more power without any say in the matter.
>>>>
>>> In principle I agree here. High connection interval has its downsides,
>>> low connection interval also. Just curios: What are the numbers about
>>> power consumption here? A few mA? I have only compared these values on
>>> peripherals running on low-power SoCs like e.g. the nrf stuff from nordic.
>>> I see around 1 mA difference there with a power consumption besides of that
>>> usually measured in the µA range. Never tested these things on a linux machine.
>>>
>>> The point here is that with this patch there is insufficient control
>>> about this. Yes, there is the debugfs interface.
>>>
>>> But if you want to provide a driver to a gatt service living on top of
>>> bluetoothd dbus api? Ask people to not use distribution kernels?
>>> What options do you have?
>>> using the monitor interface to sniff the connection handle and then
>>> call hci_le_conn_update() to set things?
>>>
>>>> The connection min/max interval settings are configuration options that
>>>> control how my bluetooth device operates.  Why are these down in debugfs
>>>> anyway?  I think that a much more appropriate fix would be to migrate
>>>> some of the BLE configuration options to sysconfdir (some place like
>>>> /etc/bluetooth/ble.conf).  That would also help in the persistence of
>>>> these configuration settings, which is kind of a pain with the debugfs
>>>> mechanism that gets wiped out and recreated on bootup.
>>>>
>>> I think that these things should be part of the  dbus api provided
>>> by bluetoothd so that a driver could decide and having defaults outside
>>> of such a dark corner like the debug fs.
>>>
>>>> A quicker fix would be to simply set the default connection min interval
>>>> and connection max interval values to the full range (6, 3200).
>>> Or just maybe a flag allowing such behavior?
>>>
>>>> *think* that this could be done by simply updating the values in
>>>> hci_core.c, the hci_alloc_dev() function:
>>>>
>>>>        hdev->le_conn_min_interval = 0x0018;
>>>>        hdev->le_conn_max_interval = 0x0028;
>>>>
>>>> would become:
>>>>
>>>>        hdev->le_conn_min_interval = 0x0006;
>>>>        hdev->le_conn_max_interval = 0x0c80;
>>>>
>>>> This should allow all devices to negotiate whatever connection interval
>>>> they want by default.  If I'm running on a device with debugfs (which I
>>>> happen to be most of the time), then I can still override these defaults
>>>> to control my system.
>>>>
>>>> Please let me know if you would like me to do any further work towards
>>>> resolving this issue.  I'd be happy to test and submit a patch that
>>>> changes the default connection min/max interval values- I could probably
>>>> get that done in the next day or two.  If you would like me to
>>>> investigate migrating configuration settings to /etc then I'd be happy
>>>> to do that as well, but it might take a bit more effort and time.
>>>>
>>> Well, all these things are important, but are new features but there is a
>>> problem:
>>> The kernel patch has gone into the stable trees and from there into distributions,
>>> so how can these new features flow down the same path.
>>>
>>> Regards,
>>> Andreas
>>>