Currently we silently ignore missing connection (attrib) in read_char(),
but not in the other GATT interfaces (such as write_char, discover_desc,
etc). The code should avoid calling read_char() when there is no active
connection instead, and logging errors will help us trace the offenders.
We need to have active connection to fully discover a HOG instance,
and in the chain
bt_hog_new()->
gatt_db_foreach_service()->
foreach_hog_service()->
hog_attach_instance()
we have not set up hog->attrib yet. So let's skip calling
foreach_hog_chrc() from hog_attach_instance(), especially since
we will be calling bt_hog_attach() pretty much immediately after
bt_hog_new(), and we will be discovering characteristics there anyway.
When calling gatt_write_char(), gatt_read_char(), etc, id == 0 indicates
error. Let's recognize this fact and log it instead of queueing request
that will never be completed.
If UHID_GET_REPORT is received but a report cannot be found, etc, the
would pass bt_hog as user_data instead of report to get_report_cb
leading to a crash.
Fixes https://github.com/bluez/bluez/issues/112
When cable pairing a PS3 clone device, we should try and keep the USB device
name to create a new btd_device so that the joypad is named after its USB name
when connecting through Bluetooth.
If that isn't done, "Shanwan" clone joypads are named like the genuine joypads, and
kernel Bluetooth quirks aren't applied.
gh-issue: https://github.com/bluez/bluez/issues/46
In commit 23b69ab3e4 ("input/hog: Cache the HID report map"), we
optimized HOG reconnection by registering report value callbacks early,
but there was a bug where we also re-register the same report value
callbacks after at CCC write callback. We should handle this case by
avoiding the second callback register if we know we have done it
earlier.
To optimize BLE HID devices reconnection response, we can cache the
report map so that the subsequent reconnections do not need round trip
time to read the report map.
This reverts commit d6cafa1f0c.
In commit d6cafa1f0c ("input/hog: Remove HID device after HoG device
disconnects"), the bt_hog structure is destroyed in order to fix a bug
where the UHID connection is not destroyed. This fix has the cost of
increasing reconnection time because every reconnection would need to
re-read the report map again. An improvement to this fix is, instead of
removing the bt_hog structure, we can just destroy the UHID with
UHID_DESTROY event and use the existing bt_hog structure to keep the
cache of the report map to avoid re-reading the report map at
reconnection.
If the HID subsystem requests a HID report to be read from the
device, we currently incorrectly strip off the first byte of the
response, if the device has report IDs set in the HID report
descriptor.
This is incorrect; unlike USB HID, the report ID is *not* included
in the HOG profile's HID reports, and instead exists out of band
in a descriptor on the report's bluetooth characteristic in the
device.
In this patch, we remove the erroneous stripping of the first
byte of the report, and (if report IDs are enabled) prepend the
report ID to the front of the result. This makes the HID report
returned indentical in format to that of a USB HID report, so
that the upper HID drivers can consume HOG device reports in the
same way as USB.
When destroying UHID, we should also unregister all event listeners so
that they don't get double registered at reconnection. It fixes a bug
where battery report is not available to kernel after reconnection and
also prevents memory leak.
Tested with Logitech M535 mouse:
* Connect mouse to the device running BlueZ
* cat /sys/class/power_supply/hid-{addr}-battery/capacity # works
* Disconnect mouse
* Reconnect mouse
* cat /sys/class/power_supply/hid-{addr}-battery/capacity # still works
According to the HID1.1 spec, part 5.3.4.9:
The HIDSDPDisable attribute is a Boolean value, which indicates
whether connection to the SDP channel and Control or Interrupt
channels are mutually exclusive. This feature supports Bluetooth
HID devices that have minimal resources, and multiplex those
resources between servicing the initialization (SDP) and runtime
(Control and Interrupt) channels.
However, Bluez still tries to connect SDP upon HID connection,
regardless of the existence of the HIDSDPDisable attribute.
This patch prevents the connection of SDP after HID has been
established, if the device has HIDSDPDisable attribute.
This patch listens to UHID_SET_REPORT event and forwards this
message to the hid device. Upon reply, we also send a report back
to the kernel as UHID_SET_REPORT_REPLY.
hidp_send_set_report no longer listen UHID_OUTPUT events, that is
handled by hidp_send_output instead.
From Bluetooth HID Profile 1.1 Spec: If a Virtual Cable is
unplugged via a HID control Virtual Unplug command, then both the
Bluetooth HID device and Bluetooth HID Host shall destroy or
invalidate all Bluetooth bonding and Virtual Cable information
that was previously stored in persistent memory for the respective
Virtually Cabled devices and hosts.
This patch removes the bonding information upon receiving and/or
sending a "virtual cable unplug".
According to bluetooth HID1.1 spec, section 5.4.3.5.3:
If the Bluetooth HID Host is bonded to a Bluetooth HID device:
If encryption is not already enabled, the Bluetooth HID Host shall
enable encryption with the Bluetooth HID device before sending an
L2CAP Connect Response with a result code of “Connection Successful”
(0x0000) after an L2CAP Connect Request is received.
This patch raises the security level to medium when listening for
incoming connection if the flag classic_bonded_only is set,
effectively starting encryption.
According to bluetooth HID1.1 spec, part 5.4.3.4.3:
If the Bluetooth HID Host is bonded to a Bluetooth HID device:
If encryption is not already enabled, the Bluetooth HID Host shall
enable encryption with the Bluetooth HID device before sending an
L2CAP Connect Request to open the HID L2CAP Control channel.
When creating connection, this patch checks whether the target
device is bonded, if yes then we use the medium security level
instead of the low one to enable encryption.
HID devices can wake the host from a suspended state. Mark the profiles
to support wake when they are accepted. If the device hasn't already
been configured with a Wake Allowed configuration, it will default to
yes when the profile is accepted.
If the intr channel was disconnected by the other party, then they
are also responsible to close the ctrl channel. Such disconnection
message would have the G_IO_ERR flag set, as opposed to it being
unset if the disconnection is initiated by us.
There doesn't seem to be an explicit rule in the specification
about this behavior, but this is enforced in the PTS qualification
tool.
According to bluetooth HID Profile spec Ver 1.0, section 7.2.2, A
host or device shall always complete the disconnection of the
interrupt channel before disconnecting the control channel.
However, the current implementation disconnects them both
simultaneously.
This patch postpone the disconnection of control channel to the
callback of interrupt watch, which shall be called upon receiving
interrupt channel disconnection response.
Set the the correct vendor and product ids for all UHID/HoG
devices when they are unknown at HoG creation time.
Before this change, when connecting a BT device with multiple HoG
services for the first time, only the first HoG instance's vendor,
product and version fields would be set by the DIS callback. This meant
that all HoG instances except the first would be left with unset values
and their UHID devices would then be created with '0000:0000' as their
vendor:product ids.
To avoid a double hog free, need to add a ref
when adding the hog to the slist.
This bug has been reproduced with gamepad-8718
which was connecting/disconnecting frantically.
Fix also a typo in the method hog_attach_instance
LEAutoSecurity can be used to enable/disable automatic upgrades of
security for LE devices, by default it is enabled so existing devices
that did not require security and were not bonded will automatically
upgrade the security.
Note: Platforms disabling this setting would require users to manually
bond the device which may require changes to the user interface to
always force bonding for input devices as APIs such as Device.Connect
will no longer work which maybe perceived as a regression.
This attempts to set the security if the device is not bonded, the
kernel will block any communication on the ATT socket while bumping
the security and if that fails the device will be disconnected which
is better than having the device dangling around without being able to
communicate with it until it is properly bonded.
This change adds a configuration for platforms to choose a more secure
posture for the HID profile. While some older mice are known to not
support pairing or encryption, some platform may choose a more secure
posture by requiring the device to be bonded and require the
connection to be encrypted when bonding is required.
Reference:
https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-00352.html
Update uhid and uinput devices with lowercase addresses (to match how
kernel prints it via %pMR). Also update uinput to include the phys
attribute and correctly set the vendor/product/version during init.
When the Bluetooth LE device disconnects, make sure to also destroy the
uHID device so that we don't have a lingering HID device accessible from
user-space.
This also fixes the input subsystem never seeing the device reattaching,
causing settings that should be applied on connection not to be applied.
https://bugzilla.kernel.org/show_bug.cgi?id=202909
Tested-by: Bastien Nocera <hadess@hadess.net>
This fixes the following error:
profiles/input/device.c: In function ‘hidp_add_connection’:
profiles/input/device.c:677:47: error: ‘%s’ directive output may be truncated writing up to 127 bytes into a region of size between 0 and 127 [-Werror=format-truncation=]
snprintf(req->name, sizeof(req->name), "%s %s",
^~
pname, sdesc);
~~~~~
profiles/input/device.c:677:4: note: ‘snprintf’ output between 2 and 256 bytes into a destination of size 128
snprintf(req->name, sizeof(req->name), "%s %s",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pname, sdesc);
When a reconnect timeout has previously run for a device, setting a new
one will throw a warning as the timeout id is not reset when the timeout
is cancelled from within its callback.
bluetoothd[7731]: GLib: Source ID 526 was not found when attempting to remove it
bluetoothd[7731]: ++++++++ backtrace ++++++++
bluetoothd[7731]: #1 g_logv+0x25d (/usr/lib64/libglib-2.0.so.0.5400.1) [0x50d5a7d]
bluetoothd[7731]: #2 g_log+0x8f (/usr/lib64/libglib-2.0.so.0.5400.1) [0x50d5bef]
bluetoothd[7731]: #3 g_source_remove+0x7c (/usr/lib64/libglib-2.0.so.0.5400.1) [0x50cd8dc]
bluetoothd[7731]: #4 intr_watch_cb+0x1b5 (profiles/input/device.c:1217) [0x42f465]
bluetoothd[7731]: #5 g_main_context_dispatch+0x157 (/usr/lib64/libglib-2.0.so.0.5400.1) [0x50cebb7]
bluetoothd[7731]: #6 g_main_context_iterate.isra.25+0x200 (/usr/lib64/libglib-2.0.so.0.5400.1) [0x50cef60]
bluetoothd[7731]: #7 g_main_loop_run+0xc2 (/usr/lib64/libglib-2.0.so.0.5400.1) [0x50cf272]
bluetoothd[7731]: #8 main+0x839 (src/main.c:772) [0x40bd69]
bluetoothd[7731]: #9 __libc_start_main+0xe7 (../csu/libc-start.c:340) [0x5d12187]
bluetoothd[7731]: #10 _start+0x2a (/home/hadess/Projects/jhbuild/bluez/src/bluetoothd) [0x40c59a]
bluetoothd[7731]: +++++++++++++++++++++++++++
This fixes the following problems:
plugins/sixaxis.c:443:7: error: ‘version’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (!setup_device(fd, sysfs_path, name, source, vid, pid, version, type, adapter))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
plugins/sixaxis.c:409:34: note: ‘version’ was declared here
uint16_t bus, vid, pid, source, version;
^~~~~~~
plugins/sixaxis.c:443:7: error: ‘source’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (!setup_device(fd, sysfs_path, name, source, vid, pid, version, type, adapter))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
plugins/sixaxis.c:409:26: note: ‘source’ was declared here
uint16_t bus, vid, pid, source, version;
^~~~~~
cc1: all warnings being treated as errors
And many instances of code going over 80 columns.
Move the struct containing the Sixaxis-compatible devices to a
header shared with the input profiles code, so as to reduce device
declaration.
Adding support for new devices should be as easy as adding the device's
declaration in profiles/input/sixaxis.h
This add support of passing a gatt-db to avoid having to discover the
services again, this should also make it easier to port to bt_gatt_client
once Android code support it.
This add support of passing a gatt-db to avoid having to discover the
services again, this should also make it easier to port to bt_gatt_client
once Android code support it.
I believe I have identified an issue with the HID-over-GATT (HoG) where,
through hidraw, the HIDIOCGFEATURE does not work correctly.
The symptom is that the ioctl call returns immediately with bogus data,
before the Read Request to the peripheral has been completed.
I believe the issue is caused by the hog-lib.c registering a handler for
both UHID_FEATURE and UHID_GET_REPORT events, which in the uhid header
file turn out to be the same enum.
This causes the get_report() to get called first, it issues the Read
Request and waits for it's completion. After this the get_feature() is
immediately called with the same uhid message, which sends the
UHID_FEATURE_ANSWER in to the kernel with stale data, which then gets
returned to the hidraw caller.
I have fixed this by removing the get_feature() as it is unnecessary
anyway. See attached patch.
I have tested with against both old and new uhid API (kernels 3.8 and
4.4).
