* [PATCH BlueZ 4/4] gas: Fix not sending response to indication
From: Vinicius Costa Gomes @ 2013-01-28 23:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1359416891-12740-1-git-send-email-vinicius.gomes@openbossa.org>
Even if the remote device is not bonded, we should send the response to the
indication. If we don't the remote device may disconnect.
---
profiles/gatt/gas.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index e0edbf3..8477dca 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -184,16 +184,16 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
DBG("Service Changed start: 0x%04X end: 0x%04X", start, end);
- if (device_is_bonded(gas->device) == FALSE) {
- DBG("Ignoring Service Changed: device is not bonded");
- return;
- }
-
/* Confirming indication received */
opdu = g_attrib_get_buffer(gas->attrib, &plen);
olen = enc_confirmation(opdu, plen);
g_attrib_send(gas->attrib, 0, opdu, olen, NULL, NULL, NULL);
+ if (device_is_bonded(gas->device) == FALSE) {
+ DBG("Ignoring Service Changed: device is not bonded");
+ return;
+ }
+
btd_device_gatt_set_service_changed(gas->device, start, end);
}
--
1.8.1.1
^ permalink raw reply related
* [PATCH] Bluetooth: Fix handling of unexpected SMP PDUs
From: Johan Hedberg @ 2013-01-29 0:04 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@intel.com>
The hdev->smp_chan pointer can be NULL if SMP PDUs arrive at unexpected
moments. To avoid NULL pointer dereferences the code should be checking
for this and simply ignore such PDUs. This patch fixes the issue by
adding the checks into each individual PDU handler. It's done there
instead of a global place since for some PDUs it *is* ok for smp_chan to
be NULL (e.g. pairing request and security request).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
CC: stable@vger.kernel.org
---
net/bluetooth/smp.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 68a9587..30b58a0 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -630,6 +630,9 @@ static u8 smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
BT_DBG("conn %p", conn);
+ if (!smp)
+ return 0;
+
if (!(conn->hcon->link_mode & HCI_LM_MASTER))
return SMP_CMD_NOTSUPP;
@@ -676,6 +679,9 @@ static u8 smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
BT_DBG("conn %p %s", conn, conn->hcon->out ? "master" : "slave");
+ if (!smp)
+ return 0;
+
memcpy(smp->pcnf, skb->data, sizeof(smp->pcnf));
skb_pull(skb, sizeof(smp->pcnf));
@@ -699,6 +705,9 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
struct smp_chan *smp = conn->smp_chan;
struct hci_dev *hdev = conn->hcon->hdev;
+ if (!smp)
+ return 0;
+
BT_DBG("conn %p", conn);
swap128(skb->data, smp->rrnd);
@@ -817,6 +826,9 @@ static int smp_cmd_encrypt_info(struct l2cap_conn *conn, struct sk_buff *skb)
struct smp_cmd_encrypt_info *rp = (void *) skb->data;
struct smp_chan *smp = conn->smp_chan;
+ if (!smp)
+ return 0;
+
skb_pull(skb, sizeof(*rp));
memcpy(smp->tk, rp->ltk, sizeof(smp->tk));
@@ -832,6 +844,9 @@ static int smp_cmd_master_ident(struct l2cap_conn *conn, struct sk_buff *skb)
struct hci_conn *hcon = conn->hcon;
u8 authenticated;
+ if (!smp)
+ return 0;
+
skb_pull(skb, sizeof(*rp));
hci_dev_lock(hdev);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Fix handling of unexpected SMP PDUs
From: Marcel Holtmann @ 2013-01-29 6:50 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359417846-5064-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> The hdev->smp_chan pointer can be NULL if SMP PDUs arrive at unexpected
> moments. To avoid NULL pointer dereferences the code should be checking
> for this and simply ignore such PDUs. This patch fixes the issue by
> adding the checks into each individual PDU handler. It's done there
> instead of a global place since for some PDUs it *is* ok for smp_chan to
> be NULL (e.g. pairing request and security request).
I am not sure we want to ignore such PDUs. Don't we have to respond with
an error and actually disconnect at this point. Otherwise this might
open up a denial of service attack.
Regards
Marcel
^ permalink raw reply
* [PATCH 1/2] neard: Fix passing negative error code to strerror
From: Szymon Janc @ 2013-01-29 8:54 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
error_reply expects non-negative error code.
---
plugins/neard.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plugins/neard.c b/plugins/neard.c
index 380eddc..a68500a 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -636,7 +636,7 @@ static DBusMessage *push_oob(DBusConnection *conn, DBusMessage *msg, void *data)
agent = adapter_get_agent(adapter);
if (!agent)
- return error_reply(msg, -ENONET);
+ return error_reply(msg, ENONET);
io_cap = agent_get_io_capability(agent);
agent_unref(agent);
--
1.8.1.1
^ permalink raw reply related
* [PATCH 2/2] neard: Fix memory leak on registering as agent
From: Szymon Janc @ 2013-01-29 8:54 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1359449671-14584-1-git-send-email-szymon.janc@tieto.com>
Message reference was not dropped in register_agent. This fix following
memory leak reported by valgrind:
454 (184 direct, 270 indirect) bytes in 1 blocks are definitely lost in loss record 207 of 220
at 0x4C29DB4: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x513DCF2: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514222E: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5149F46: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514A070: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514AA63: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514B0A5: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5149E0C: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5134D24: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5136088: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5135643: dbus_connection_send_with_reply_and_block (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5130C93: dbus_bus_register (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
102 bytes in 1 blocks are indirectly lost in loss record 154 of 220
at 0x4C2B7B2: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x514F02F: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514F0DD: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514F239: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514DE0A: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514E3D3: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x513C138: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x513FF4D: dbus_message_iter_append_basic (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5141790: dbus_message_new_error (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5135C70: dbus_connection_dispatch (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x40A747: message_dispatch (mainloop.c:76)
by 0x4E7A91A: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.3)
168 bytes in 1 blocks are indirectly lost in loss record 185 of 220
at 0x4C2B7B2: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x514F02F: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514F0DD: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514F239: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x513A3B3: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514228F: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5149F46: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514A070: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514AA63: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x514B0A5: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5149E0C: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
by 0x5134D24: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.5.8)
---
plugins/neard.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/plugins/neard.c b/plugins/neard.c
index a68500a..b0150e9 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -121,12 +121,15 @@ static void register_agent(void)
if (!dbus_connection_send_with_reply(btd_get_dbus_connection(),
message, &call, -1)) {
+ dbus_message_unref(message);
error("D-Bus send failed");
return;
}
dbus_pending_call_set_notify(call, register_agent_cb, NULL, NULL);
dbus_pending_call_unref(call);
+
+ dbus_message_unref(message);
}
static void unregister_agent(void)
--
1.8.1.1
^ permalink raw reply related
* [PATCH v2] Bluetooth: Fix handling of unexpected SMP PDUs
From: Johan Hedberg @ 2013-01-29 16:44 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@intel.com>
The conn->smp_chan pointer can be NULL if SMP PDUs arrive at unexpected
moments. To avoid NULL pointer dereferences the code should be checking
for this and disconnect if an unexpected SMP PDU arrives. This patch
fixes the issue by adding a check for conn->smp_chan for all other PDUs
except pairing request and security request (which are are the first
PDUs to come to initialize the SMP context).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
CC: stable@vger.kernel.org
---
v2: Move the checks to a single place in smp_sig_channel() and instead
of ignoring the PDUs return failure from smp_sig_channel() to trigger a
disconnection.
net/bluetooth/smp.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 68a9587..5abefb1 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -859,6 +859,19 @@ int smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
skb_pull(skb, sizeof(code));
+ /*
+ * The SMP context must be initialized for all other PDUs except
+ * pairing and security requests. If we get any other PDU when
+ * not initialized simply disconnect (done if this function
+ * returns an error).
+ */
+ if (code != SMP_CMD_PAIRING_REQ && code != SMP_CMD_SECURITY_REQ &&
+ !conn->smp_chan) {
+ BT_ERR("Unexpected SMP command 0x%02x. Disconnecting.", code);
+ kfree_skb(skb);
+ return -ENOTSUPP;
+ }
+
switch (code) {
case SMP_CMD_PAIRING_REQ:
reason = smp_cmd_pairing_req(conn, skb);
--
1.7.10.4
^ permalink raw reply related
* [PATCH] hidp: Make hidp_get_raw_report abort if the session is terminating
From: Karl Relton @ 2013-01-29 16:52 UTC (permalink / raw)
To: linux-bluetooth
From: Karl Relton <karllinuxtest.relton@ntlworld.com>
After linux 3.2 the hid_destroy_device call in hidp_session cleaning up
invokes a hook to the power_supply code which in turn tries to read the
battery capacity. This read will trigger a call to hidp_get_raw_report
which is bound to fail because the device is being taken away - so rather
than wait for the 5 second timeout failure this change enables it to fail
straight away.
Signed-off-by: Karl Relton <karllinuxtest.relton@ntlworld.com>
---
net/bluetooth/hidp/core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index b2bcbe2..a4c1bb0 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -311,6 +311,9 @@ static int hidp_get_raw_report(struct hid_device *hid,
int numbered_reports = hid->report_enum[report_type].numbered;
int ret;
+ if (atomic_read(&session->terminate))
+ return -EIO;
+
switch (report_type) {
case HID_FEATURE_REPORT:
report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_FEATURE;
@@ -695,8 +698,10 @@ static int hidp_session(void *arg)
set_current_state(TASK_INTERRUPTIBLE);
while (!atomic_read(&session->terminate)) {
if (ctrl_sk->sk_state != BT_CONNECTED ||
- intr_sk->sk_state != BT_CONNECTED)
+ intr_sk->sk_state != BT_CONNECTED) {
+ atomic_inc(&session->terminate);
break;
+ }
while ((skb = skb_dequeue(&intr_sk->sk_receive_queue))) {
skb_orphan(skb);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Fix handling of unexpected SMP PDUs
From: Johan Hedberg @ 2013-01-29 17:19 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1359442248.16748.40.camel@aeonflux>
Hi Marcel,
On Tue, Jan 29, 2013, Marcel Holtmann wrote:
> > The hdev->smp_chan pointer can be NULL if SMP PDUs arrive at unexpected
> > moments. To avoid NULL pointer dereferences the code should be checking
> > for this and simply ignore such PDUs. This patch fixes the issue by
> > adding the checks into each individual PDU handler. It's done there
> > instead of a global place since for some PDUs it *is* ok for smp_chan to
> > be NULL (e.g. pairing request and security request).
>
> I am not sure we want to ignore such PDUs. Don't we have to respond with
> an error and actually disconnect at this point. Otherwise this might
> open up a denial of service attack.
I couldn't figure out any appropriate response since SMP doesn't really
have clear command-response pairs for everything. I've sent another
patch which still doesn't send a response but instead of just ignoring
the unexpected packet a disconnection is triggered.
Johan
^ permalink raw reply
* Re: [PATCH v2] Bluetooth: Fix handling of unexpected SMP PDUs
From: Marcel Holtmann @ 2013-01-29 17:58 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359477863-24645-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> The conn->smp_chan pointer can be NULL if SMP PDUs arrive at unexpected
> moments. To avoid NULL pointer dereferences the code should be checking
> for this and disconnect if an unexpected SMP PDU arrives. This patch
> fixes the issue by adding a check for conn->smp_chan for all other PDUs
> except pairing request and security request (which are are the first
> PDUs to come to initialize the SMP context).
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> CC: stable@vger.kernel.org
> ---
> v2: Move the checks to a single place in smp_sig_channel() and instead
> of ignoring the PDUs return failure from smp_sig_channel() to trigger a
> disconnection.
>
> net/bluetooth/smp.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
this looks way better.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* [PATCH BlueZ] core: Fix g_source_remove() with zero ID while removing device
From: Anderson Lizardo @ 2013-01-29 18:52 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
store_device_info_cb() is also used as callback for g_idle_add() and
therefore sets device->store_id to zero. During device removal it may be
called manually, which must be done only after the existing
device->store_id is removed from mainloop.
Fix this GLib error (and a bunch of invalid read/writes when
store_device_info_cb() was called after device removal due to this bug):
bluetoothd[1192]: src/device.c:device_remove() Removing device
/org/bluez/hci0/dev_12_34_12_34_12_34
(bluetoothd:1192): GLib-CRITICAL **: g_source_remove: assertion `tag >
0' failed
bluetoothd[1192]: src/device.c:btd_device_unref() Freeing device
/org/bluez/hci0/dev_12_34_12_34_12_34
bluetoothd[1192]: src/device.c:device_free() 0x463a2a0
---
src/device.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/device.c b/src/device.c
index 34902b3..adf405a 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2187,11 +2187,11 @@ void device_remove(struct btd_device *device, gboolean remove_stored)
do_disconnect(device);
if (device->store_id > 0) {
- if (!remove_stored)
- store_device_info_cb(device);
-
g_source_remove(device->store_id);
device->store_id = 0;
+
+ if (!remove_stored)
+ store_device_info_cb(device);
}
if (remove_stored)
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ v2 1/4] device: Fix invalid memory access during Find Included
From: Vinicius Costa Gomes @ 2013-01-29 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
When doing the Find Included Services GATT procedure, the status of the ATT
procedure was being ignored, and in the case of a timeout it is possible to
crash bluetooth with an invalid memory access.
Valgrind log:
==1755== Invalid read of size 8
==1755== at 0x46971A: find_included_cb (device.c:2964)
==1755== by 0x4465AE: isd_unref (gatt.c:92)
==1755== by 0x446885: find_included_cb (gatt.c:425)
==1755== by 0x448266: disconnect_timeout (gattrib.c:269)
==1755== by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x40A2EE: main (main.c:583)
==1755== Address 0x69530a8 is 8 bytes inside a block of size 64 free'd
==1755== at 0x4C2874F: free (vg_replace_malloc.c:446)
==1755== by 0x40BFA6: service_filter (watch.c:486)
==1755== by 0x40BC6A: message_filter (watch.c:554)
==1755== by 0x5160A1D: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.7.2)
==1755== by 0x40AAB7: message_dispatch (mainloop.c:76)
==1755== by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x40A2EE: main (main.c:583)
==1755==
==1755== Invalid read of size 8
==1755== at 0x4486D5: g_attrib_get_buffer (gattrib.c:657)
==1755== by 0x4467C5: find_included (gatt.c:363)
==1755== by 0x4465AE: isd_unref (gatt.c:92)
==1755== by 0x446885: find_included_cb (gatt.c:425)
==1755== by 0x448266: disconnect_timeout (gattrib.c:269)
==1755== by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x40A2EE: main (main.c:583)
==1755== Address 0x18 is not stack'd, malloc'd or (recently) free'd
==1755==
==1755==
==1755== Process terminating with default action of signal 11 (SIGSEGV)
==1755== Access not within mapped region at address 0x18
==1755== at 0x4486D5: g_attrib_get_buffer (gattrib.c:657)
==1755== by 0x4467C5: find_included (gatt.c:363)
==1755== by 0x4465AE: isd_unref (gatt.c:92)
==1755== by 0x446885: find_included_cb (gatt.c:425)
==1755== by 0x448266: disconnect_timeout (gattrib.c:269)
==1755== by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755== by 0x40A2EE: main (main.c:583)
---
attrib/gatt.c | 5 ++++-
src/device.c | 6 ++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/attrib/gatt.c b/attrib/gatt.c
index d54feac..44d3eb6 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -89,7 +89,10 @@ static void isd_unref(struct included_discovery *isd)
if (g_atomic_int_dec_and_test(&isd->refs) == FALSE)
return;
- isd->cb(isd->includes, isd->err, isd->user_data);
+ if (isd->err)
+ isd->cb(NULL, isd->err, isd->user_data);
+ else
+ isd->cb(isd->includes, isd->err, isd->user_data);
g_slist_free_full(isd->includes, g_free);
g_attrib_unref(isd->attrib);
diff --git a/src/device.c b/src/device.c
index 34902b3..ceaa575 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2988,6 +2988,12 @@ static void find_included_cb(GSList *includes, uint8_t status,
struct gatt_primary *prim;
GSList *l;
+ if (status != 0) {
+ error("Find included services failed: %s (%d)",
+ att_ecode2str(status), status);
+ goto done;
+ }
+
if (includes == NULL)
goto done;
--
1.8.1.1
^ permalink raw reply related
* [PATCH BlueZ v2 2/4] gas: Move all the code to only one file
From: Vinicius Costa Gomes @ 2013-01-29 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1359486007-3273-1-git-send-email-vinicius.gomes@openbossa.org>
Our Generic Attribute/Access Service plugin is small and simple enough
to be kept in only one file.
---
Makefile.plugins | 4 +--
profiles/gatt/gas.c | 51 +++++++++++++++++++++++++++++--
profiles/gatt/gas.h | 25 ----------------
profiles/gatt/main.c | 47 -----------------------------
profiles/gatt/manager.c | 79 -------------------------------------------------
profiles/gatt/manager.h | 24 ---------------
6 files changed, 49 insertions(+), 181 deletions(-)
delete mode 100644 profiles/gatt/gas.h
delete mode 100644 profiles/gatt/main.c
delete mode 100644 profiles/gatt/manager.c
delete mode 100644 profiles/gatt/manager.h
diff --git a/Makefile.plugins b/Makefile.plugins
index faab011..f497782 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -69,9 +69,7 @@ builtin_sources += profiles/health/mcap_lib.h profiles/health/mcap_internal.h \
endif
builtin_modules += gatt
-builtin_sources += profiles/gatt/main.c profiles/gatt/manager.h \
- profiles/gatt/manager.c profiles/gatt/gas.h \
- profiles/gatt/gas.c
+builtin_sources += profiles/gatt/gas.c
builtin_modules += scanparam
builtin_sources += profiles/scanparam/scan.c
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index 429850b..c0520af 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -35,15 +35,16 @@
#include <btio/btio.h>
#include "lib/uuid.h"
+#include "plugin.h"
#include "adapter.h"
#include "device.h"
+#include "profile.h"
#include "attrib/att.h"
#include "attrib/gattrib.h"
#include "attio.h"
#include "attrib/gatt.h"
#include "log.h"
#include "textfile.h"
-#include "gas.h"
/* Generic Attribute/Access Service */
struct gas {
@@ -367,7 +368,7 @@ static void attio_disconnected_cb(gpointer user_data)
gas->attrib = NULL;
}
-int gas_register(struct btd_device *device, struct att_range *gap,
+static int gas_register(struct btd_device *device, struct att_range *gap,
struct att_range *gatt)
{
struct gas *gas;
@@ -392,7 +393,7 @@ int gas_register(struct btd_device *device, struct att_range *gap,
return 0;
}
-void gas_unregister(struct btd_device *device)
+static void gas_unregister(struct btd_device *device)
{
struct gas *gas;
GSList *l;
@@ -405,3 +406,47 @@ void gas_unregister(struct btd_device *device)
devices = g_slist_remove(devices, gas);
gas_free(gas);
}
+
+static int gatt_driver_probe(struct btd_profile *p, struct btd_device *device,
+ GSList *uuids)
+{
+ struct gatt_primary *gap, *gatt;
+
+ gap = btd_device_get_primary(device, GAP_UUID);
+ gatt = btd_device_get_primary(device, GATT_UUID);
+
+ if (gap == NULL || gatt == NULL) {
+ error("GAP and GATT are mandatory");
+ return -EINVAL;
+ }
+
+ return gas_register(device, &gap->range, &gatt->range);
+}
+
+static void gatt_driver_remove(struct btd_profile *p,
+ struct btd_device *device)
+{
+ gas_unregister(device);
+}
+
+static struct btd_profile gatt_profile = {
+ .name = "gap-gatt-profile",
+ .remote_uuids = BTD_UUIDS(GAP_UUID, GATT_UUID),
+ .device_probe = gatt_driver_probe,
+ .device_remove = gatt_driver_remove
+};
+
+static int gatt_init(void)
+{
+ btd_profile_register(&gatt_profile);
+
+ return 0;
+}
+
+static void gatt_exit(void)
+{
+ btd_profile_unregister(&gatt_profile);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(gatt, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+ gatt_init, gatt_exit)
diff --git a/profiles/gatt/gas.h b/profiles/gatt/gas.h
deleted file mode 100644
index 34853c7..0000000
--- a/profiles/gatt/gas.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- *
- * BlueZ - Bluetooth protocol stack for Linux
- *
- * Copyright (C) 2012 Instituto Nokia de Tecnologia - INdT
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- *
- */
-
-int gas_register(struct btd_device *device, struct att_range *gap,
- struct att_range *gatt);
-void gas_unregister(struct btd_device *device);
diff --git a/profiles/gatt/main.c b/profiles/gatt/main.c
deleted file mode 100644
index ecd4455..0000000
--- a/profiles/gatt/main.c
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- *
- * BlueZ - Bluetooth protocol stack for Linux
- *
- * Copyright (C) 2012 Instituto Nokia de Tecnologia - INdT
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <stdint.h>
-#include <glib.h>
-#include <errno.h>
-
-#include "plugin.h"
-#include "manager.h"
-#include "hcid.h"
-#include "log.h"
-
-static int gatt_init(void)
-{
- return gatt_manager_init();
-}
-
-static void gatt_exit(void)
-{
- gatt_manager_exit();
-}
-
-BLUETOOTH_PLUGIN_DEFINE(gatt, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
- gatt_init, gatt_exit)
diff --git a/profiles/gatt/manager.c b/profiles/gatt/manager.c
deleted file mode 100644
index 2f2bd14..0000000
--- a/profiles/gatt/manager.c
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- *
- * BlueZ - Bluetooth protocol stack for Linux
- *
- * Copyright (C) 2012 Instituto Nokia de Tecnologia - INdT
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif
-
-#include <glib.h>
-#include <errno.h>
-#include <stdbool.h>
-
-#include "lib/uuid.h"
-#include "adapter.h"
-#include "device.h"
-#include "profile.h"
-#include "attrib/att.h"
-#include "attrib/gattrib.h"
-#include "attrib/gatt.h"
-#include "gas.h"
-#include "log.h"
-#include "manager.h"
-
-static int gatt_driver_probe(struct btd_profile *p, struct btd_device *device,
- GSList *uuids)
-{
- struct gatt_primary *gap, *gatt;
-
- gap = btd_device_get_primary(device, GAP_UUID);
- gatt = btd_device_get_primary(device, GATT_UUID);
-
- if (gap == NULL || gatt == NULL) {
- error("GAP and GATT are mandatory");
- return -EINVAL;
- }
-
- return gas_register(device, &gap->range, &gatt->range);
-}
-
-static void gatt_driver_remove(struct btd_profile *p,
- struct btd_device *device)
-{
- gas_unregister(device);
-}
-
-static struct btd_profile gatt_profile = {
- .name = "gap-gatt-profile",
- .remote_uuids = BTD_UUIDS(GAP_UUID, GATT_UUID),
- .device_probe = gatt_driver_probe,
- .device_remove = gatt_driver_remove
-};
-
-int gatt_manager_init(void)
-{
- return btd_profile_register(&gatt_profile);
-}
-
-void gatt_manager_exit(void)
-{
- btd_profile_unregister(&gatt_profile);
-}
diff --git a/profiles/gatt/manager.h b/profiles/gatt/manager.h
deleted file mode 100644
index 502fceb..0000000
--- a/profiles/gatt/manager.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- *
- * BlueZ - Bluetooth protocol stack for Linux
- *
- * Copyright (C) 2012 Instituto Nokia de Tecnologia - INdT
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- *
- */
-
-int gatt_manager_init(void);
-void gatt_manager_exit(void);
--
1.8.1.1
^ permalink raw reply related
* [PATCH BlueZ v2 3/4] gas: Fix not sending response to indication
From: Vinicius Costa Gomes @ 2013-01-29 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1359486007-3273-1-git-send-email-vinicius.gomes@openbossa.org>
Even if the remote device is not bonded, we should send the response to the
indication. If we don't the remote device may disconnect.
---
profiles/gatt/gas.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index c0520af..9360201 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -183,16 +183,16 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
DBG("Service Changed start: 0x%04X end: 0x%04X", start, end);
- if (device_is_bonded(gas->device) == FALSE) {
- DBG("Ignoring Service Changed: device is not bonded");
- return;
- }
-
/* Confirming indication received */
opdu = g_attrib_get_buffer(gas->attrib, &plen);
olen = enc_confirmation(opdu, plen);
g_attrib_send(gas->attrib, 0, opdu, olen, NULL, NULL, NULL);
+ if (device_is_bonded(gas->device) == FALSE) {
+ DBG("Ignoring Service Changed: device is not bonded");
+ return;
+ }
+
btd_device_gatt_set_service_changed(gas->device, start, end);
}
--
1.8.1.1
^ permalink raw reply related
* [PATCH BlueZ v2 4/4] device: Fix missing PDUs during encryption procedure
From: Vinicius Costa Gomes @ 2013-01-29 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1359486007-3273-1-git-send-email-vinicius.gomes@openbossa.org>
In case the remote device sends an ATT PDU while encryption is going
on, we may lose it because the ATT socket (with security level medium),
would only be attached when encryption finishes.
---
src/device.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/device.c b/src/device.c
index ceaa575..0d2d3ee 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3167,7 +3167,6 @@ int device_connect_le(struct btd_device *dev)
{
struct btd_adapter *adapter = dev->adapter;
struct att_callbacks *attcb;
- BtIOSecLevel sec_level;
GIOChannel *io;
GError *gerr = NULL;
char addr[18];
@@ -3185,21 +3184,18 @@ int device_connect_le(struct btd_device *dev)
attcb->success = att_success_cb;
attcb->user_data = dev;
- if (dev->paired)
- sec_level = BT_IO_SEC_MEDIUM;
- else
- sec_level = BT_IO_SEC_LOW;
-
/*
* This connection will help us catch any PDUs that comes before
- * pairing finishes
+ * pairing finishes. Its security level is low, because we don't
+ * want to miss any PDU that may come before the encryption
+ * procedure finishes
*/
io = bt_io_connect(att_connect_cb, attcb, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, adapter_get_address(adapter),
BT_IO_OPT_DEST_BDADDR, &dev->bdaddr,
BT_IO_OPT_DEST_TYPE, dev->bdaddr_type,
BT_IO_OPT_CID, ATT_CID,
- BT_IO_OPT_SEC_LEVEL, sec_level,
+ BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
BT_IO_OPT_INVALID);
if (io == NULL) {
--
1.8.1.1
^ permalink raw reply related
* Re: [PATCH 1/2] neard: Fix passing negative error code to strerror
From: Johan Hedberg @ 2013-01-29 21:59 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1359449671-14584-1-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Tue, Jan 29, 2013, Szymon Janc wrote:
> error_reply expects non-negative error code.
> ---
> plugins/neard.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Both patches have been applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ] core: Fix g_source_remove() with zero ID while removing device
From: Johan Hedberg @ 2013-01-29 22:01 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1359485546-28790-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Lizardo,
On Tue, Jan 29, 2013, Anderson Lizardo wrote:
> store_device_info_cb() is also used as callback for g_idle_add() and
> therefore sets device->store_id to zero. During device removal it may be
> called manually, which must be done only after the existing
> device->store_id is removed from mainloop.
>
> Fix this GLib error (and a bunch of invalid read/writes when
> store_device_info_cb() was called after device removal due to this bug):
>
> bluetoothd[1192]: src/device.c:device_remove() Removing device
> /org/bluez/hci0/dev_12_34_12_34_12_34
>
> (bluetoothd:1192): GLib-CRITICAL **: g_source_remove: assertion `tag >
> 0' failed
> bluetoothd[1192]: src/device.c:btd_device_unref() Freeing device
> /org/bluez/hci0/dev_12_34_12_34_12_34
> bluetoothd[1192]: src/device.c:device_free() 0x463a2a0
> ---
> src/device.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ v2 1/4] device: Fix invalid memory access during Find Included
From: Johan Hedberg @ 2013-01-29 22:05 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359486007-3273-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Tue, Jan 29, 2013, Vinicius Costa Gomes wrote:
> When doing the Find Included Services GATT procedure, the status of the ATT
> procedure was being ignored, and in the case of a timeout it is possible to
> crash bluetooth with an invalid memory access.
>
> Valgrind log:
>
> ==1755== Invalid read of size 8
> ==1755== at 0x46971A: find_included_cb (device.c:2964)
> ==1755== by 0x4465AE: isd_unref (gatt.c:92)
> ==1755== by 0x446885: find_included_cb (gatt.c:425)
> ==1755== by 0x448266: disconnect_timeout (gattrib.c:269)
> ==1755== by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x40A2EE: main (main.c:583)
> ==1755== Address 0x69530a8 is 8 bytes inside a block of size 64 free'd
> ==1755== at 0x4C2874F: free (vg_replace_malloc.c:446)
> ==1755== by 0x40BFA6: service_filter (watch.c:486)
> ==1755== by 0x40BC6A: message_filter (watch.c:554)
> ==1755== by 0x5160A1D: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.7.2)
> ==1755== by 0x40AAB7: message_dispatch (mainloop.c:76)
> ==1755== by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x40A2EE: main (main.c:583)
> ==1755==
> ==1755== Invalid read of size 8
> ==1755== at 0x4486D5: g_attrib_get_buffer (gattrib.c:657)
> ==1755== by 0x4467C5: find_included (gatt.c:363)
> ==1755== by 0x4465AE: isd_unref (gatt.c:92)
> ==1755== by 0x446885: find_included_cb (gatt.c:425)
> ==1755== by 0x448266: disconnect_timeout (gattrib.c:269)
> ==1755== by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x40A2EE: main (main.c:583)
> ==1755== Address 0x18 is not stack'd, malloc'd or (recently) free'd
> ==1755==
> ==1755==
> ==1755== Process terminating with default action of signal 11 (SIGSEGV)
> ==1755== Access not within mapped region at address 0x18
> ==1755== at 0x4486D5: g_attrib_get_buffer (gattrib.c:657)
> ==1755== by 0x4467C5: find_included (gatt.c:363)
> ==1755== by 0x4465AE: isd_unref (gatt.c:92)
> ==1755== by 0x446885: find_included_cb (gatt.c:425)
> ==1755== by 0x448266: disconnect_timeout (gattrib.c:269)
> ==1755== by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==1755== by 0x40A2EE: main (main.c:583)
> ---
> attrib/gatt.c | 5 ++++-
> src/device.c | 6 ++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
Patches 1-3 have been applied. We still need to discuss 4/4 though since
1) We should always try to secure the link as well as possible (meaning
encrypt it if we have an LTK)
and
2) Even if we do try to encrypt the link the ATT socket still needs to
remain functional all the time since there could be PDUs coming and
going that do not need high security.
Johan
^ permalink raw reply
* [PATCH 1/5] Bluetooth: Fix L2CAP socket shutdown for LE connections
From: Andre Guedes @ 2013-01-29 22:59 UTC (permalink / raw)
To: linux-bluetooth
During the L2CAP socket shutdown, the LE connection is not terminated
as expected. This bug can be reproduced using l2test tool. Once the
LE connection is established, kill l2test and the LE connection will
not terminate.
This patch fixes hci_conn_disconnect function so it is able to
terminate LE connections.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
net/bluetooth/hci_conn.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..0492949 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -250,6 +250,7 @@ static void hci_conn_disconnect(struct hci_conn *conn)
switch (conn->type) {
case ACL_LINK:
+ case LE_LINK:
hci_acl_disconn(conn, reason);
break;
case AMP_LINK:
--
1.8.1.1
^ permalink raw reply related
* [PATCH 2/5] Bluetooth: Fix SCO socket shutdown
From: Andre Guedes @ 2013-01-29 22:59 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359500396-8184-1-git-send-email-andre.guedes@openbossa.org>
During the SCO socket shutdown, the connection is not terminated a
expected. This bug can be reproduced using scotest tool. Once the
connection is established, kill scotest and the connection will not
terminate.
This patch fixes hci_conn_disconnect function so it is able to
terminate SCO connections.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
net/bluetooth/hci_conn.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0492949..28cfa72 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -251,6 +251,8 @@ static void hci_conn_disconnect(struct hci_conn *conn)
switch (conn->type) {
case ACL_LINK:
case LE_LINK:
+ case SCO_LINK:
+ case ESCO_LINK:
hci_acl_disconn(conn, reason);
break;
case AMP_LINK:
--
1.8.1.1
^ permalink raw reply related
* [PATCH 3/5] Bluetooth: Refactor hci_conn_disconnect
From: Andre Guedes @ 2013-01-29 22:59 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359500396-8184-1-git-send-email-andre.guedes@openbossa.org>
This patch does a trivial refactoring in hci_conn_disconnect function.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
net/bluetooth/hci_conn.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 28cfa72..4925a02 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -249,15 +249,12 @@ static void hci_conn_disconnect(struct hci_conn *conn)
__u8 reason = hci_proto_disconn_ind(conn);
switch (conn->type) {
- case ACL_LINK:
- case LE_LINK:
- case SCO_LINK:
- case ESCO_LINK:
- hci_acl_disconn(conn, reason);
- break;
case AMP_LINK:
hci_amp_disconn(conn, reason);
break;
+ default:
+ hci_acl_disconn(conn, reason);
+ break;
}
}
--
1.8.1.1
^ permalink raw reply related
* [PATCH 4/5] Bluetooth: Rename hci_acl_disconn
From: Andre Guedes @ 2013-01-29 22:59 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359500396-8184-1-git-send-email-andre.guedes@openbossa.org>
As hci_acl_disconn function basically sends the HCI Disconnect Command
and it is used to disconnect ACL, SCO and LE links, renaming it to
hci_disconnect is more suitable.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
include/net/bluetooth/hci_core.h | 2 +-
net/bluetooth/hci_conn.c | 4 ++--
net/bluetooth/hci_core.c | 2 +-
net/bluetooth/hci_event.c | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 90cf75a..787d3b9 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -574,7 +574,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
return NULL;
}
-void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
+void hci_disconnect(struct hci_conn *conn, __u8 reason);
void hci_setup_sync(struct hci_conn *conn, __u16 handle);
void hci_sco_setup(struct hci_conn *conn, __u8 status);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 4925a02..b9f9016 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -117,7 +117,7 @@ static void hci_acl_create_connection_cancel(struct hci_conn *conn)
hci_send_cmd(conn->hdev, HCI_OP_CREATE_CONN_CANCEL, sizeof(cp), &cp);
}
-void hci_acl_disconn(struct hci_conn *conn, __u8 reason)
+void hci_disconnect(struct hci_conn *conn, __u8 reason)
{
struct hci_cp_disconnect cp;
@@ -253,7 +253,7 @@ static void hci_conn_disconnect(struct hci_conn *conn)
hci_amp_disconn(conn, reason);
break;
default:
- hci_acl_disconn(conn, reason);
+ hci_disconnect(conn, reason);
break;
}
}
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 618ca1a..388c456 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2398,7 +2398,7 @@ static void hci_link_tx_to(struct hci_dev *hdev, __u8 type)
if (c->type == type && c->sent) {
BT_ERR("%s killing stalled connection %pMR",
hdev->name, &c->dst);
- hci_acl_disconn(c, HCI_ERROR_REMOTE_USER_TERM);
+ hci_disconnect(c, HCI_ERROR_REMOTE_USER_TERM);
}
}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d4fcba6..5c78480 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2399,7 +2399,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
if (ev->status && conn->state == BT_CONNECTED) {
- hci_acl_disconn(conn, HCI_ERROR_AUTH_FAILURE);
+ hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
hci_conn_put(conn);
goto unlock;
}
@@ -3472,7 +3472,7 @@ static void hci_key_refresh_complete_evt(struct hci_dev *hdev,
clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
if (ev->status && conn->state == BT_CONNECTED) {
- hci_acl_disconn(conn, HCI_ERROR_AUTH_FAILURE);
+ hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
hci_conn_put(conn);
goto unlock;
}
--
1.8.1.1
^ permalink raw reply related
* [PATCH 5/5] Bluetooth: Reduce critical section in sco_conn_ready
From: Andre Guedes @ 2013-01-29 22:59 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359500396-8184-1-git-send-email-andre.guedes@openbossa.org>
This patch reduces the critical section protected by sco_conn_lock in
sco_conn_ready function. The lock is acquired only when it is really
needed.
This patch fixes the following lockdep warning which is generated
when the host terminates a SCO connection.
Today, this warning is a false positive. There is no way those
two threads reported by lockdep are running at the same time since
hdev->workqueue (where rx_work is queued) is single-thread. However,
if somehow this behavior is changed in future, we will have a
potential deadlock.
======================================================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc1+ #7 Not tainted
-------------------------------------------------------
kworker/u:1H/1018 is trying to acquire lock:
(&(&conn->lock)->rlock){+.+...}, at: [<ffffffffa0033ba6>] sco_chan_del+0x66/0x190 [bluetooth]
but task is already holding lock:
(slock-AF_BLUETOOTH-BTPROTO_SCO){+.+...}, at: [<ffffffffa0033d5a>] sco_conn_del+0x8a/0xe0 [bluetooth]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (slock-AF_BLUETOOTH-BTPROTO_SCO){+.+...}:
[<ffffffff81083011>] lock_acquire+0xb1/0xe0
[<ffffffff813efd01>] _raw_spin_lock+0x41/0x80
[<ffffffffa003436e>] sco_connect_cfm+0xbe/0x350 [bluetooth]
[<ffffffffa0015d6c>] hci_event_packet+0xd3c/0x29b0 [bluetooth]
[<ffffffffa0004583>] hci_rx_work+0x133/0x870 [bluetooth]
[<ffffffff8104d65f>] process_one_work+0x2bf/0x4f0
[<ffffffff81050022>] worker_thread+0x2b2/0x3e0
[<ffffffff81056021>] kthread+0xd1/0xe0
[<ffffffff813f14bc>] ret_from_fork+0x7c/0xb0
-> #0 (&(&conn->lock)->rlock){+.+...}:
[<ffffffff81082215>] __lock_acquire+0x1465/0x1c70
[<ffffffff81083011>] lock_acquire+0xb1/0xe0
[<ffffffff813efd01>] _raw_spin_lock+0x41/0x80
[<ffffffffa0033ba6>] sco_chan_del+0x66/0x190 [bluetooth]
[<ffffffffa0033d6d>] sco_conn_del+0x9d/0xe0 [bluetooth]
[<ffffffffa0034653>] sco_disconn_cfm+0x53/0x60 [bluetooth]
[<ffffffffa000fef3>] hci_disconn_complete_evt.isra.54+0x363/0x3c0 [bluetooth]
[<ffffffffa00150f7>] hci_event_packet+0xc7/0x29b0 [bluetooth]
[<ffffffffa0004583>] hci_rx_work+0x133/0x870 [bluetooth]
[<ffffffff8104d65f>] process_one_work+0x2bf/0x4f0
[<ffffffff81050022>] worker_thread+0x2b2/0x3e0
[<ffffffff81056021>] kthread+0xd1/0xe0
[<ffffffff813f14bc>] ret_from_fork+0x7c/0xb0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(slock-AF_BLUETOOTH-BTPROTO_SCO);
lock(&(&conn->lock)->rlock);
lock(slock-AF_BLUETOOTH-BTPROTO_SCO);
lock(&(&conn->lock)->rlock);
*** DEADLOCK ***
4 locks held by kworker/u:1H/1018:
#0: (hdev->name#2){.+.+.+}, at: [<ffffffff8104d5f8>] process_one_work+0x258/0x4f0
#1: ((&hdev->rx_work)){+.+.+.}, at: [<ffffffff8104d5f8>] process_one_work+0x258/0x4f0
#2: (&hdev->lock){+.+.+.}, at: [<ffffffffa000fbe9>] hci_disconn_complete_evt.isra.54+0x59/0x3c0 [bluetooth]
#3: (slock-AF_BLUETOOTH-BTPROTO_SCO){+.+...}, at: [<ffffffffa0033d5a>] sco_conn_del+0x8a/0xe0 [bluetooth]
stack backtrace:
Pid: 1018, comm: kworker/u:1H Not tainted 3.8.0-rc1+ #7
Call Trace:
[<ffffffff813e92f9>] print_circular_bug+0x1fb/0x20c
[<ffffffff81082215>] __lock_acquire+0x1465/0x1c70
[<ffffffff81083011>] lock_acquire+0xb1/0xe0
[<ffffffffa0033ba6>] ? sco_chan_del+0x66/0x190 [bluetooth]
[<ffffffff813efd01>] _raw_spin_lock+0x41/0x80
[<ffffffffa0033ba6>] ? sco_chan_del+0x66/0x190 [bluetooth]
[<ffffffffa0033ba6>] sco_chan_del+0x66/0x190 [bluetooth]
[<ffffffffa0033d6d>] sco_conn_del+0x9d/0xe0 [bluetooth]
[<ffffffffa0034653>] sco_disconn_cfm+0x53/0x60 [bluetooth]
[<ffffffffa000fef3>] hci_disconn_complete_evt.isra.54+0x363/0x3c0 [bluetooth]
[<ffffffffa000fbd0>] ? hci_disconn_complete_evt.isra.54+0x40/0x3c0 [bluetooth]
[<ffffffffa00150f7>] hci_event_packet+0xc7/0x29b0 [bluetooth]
[<ffffffff81202e90>] ? __dynamic_pr_debug+0x80/0x90
[<ffffffff8133ff7d>] ? kfree_skb+0x2d/0x40
[<ffffffffa0021644>] ? hci_send_to_monitor+0x1a4/0x1c0 [bluetooth]
[<ffffffffa0004583>] hci_rx_work+0x133/0x870 [bluetooth]
[<ffffffff8104d5f8>] ? process_one_work+0x258/0x4f0
[<ffffffff8104d65f>] process_one_work+0x2bf/0x4f0
[<ffffffff8104d5f8>] ? process_one_work+0x258/0x4f0
[<ffffffff8104fdc1>] ? worker_thread+0x51/0x3e0
[<ffffffffa0004450>] ? hci_tx_work+0x800/0x800 [bluetooth]
[<ffffffff81050022>] worker_thread+0x2b2/0x3e0
[<ffffffff8104fd70>] ? busy_worker_rebind_fn+0x100/0x100
[<ffffffff81056021>] kthread+0xd1/0xe0
[<ffffffff81055f50>] ? flush_kthread_worker+0xc0/0xc0
[<ffffffff813f14bc>] ret_from_fork+0x7c/0xb0
[<ffffffff81055f50>] ? flush_kthread_worker+0xc0/0xc0
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
This lockdep warning has been around for a long time. I could test
until Linux 3.4, but it seems it is older than that. However, in
current bluetooth-next, this warning has been masked by commit
53502d69be49e3dd5bc95ab0f2deeaea260bd617 which introduced a bug in
SCO socket shutdown routine.
The bug in SCO socket shutdown routine has been fixed by patch 02/05
from this patchset.
net/bluetooth/sco.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 531a93d..e435641 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -900,8 +900,6 @@ static void sco_conn_ready(struct sco_conn *conn)
BT_DBG("conn %p", conn);
- sco_conn_lock(conn);
-
if (sk) {
sco_sock_clear_timer(sk);
bh_lock_sock(sk);
@@ -909,9 +907,13 @@ static void sco_conn_ready(struct sco_conn *conn)
sk->sk_state_change(sk);
bh_unlock_sock(sk);
} else {
+ sco_conn_lock(conn);
+
parent = sco_get_sock_listen(conn->src);
- if (!parent)
- goto done;
+ if (!parent) {
+ sco_conn_unlock(conn);
+ return;
+ }
bh_lock_sock(parent);
@@ -919,7 +921,8 @@ static void sco_conn_ready(struct sco_conn *conn)
BTPROTO_SCO, GFP_ATOMIC);
if (!sk) {
bh_unlock_sock(parent);
- goto done;
+ sco_conn_unlock(conn);
+ return;
}
sco_sock_init(sk, parent);
@@ -939,10 +942,9 @@ static void sco_conn_ready(struct sco_conn *conn)
parent->sk_data_ready(parent, 1);
bh_unlock_sock(parent);
- }
-done:
- sco_conn_unlock(conn);
+ sco_conn_unlock(conn);
+ }
}
/* ----- SCO interface with lower layer (HCI) ----- */
--
1.8.1.1
^ permalink raw reply related
* Re: [PATCH] Add heartrate monitoring LE device to auto connect list.
From: Johan Hedberg @ 2013-01-29 23:36 UTC (permalink / raw)
To: Damjan Cvetko; +Cc: linux-bluetooth
In-Reply-To: <1359408442-8002-1-git-send-email-damjan.cvetko@monotek.net>
Hi Damjan,
On Mon, Jan 28, 2013, Damjan Cvetko wrote:
> ---
> profiles/heartrate/heartrate.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
> index 5c56d3f..1788d4f 100644
> --- a/profiles/heartrate/heartrate.c
> +++ b/profiles/heartrate/heartrate.c
> @@ -801,6 +801,8 @@ static int heartrate_device_register(struct btd_device *device,
> hr->attioid = btd_device_add_attio_callback(device, attio_connected_cb,
> attio_disconnected_cb, hr);
>
> + device_set_auto_connect(device, TRUE);
> +
> return 0;
> }
This patch isn't needed anymore since btd_device_add_attio_callback was
fixed to enable the auto connecting. Please verify that latest git works
for you.
Johan
^ permalink raw reply
* Re: [PATCH 3/5] Bluetooth: Refactor hci_conn_disconnect
From: Marcel Holtmann @ 2013-01-30 8:01 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1359500396-8184-3-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch does a trivial refactoring in hci_conn_disconnect function.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 28cfa72..4925a02 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -249,15 +249,12 @@ static void hci_conn_disconnect(struct hci_conn *conn)
> __u8 reason = hci_proto_disconn_ind(conn);
>
> switch (conn->type) {
> - case ACL_LINK:
> - case LE_LINK:
> - case SCO_LINK:
> - case ESCO_LINK:
> - hci_acl_disconn(conn, reason);
> - break;
> case AMP_LINK:
> hci_amp_disconn(conn, reason);
> break;
> + default:
> + hci_acl_disconn(conn, reason);
> + break;
> }
just make it one patch that fixes all cases. Trying to make 3 patches
out of this is just confusing. Rather write a proper commit message that
explains it all.
Regards
Marcel
^ permalink raw reply
* [PATCH v2 1/3] Bluetooth: Fix hci_conn timeout routine
From: Andre Guedes @ 2013-01-30 14:50 UTC (permalink / raw)
To: linux-bluetooth
If occurs a LE or SCO hci_conn timeout and the connection is already
established (BT_CONNECTED state), the connection is not terminated as
expected. This bug can be reproduced using l2test or scotest tool.
Once the connection is established, kill l2test/scotest and the
connection won't be terminated.
This patch fixes hci_conn_disconnect helper so it is able to
terminate LE and SCO connections, as well as ACL.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
net/bluetooth/hci_conn.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..4925a02 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -249,12 +249,12 @@ static void hci_conn_disconnect(struct hci_conn *conn)
__u8 reason = hci_proto_disconn_ind(conn);
switch (conn->type) {
- case ACL_LINK:
- hci_acl_disconn(conn, reason);
- break;
case AMP_LINK:
hci_amp_disconn(conn, reason);
break;
+ default:
+ hci_acl_disconn(conn, reason);
+ break;
}
}
--
1.8.1.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox