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

[Andreas Kemnade <andreas@kemnade.info>]

[-- 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 --]