* [PATCH] android/avrcp: trivial header include cleanup
From: Andrei Emeltchenko @ 2014-02-25 13:51 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
android/avrcp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/avrcp.c b/android/avrcp.c
index acffd56..48444a4 100644
--- a/android/avrcp.c
+++ b/android/avrcp.c
@@ -34,10 +34,10 @@
#include "lib/sdp_lib.h"
#include "src/log.h"
#include "bluetooth.h"
-#include "avrcp.h"
-#include "avrcp-lib.h"
#include "hal-msg.h"
#include "ipc.h"
+#include "avrcp-lib.h"
+#include "avrcp.h"
#define L2CAP_PSM_AVCTP 0x17
--
1.8.3.2
^ permalink raw reply related
* [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test
From: Andrei Emeltchenko @ 2014-02-25 13:56 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Test verifies that the get capabilities command issued from the
Controller. For that we add the command to avrcp-lib and it will be used
in AVRCP later.
---
android/avrcp-lib.c | 40 ++++++++++++++++++++++++++++++++++++++++
android/avrcp-lib.h | 2 ++
android/avrcp.c | 1 +
unit/test-avrcp.c | 17 +++++++++++++++++
4 files changed, 60 insertions(+)
diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index c280cf8..136801e 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -50,6 +50,12 @@
#define AVRCP_STATUS_NO_AVAILABLE_PLAYERS 0x15
#define AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED 0x16
+/* Packet types */
+#define AVRCP_PACKET_TYPE_SINGLE 0x00
+#define AVRCP_PACKET_TYPE_START 0x01
+#define AVRCP_PACKET_TYPE_CONTINUING 0x02
+#define AVRCP_PACKET_TYPE_END 0x03
+
#if __BYTE_ORDER == __LITTLE_ENDIAN
struct avrcp_header {
@@ -202,6 +208,18 @@ static bool handle_passthrough_pdu(struct avctp *conn, uint8_t op,
return handler->func(session);
}
+/*
+ * set_company_id:
+ *
+ * Set three-byte Company_ID into outgoing AVRCP message
+ */
+static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
+{
+ cid[0] = cid_in >> 16;
+ cid[1] = cid_in >> 8;
+ cid[2] = cid_in;
+}
+
struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
{
struct avrcp *session;
@@ -253,3 +271,25 @@ int avrcp_init_uinput(struct avrcp *session, const char *name,
{
return avctp_init_uinput(session->conn, name, address);
}
+
+#define AVRCP_GET_CAPABILITIES_PARAM_LENGTH 1
+void avrcp_get_capabilities(struct avrcp *session, avctp_rsp_cb func)
+{
+ uint8_t buf[AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH];
+ struct avrcp_header *pdu = (void *) buf;
+ uint8_t length;
+
+ memset(buf, 0, sizeof(buf));
+
+ set_company_id(pdu->company_id, IEEEID_BTSIG);
+ pdu->pdu_id = AVRCP_GET_CAPABILITIES;
+ pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
+ pdu->params[0] = CAP_EVENTS_SUPPORTED;
+ pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH);
+
+ length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
+
+ avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
+ AVC_SUBUNIT_PANEL, buf, length,
+ func, session);
+}
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 2337429..4f3a632 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -74,3 +74,5 @@ void avrcp_set_passthrough_handlers(struct avrcp *session,
void *user_data);
int avrcp_init_uinput(struct avrcp *session, const char *name,
const char *address);
+
+void avrcp_get_capabilities(struct avrcp *session, avctp_rsp_cb func);
diff --git a/android/avrcp.c b/android/avrcp.c
index 48444a4..3d39d91 100644
--- a/android/avrcp.c
+++ b/android/avrcp.c
@@ -36,6 +36,7 @@
#include "bluetooth.h"
#include "hal-msg.h"
#include "ipc.h"
+#include "avctp.h"
#include "avrcp-lib.h"
#include "avrcp.h"
diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 53e9237..4a92860 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -297,6 +297,16 @@ static void test_server(gconstpointer data)
execute_context(context);
}
+static void test_client(gconstpointer data)
+{
+ struct context *context = create_context(0x0100, data);
+
+ if (g_str_equal(context->data->test_name, "/TP/CFG/BV-01-C"))
+ avrcp_get_capabilities(context->session, NULL);
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -364,5 +374,12 @@ int main(int argc, char *argv[])
raw_pdu(0x02, 0x11, 0x0e, 0x09, 0x48, 0x7c,
AVC_PLAY | 0x80, 0x00));
+ /* Metadata transfer tests */
+
+ define_test("/TP/CFG/BV-01-C", test_client,
+ raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+ 0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
+ 0x01, 0x03));
+
return g_test_run();
}
--
1.8.3.2
^ permalink raw reply related
* [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib
From: Andrei Emeltchenko @ 2014-02-25 13:56 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1393336605-2467-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
android/avrcp-lib.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
android/avrcp-lib.h | 12 +++++++++++
2 files changed, 72 insertions(+)
diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 136801e..95e10f2 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -56,6 +56,15 @@
#define AVRCP_PACKET_TYPE_CONTINUING 0x02
#define AVRCP_PACKET_TYPE_END 0x03
+/* Capabilities for AVRCP_GET_CAPABILITIES pdu */
+#define CAP_COMPANY_ID 0x02
+#define CAP_EVENTS_SUPPORTED 0x03
+
+/* Company IDs supported by this device */
+static uint32_t company_ids[] = {
+ IEEEID_BTSIG,
+};
+
#if __BYTE_ORDER == __LITTLE_ENDIAN
struct avrcp_header {
@@ -108,6 +117,8 @@ struct avrcp {
const struct avrcp_passthrough_handler *passthrough_handlers;
void *passthrough_data;
unsigned int passthrough_id;
+
+ uint16_t supported_events;
};
void avrcp_shutdown(struct avrcp *session)
@@ -220,6 +231,53 @@ static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
cid[2] = cid_in;
}
+static uint8_t avrcp_handle_get_capabilities(struct avrcp *session,
+ uint8_t transaction, uint16_t *params_len,
+ uint8_t *params, void *user_data)
+{
+ unsigned int i;
+
+ DBG("id %d params_len %d", params[0], *params_len);
+
+ if (*params_len != 1)
+ goto fail;
+
+ switch (params[0]) {
+ case CAP_COMPANY_ID:
+ for (i = 0; i < G_N_ELEMENTS(company_ids); i++)
+ set_company_id(¶ms[2 + i * 3], company_ids[i]);
+
+ *params_len = 2 + (3 * G_N_ELEMENTS(company_ids));
+ params[1] = G_N_ELEMENTS(company_ids);
+
+ return AVC_CTYPE_STABLE;
+ case CAP_EVENTS_SUPPORTED:
+ params[1] = 0;
+ for (i = 1; i <= AVRCP_EVENT_LAST; i++) {
+ if (session->supported_events & (1 << i)) {
+ params[1]++;
+ params[params[1] + 1] = i;
+ }
+ }
+
+ *params_len = 2 + params[1];
+
+ return AVC_CTYPE_STABLE;
+ }
+
+fail:
+ *params_len = htons(1);
+ params[0] = AVRCP_STATUS_INVALID_PARAM;
+
+ return AVC_CTYPE_REJECTED;
+}
+
+static const struct avrcp_control_handler control_handlers[] = {
+ { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
+ avrcp_handle_get_capabilities },
+ { },
+};
+
struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
{
struct avrcp *session;
@@ -241,6 +299,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
handle_vendordep_pdu,
session);
+ avrcp_set_control_handlers(session, control_handlers, NULL);
+
return session;
}
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 4f3a632..0821287 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -46,6 +46,18 @@
#define AVRCP_ADD_TO_NOW_PLAYING 0x90
#define AVRCP_GENERAL_REJECT 0xA0
+/* Notification events */
+#define AVRCP_EVENT_STATUS_CHANGED 0x01
+#define AVRCP_EVENT_TRACK_CHANGED 0x02
+#define AVRCP_EVENT_TRACK_REACHED_END 0x03
+#define AVRCP_EVENT_TRACK_REACHED_START 0x04
+#define AVRCP_EVENT_SETTINGS_CHANGED 0x08
+#define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED 0x0a
+#define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED 0x0b
+#define AVRCP_EVENT_UIDS_CHANGED 0x0c
+#define AVRCP_EVENT_VOLUME_CHANGED 0x0d
+#define AVRCP_EVENT_LAST AVRCP_EVENT_VOLUME_CHANGED
+
struct avrcp;
struct avrcp_control_handler {
--
1.8.3.2
^ permalink raw reply related
* [PATCH 3/3] unit/avrcp: Add /TP/CFG/BV-02-C test
From: Andrei Emeltchenko @ 2014-02-25 13:56 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1393336605-2467-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Test verifies that the Target responds to Get Capability request.
---
unit/test-avrcp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 4a92860..71a4d0e 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -381,5 +381,13 @@ int main(int argc, char *argv[])
0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
0x01, 0x03));
+ define_test("/TP/CFG/BV-02-C", test_server,
+ raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+ 0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
+ 0x01, 0x02),
+ raw_pdu(0x02, 0x11, 0x0e, 0x0c, 0x48, 0x00,
+ 0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
+ 0x01, 0x02, 0x01, 0x00, 0x19, 0x58));
+
return g_test_run();
}
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] android/avrcp: Fix passing wrong len
From: Luiz Augusto von Dentz @ 2014-02-25 14:09 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1393336058-431-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On Tue, Feb 25, 2014 at 3:47 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> When handling vendor dependent pdus len was passed in wrong order to
> callback.
> ---
> android/avrcp-lib.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> index c78881f..c280cf8 100644
> --- a/android/avrcp-lib.c
> +++ b/android/avrcp-lib.c
> @@ -128,14 +128,14 @@ static size_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
> const struct avrcp_control_handler *handler;
> struct avrcp_header *pdu = (void *) operands;
> uint32_t company_id = ntoh24(pdu->company_id);
> + uint16_t params_len = ntohs(pdu->params_len);
>
> if (company_id != IEEEID_BTSIG) {
> *code = AVC_CTYPE_NOT_IMPLEMENTED;
> return 0;
> }
>
> - DBG("AVRCP PDU 0x%02X, len 0x%04X", pdu->pdu_id,
> - ntohs(pdu->params_len));
> + DBG("AVRCP PDU 0x%02X, len 0x%04X", pdu->pdu_id, params_len);
>
> pdu->packet_type = 0;
> pdu->rsvd = 0;
> @@ -163,10 +163,10 @@ static size_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
> goto reject;
> }
>
> - *code = handler->func(session, transaction, &pdu->params_len,
> + *code = handler->func(session, transaction, ¶ms_len,
> pdu->params, session->control_data);
>
> - return AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
> + return AVRCP_HEADER_LENGTH + params_len;
>
> reject:
> pdu->params_len = htons(1);
> --
> 1.8.3.2
This will not work, because the response is done using the same buffer
if you don't pass the pdu->param_len the callback cannot overwrite it
with the response length, we might however change the response to be
async since the Android HAL does require that anyway.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib
From: Luiz Augusto von Dentz @ 2014-02-25 14:19 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1393336605-2467-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On Tue, Feb 25, 2014 at 3:56 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ---
> android/avrcp-lib.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> android/avrcp-lib.h | 12 +++++++++++
> 2 files changed, 72 insertions(+)
>
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> index 136801e..95e10f2 100644
> --- a/android/avrcp-lib.c
> +++ b/android/avrcp-lib.c
> @@ -56,6 +56,15 @@
> #define AVRCP_PACKET_TYPE_CONTINUING 0x02
> #define AVRCP_PACKET_TYPE_END 0x03
>
> +/* Capabilities for AVRCP_GET_CAPABILITIES pdu */
> +#define CAP_COMPANY_ID 0x02
> +#define CAP_EVENTS_SUPPORTED 0x03
> +
> +/* Company IDs supported by this device */
> +static uint32_t company_ids[] = {
> + IEEEID_BTSIG,
> +};
> +
> #if __BYTE_ORDER == __LITTLE_ENDIAN
>
> struct avrcp_header {
> @@ -108,6 +117,8 @@ struct avrcp {
> const struct avrcp_passthrough_handler *passthrough_handlers;
> void *passthrough_data;
> unsigned int passthrough_id;
> +
> + uint16_t supported_events;
> };
>
> void avrcp_shutdown(struct avrcp *session)
> @@ -220,6 +231,53 @@ static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
> cid[2] = cid_in;
> }
>
> +static uint8_t avrcp_handle_get_capabilities(struct avrcp *session,
> + uint8_t transaction, uint16_t *params_len,
> + uint8_t *params, void *user_data)
> +{
> + unsigned int i;
> +
> + DBG("id %d params_len %d", params[0], *params_len);
> +
> + if (*params_len != 1)
> + goto fail;
> +
> + switch (params[0]) {
> + case CAP_COMPANY_ID:
> + for (i = 0; i < G_N_ELEMENTS(company_ids); i++)
> + set_company_id(¶ms[2 + i * 3], company_ids[i]);
> +
> + *params_len = 2 + (3 * G_N_ELEMENTS(company_ids));
> + params[1] = G_N_ELEMENTS(company_ids);
> +
> + return AVC_CTYPE_STABLE;
> + case CAP_EVENTS_SUPPORTED:
> + params[1] = 0;
> + for (i = 1; i <= AVRCP_EVENT_LAST; i++) {
> + if (session->supported_events & (1 << i)) {
> + params[1]++;
> + params[params[1] + 1] = i;
> + }
> + }
> +
> + *params_len = 2 + params[1];
> +
> + return AVC_CTYPE_STABLE;
> + }
> +
> +fail:
> + *params_len = htons(1);
> + params[0] = AVRCP_STATUS_INVALID_PARAM;
> +
> + return AVC_CTYPE_REJECTED;
> +}
> +
> +static const struct avrcp_control_handler control_handlers[] = {
> + { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
> + avrcp_handle_get_capabilities },
> + { },
> +};
> +
> struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> {
> struct avrcp *session;
> @@ -241,6 +299,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> handle_vendordep_pdu,
> session);
>
> + avrcp_set_control_handlers(session, control_handlers, NULL);
> +
> return session;
> }
>
> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> index 4f3a632..0821287 100644
> --- a/android/avrcp-lib.h
> +++ b/android/avrcp-lib.h
> @@ -46,6 +46,18 @@
> #define AVRCP_ADD_TO_NOW_PLAYING 0x90
> #define AVRCP_GENERAL_REJECT 0xA0
>
> +/* Notification events */
> +#define AVRCP_EVENT_STATUS_CHANGED 0x01
> +#define AVRCP_EVENT_TRACK_CHANGED 0x02
> +#define AVRCP_EVENT_TRACK_REACHED_END 0x03
> +#define AVRCP_EVENT_TRACK_REACHED_START 0x04
> +#define AVRCP_EVENT_SETTINGS_CHANGED 0x08
> +#define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED 0x0a
> +#define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED 0x0b
> +#define AVRCP_EVENT_UIDS_CHANGED 0x0c
> +#define AVRCP_EVENT_VOLUME_CHANGED 0x0d
> +#define AVRCP_EVENT_LAST AVRCP_EVENT_VOLUME_CHANGED
> +
> struct avrcp;
>
> struct avrcp_control_handler {
> --
> 1.8.3.2
That is the actual AVRCP implementation not the library, the library
only offer means to handle the commands but don't parse it there since
we can't do anything with it. For unit tests you can implement dummy
handlers as we did for passthrough.
^ permalink raw reply
* [v2] android/pics: Add PICS and PIXIT for HFP
From: Sebastian Chlad @ 2014-02-25 14:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sebastian Chlad
---
android/Makefile.am | 2 +
android/pics-hfp.txt | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++
android/pixit-hfp.txt | 37 +++++++++
3 files changed, 256 insertions(+)
create mode 100644 android/pics-hfp.txt
create mode 100644 android/pixit-hfp.txt
diff --git a/android/Makefile.am b/android/Makefile.am
index 58cfbae..b944ca9 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -189,6 +189,7 @@ EXTRA_DIST += android/Android.mk android/README \
android/pics-avctp.txt \
android/pics-avrcp.txt \
android/pics-hsp.txt \
+ android/pics-hfp.txt \
android/pixit-l2cap.txt \
android/pixit-gap.txt \
android/pixit-did.txt \
@@ -201,6 +202,7 @@ EXTRA_DIST += android/Android.mk android/README \
android/pixit-avctp.txt \
android/pixit-avrcp.txt \
android/pixit-hsp.txt \
+ android/pixit-hfp.txt \
android/pts-l2cap.txt \
android/pts-gap.txt \
android/pts-did.txt \
diff --git a/android/pics-hfp.txt b/android/pics-hfp.txt
new file mode 100644
index 0000000..ae6d33e
--- /dev/null
+++ b/android/pics-hfp.txt
@@ -0,0 +1,217 @@
+HFP PICS for the PTS tool.
+
+PTS version: 5.0
+
+* - different than PTS defaults
+# - not yet implemented/supported
+
+M - mandatory
+O - optional
+
+ Version
+-------------------------------------------------------------------------------
+Parameter Name Selected Description
+-------------------------------------------------------------------------------
+TSPC_HFP_0_1 False Version: Hands-Free Profile v1.5 (O.1)
+TSPC_HFP_0_2 True (*) Version: Hands-Free Profile v1.6 (O.1)
+-------------------------------------------------------------------------------
+O.1: It is mandatory to support only one of the adopted versions.
+-------------------------------------------------------------------------------
+
+
+ Roles
+-------------------------------------------------------------------------------
+Parameter Name Selected Description
+-------------------------------------------------------------------------------
+TSPC_HFP_1_1 True (*) Role: Audio Gateway (AG) (O.1)
+TSPC_HFP_1_2 False Role: Hands-Free (HF) (O.1)
+-------------------------------------------------------------------------------
+O.1: It is mandatory to support at least one of the defined roles.
+-------------------------------------------------------------------------------
+
+
+ Audio Gateway Role
+-------------------------------------------------------------------------------
+Parameter Name Selected Description
+-------------------------------------------------------------------------------
+TSPC_HFP_2_1 True Connection management (M)
+TSPC_HFP_2_1a True (*) SLC initiation during active ongoing call (O)
+TSPC_HFP_2_2 True Phone Status Information (M)
+TSPC_HFP_2_3 True Audio connection handling (M)
+TSPC_HFP_2_3a False Audio connection establishment independent of
+ call processing (O)
+TSPC_HFP_2_3b True (*) eSCO support in Audio Connection (C.10)
+TSPC_HFP_2_3c False Codec negotiation (C.7)
+TSPC_HFP_2_4a False Accept an incoming voice call
+ (in-band ring) (C.1)
+TSPC_HFP_2_4b True (*) Accept an incoming voice call
+ (no in-band ring) (C.1)
+TSPC_HFP_2_4c False Capability to change the "in-band ring"
+ settings (O)
+TSPC_HFP_2_5 True (*) Reject an incoming voice call (O)
+TSPC_HFP_2_6 True Terminate a call (M)
+TSPC_HFP_2_7 True Audio connection transfer during an ongoing
+ call (M)
+TSPC_HFP_2_7a True (*) HF-initiated Audio transfer to AG during
+ ongoing call (O)
+TSPC_HFP_2_8 True Place a call with a phone number supplied by
+ the HF (M)
+TSPC_HFP_2_9 True Place a call using memory dialing (M)
+TSPC_HFP_2_10 True Place a call to the last number dialed (M)
+TSPC_HFP_2_11 True Call waiting notification (M)
+TSPC_HFP_2_12 True (*) Three Way Calling (O)
+TSPC_HFP_2_12a True (*) User Busy (AT+CHLD value 0) (C.3)
+TSPC_HFP_2_12b True (*) Call Hold Handling (AT+CHLD value 1,2) (C.2)
+TSPC_HFP_2_12c True (*) Three Way Call (AT+CHLD value 3) (C.3)
+TSPC_HFP_2_12d False Explicit Call Transfer (AT+CHLD value 4) (C.3)
+TSPC_HFP_2_13 True Calling Line Identification (CLI) (M)
+TSPC_HFP_2_14 True (*) Echo canceling (EC) and Noise reduction (NR) (O)
+TSPC_HFP_2_15 True (*) Voice recognition activation (O)
+TSPC_HFP_2_15a False Initiate voice recognition from AG (C.6)
+TSPC_HFP_2_15b False Autonomous voice deactivation (C.6)
+TSPC_HFP_2_16 False Attach a phone number to a voice tag (O)
+TSPC_HFP_2_17 True Ability to transmit DTMF codes (M)
+TSPC_HFP_2_18a True (*) Remote audio volume control – speaker (O)
+TSPC_HFP_2_18b False Remote audio volume control – microphone (O)
+TSPC_HFP_2_18c True (*) Volume Level Synchronization – speaker and
+ microphone (C.5)
+TSPC_HFP_2_19 False Response and hold (O)
+TSPC_HFP_2_20 True Subscriber Number Information (M)
+TSPC_HFP_2_21a True Enhanced Call Status (C.4)
+TSPC_HFP_2_21b False Enhanced Call Control (C.3)
+TSPC_HFP_2_21c True (*) Enhanced Call Status with limited network
+ notification (C.4)
+TSPC_HFP_2_22 False Support for automatic link loss recovery (O)
+TSPC_HFP_2_23 True (*) Individual Indicator Activation (C.9)
+TSPC_HFP_2_24 False Wide Band Speech service (C.8)
+TSPC_HFP_2_25 False Support roaming function (O)
+-------------------------------------------------------------------------------
+C.1: The AG must support one of item TSPC_HFP_2_4a or TSPC_HFP_2_4b
+C.2: Mandatory if TSPC_HFP_2_12is TRUE; otherwise excluded
+C.3: Optional if TSPC_HFP_2_12 is TRUE; otherwise excluded
+C.4: The AG must support one of item TSPC_HFP_2_21a or TSPC_HFP_2_21c
+C.5: Mandatory if TSPC_HFP_2_18a or TSPC_HFP_2_18b; otherwise optional
+C.6: Optional if TSPC_HFP_2_15 is supported, otherwise excluded
+C.7: Mandatory if TSPC_HFP_2_24 otherwise excluded
+C.8: Excluded if TSPC_HFP_0_1 otherwise optional
+C.9: Excluded if TSPC_HFP_0_1 otherwise mandatory
+C.10: Mandatory if TSPC_HFP_2_24 otherwise optional
+-------------------------------------------------------------------------------
+
+
+ Hands-Free Role
+-------------------------------------------------------------------------------
+Parameter Name Selected Description
+-------------------------------------------------------------------------------
+TSPC_HFP_3_1 False (*) Connection Management (M)
+TSPC_HFP_3_2a False (*) Phone Status Information ("service" and "call"
+ indicators) (M)
+TSPC_HFP_3_2b False Phone Status Information ("callsetup"
+ indicators) (O)
+TSPC_HFP_3_2c False Accept indicator of signal strength (O)
+TSPC_HFP_3_2d False Accept indicator of roaming state ("roam:") (O)
+TSPC_HFP_3_2e False Accept indicator of battery level ("battchg") (O)
+TSPC_HFP_3_2f False Accept indicator of operator selection (O)
+TSPC_HFP_3_3 False (*) Audio connection handling (M)
+TSPC_HFP_3_3a False Audio connection establishment independent
+ of call processing (O)
+TSPC_HFP_3_3b False eSCO support in Audio Connection (C.7)
+TSPC_HFP_3_3c False Codec negotiation (C.5)
+TSPC_HFP_3_4a False (*) Accept an incoming voice call (in-band ring) (M)
+TSPC_HFP_3_4b False (*) Accept an incoming voice call (no in-band
+ ring) (M)
+TSPC_HFP_3_4c False Accept an incoming voice call (in-band ring
+ muting) (O)
+TSPC_HFP_3_5 False (*) Reject an incoming voice call (M)
+TSPC_HFP_3_6 False (*) Terminate a call (M)
+
+TSPC_HFP_3_7 False (*) Audio connection transfer during an ongoing
+ call (M)
+TSPC_HFP_3_7a False HF-initiated Audio transfer to AG during
+ ongoing call (O)
+TSPC_HFP_3_8 False Place a call with a phone number supplied by
+ the HF (O)
+TSPC_HFP_3_9 False Place a call using memory dialing (O)
+TSPC_HFP_3_10 False Place a call to the last number dialed (O)
+TSPC_HFP_3_11 False Call waiting notification (O)
+TSPC_HFP_3_12 False Three Way Calling (O)
+TSPC_HFP_3_12a False Three way calling (AT+CHLD values 0) (C.2)
+TSPC_HFP_3_12b False Three way calling (AT+CHLD values 1 and 2) (C.1)
+TSPC_HFP_3_12c False Three way calling (AT+CHLD value 3) (C.2)
+TSPC_HFP_3_12d False Three way calling (AT+CHLD value 4) (C.2)
+TSPC_HFP_3_12e False Originate new call with established call in
+ progress (C.2)
+TSPC_HFP_3_13 False Calling Line Identification (CLI) (O)
+TSPC_HFP_3_14 False Echo cancelling (EC) and Noise reduction (NR) (O)
+TSPC_HFP_3_15 False Voice recognition activation/deactivation (O)
+TSPC_HFP_3_16 False Attach a phone number to a voice tag (O)
+TSPC_HFP_3_17 False Ability to transmit DTMF codes (O)
+TSPC_HFP_3_18a False Remote audio volume control – speaker (O)
+TSPC_HFP_3_18b False Remote audio volume control – microphone (O)
+TSPC_HFP_3_18c False Volume Level Synchronization – speaker (C.3)
+TSPC_HFP_3_18d False Volume Level Synchronization – microphone (C.4)
+TSPC_HFP_3_18e False HF informs AG about local changes of audio
+ volume (O)
+TSPC_HFP_3_18f False HF informs AG about local changes of
+ microphone gain (O)
+TSPC_HFP_3_19 False Response and hold (O)
+TSPC_HFP_3_20 False Subscriber Number Information (O)
+TSPC_HFP_3_21a False Enhanced Call Status (O)
+TSPC_HFP_3_21b False Enhanced Call Control (C.2)
+TSPC_HFP_3_22 False Support for automatic link loss recovery (O)
+TSPC_HFP_3_23 False Individual Indicator Activation (C.6)
+TSPC_HFP_3_24 False Wide Band Speech service (C.6)
+-------------------------------------------------------------------------------
+C.1: Mandatory if TSPC_HFP_3_12; otherwise excluded
+C.2: Optional if TSPC_HFP_3_12; otherwise excluded
+C.3: Mandatory if TSPC_HFP_3_18a or TSPC_HFP_3_18b, otherwise optional
+C.4: Mandatory if TSPC_HFP_3_18a, otherwise optional
+C.5: Mandatory if TSPC_HFP_3_24 otherwise excluded
+C.6: Excluded if TSPC_HFP_0_1 otherwise optional
+C.7: Mandatory if TSPC_HFP_3_24 otherwise optional
+-------------------------------------------------------------------------------
+
+
+ Audio Coding Requirements
+-------------------------------------------------------------------------------
+Parameter Name Selected Description
+-------------------------------------------------------------------------------
+TSPC_HFP_4_1 True CVSD audio coding over SCO (M)
+TSPC_HFP_4_2 False mSBC audio coding over eSCO (C.1)
+-------------------------------------------------------------------------------
+C.1: Mandatory if Wide band speech service is supported TSPC_HFP_2_24 or
+ TSPC_HFP_3_24, otherwise excluded
+-------------------------------------------------------------------------------
+
+
+ Supplementary Interoperability Verification
+-------------------------------------------------------------------------------
+Parameter Name Selected Description
+-------------------------------------------------------------------------------
+TSPC_HFP_8_1 False Multiple audio transfers during call –
+ AG and HF initiated (C.1)
+TSPC_HFP_8_2 False Audio transfer by SLC release during
+ an active call (C.1)
+TSPC_HFP_8_3 False Audio transfer by powering ON HF (O)
+TSPC_HFP_8_4 False SLC during SDP response (O)
+TSPC_HFP_8_5 False Handle dynamic server channel number for HFP
+ service (O)
+TSPC_HFP_8_6 False HF disallows connections in non-discoverable
+ mode (C.2)
+TSPC_HFP_8_7 False HF connects to AG during incoming call (O)
+TSPC_HFP_8_8 False Link loss during incoming call (C.3)
+TSPC_HFP_8_9 False SLC release during incoming call (C.3)
+TSPC_HFP_8_10 False Voice Recognition Activation (C.4)
+TSPC_HFP_8_11 False Place outgoing call by dialing number on
+ the AG (O)
+TSPC_HFP_8_12 False Active call termination – NO CARRIER signal
+ (C.5)
+-------------------------------------------------------------------------------
+C.1: Optional if TSPC_HFP_2_7a or TSPC_HFP_3_7a is supported,
+ otherwise excluded
+C.2: Optional if TSPC_HFP_1_2 is supported, otherwise excluded
+C.3: Optional if TSPC_HFP_1_1 is supported, otherwise excluded
+C.4: Optional if TSPC_HFP_2_15 or TSPC_HFP_3_15 is supported,
+ otherwise excluded
+C.5: Optional if TSPC_HFP_2_6 is supported, otherwise excluded
+-------------------------------------------------------------------------------
diff --git a/android/pixit-hfp.txt b/android/pixit-hfp.txt
new file mode 100644
index 0000000..c1b2503
--- /dev/null
+++ b/android/pixit-hfp.txt
@@ -0,0 +1,37 @@
+HFP PIXIT for the PTS tool.
+
+PTS version: 5.0
+
+* - different than PTS defaults
+& - should be set to IUT Bluetooth address
+# - should be set to the respective phone numbers
+
+ Required PIXIT settings
+-------------------------------------------------------------------------------
+Parameter Name Value
+-------------------------------------------------------------------------------
+TSPX_security_enabled TRUE
+TSPX_bd_addr_iut 012345678901 (*&)
+TSPX_hf_class_of_device 200408
+TSPX_ag_class_of_device 400204
+TSPX_packet_type_sco 00A0
+TSPX_phone_number 1234567 (#)
+TSPX_second_phone_number 7654321 (#)
+TSPX_phone_number_type 129
+TSPX_second_phone_number_type 129
+TSPX_phone_number_memory 1
+TSPX_phone_number_memory_invalid_index 9999
+TSPX_scan_all_memory_dial_locations FALSE
+TSPX_pin_code 0000
+TSPX_time_guard 300000
+TSPX_use_implicit_send TRUE
+TSPX_verbose_implicit_send FALSE
+TSPX_delete_link_key FALSE
+TSPX_server_channel_tester 01
+TSPX_server_channel_iut 00
+TSPX_verify_CLIP_information TRUE
+TSPX_no_fail_verdict FALSE
+TSPX_network_supports_correct_call_and_callstatus TRUE
+TSPX_secure_simple_pairing_pass_key_confirmation FALSE
+TSPX_AG_match_tester_BRSF_codec_negotiation_bit FALSE
+-------------------------------------------------------------------------------
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Fix NULL pointer dereference when sending data
From: Johan Hedberg @ 2014-02-25 14:52 UTC (permalink / raw)
To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1393323371-13232-1-git-send-email-andrzej.kaczmarek@tieto.com>
Hi Andrzej,
On Tue, Feb 25, 2014, Andrzej Kaczmarek wrote:
> When trying to allocate skb for new PDU, l2cap_chan is unlocked so we
> can sleep waiting for memory as otherwise there's possible deadlock as
> fixed in e454c84464. However, in a6a5568c03 lock was moved from socket
> to channel level and it's no longer safe to just unlock and lock again
> without checking l2cap_chan state since channel can be disconnected
> when lock is not held.
>
> This patch adds missing checks for l2cap_chan state when returning from
> call which allocates skb.
>
> Scenario is easily reproducible by running rfcomm-tester in a loop.
>
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffffa0442169>] l2cap_do_send+0x29/0x120 [bluetooth]
> PGD 0
> Oops: 0000 [#1] SMP
> Modules linked in:
> CPU: 7 PID: 4038 Comm: krfcommd Not tainted 3.14.0-rc2+ #15
> Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A10 11/24/2011
> task: ffff8802bdd731c0 ti: ffff8801ec986000 task.ti: ffff8801ec986000
> RIP: 0010:[<ffffffffa0442169>] [<ffffffffa0442169>] l2cap_do_send+0x29/0x120
> RSP: 0018:ffff8801ec987ad8 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: ffff8800c5796800 RCX: 0000000000000000
> RDX: ffff880410e7a800 RSI: ffff8802b6c1da00 RDI: ffff8800c5796800
> RBP: ffff8801ec987af8 R08: 00000000000000c0 R09: 0000000000000300
> R10: 000000000000573b R11: 000000000000573a R12: ffff8802b6c1da00
> R13: 0000000000000000 R14: ffff8802b6c1da00 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff88042dce0000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 000000041257c000 CR4: 00000000000407e0
> Stack:
> ffff8801ec987d78 ffff8800c5796800 ffff8801ec987d78 0000000000000000
> ffff8801ec987ba8 ffffffffa0449e37 0000000000000004 ffff8801ec987af0
> ffff8801ec987d40 0000000000000282 0000000000000000 ffffffff00000004
> Call Trace:
> [<ffffffffa0449e37>] l2cap_chan_send+0xaa7/0x1120 [bluetooth]
> [<ffffffff81770100>] ? _raw_spin_unlock_bh+0x20/0x40
> [<ffffffffa045188b>] l2cap_sock_sendmsg+0xcb/0x110 [bluetooth]
> [<ffffffff81652b0f>] sock_sendmsg+0xaf/0xc0
> [<ffffffff810a8381>] ? update_curr+0x141/0x200
> [<ffffffff810a8961>] ? dequeue_entity+0x181/0x520
> [<ffffffff81652b60>] kernel_sendmsg+0x40/0x60
> [<ffffffffa04a8505>] rfcomm_send_frame+0x45/0x70 [rfcomm]
> [<ffffffff810766f0>] ? internal_add_timer+0x20/0x50
> [<ffffffffa04a8564>] rfcomm_send_cmd+0x34/0x60 [rfcomm]
> [<ffffffffa04a8605>] rfcomm_send_disc+0x75/0xa0 [rfcomm]
> [<ffffffffa04aacec>] rfcomm_run+0x8cc/0x1a30 [rfcomm]
> [<ffffffffa04aa420>] ? rfcomm_check_accept+0xc0/0xc0 [rfcomm]
> [<ffffffff8108e3a9>] kthread+0xc9/0xe0
> [<ffffffff8108e2e0>] ? flush_kthread_worker+0xb0/0xb0
> [<ffffffff817795fc>] ret_from_fork+0x7c/0xb0
> [<ffffffff8108e2e0>] ? flush_kthread_worker+0xb0/0xb0
> Code: 00 00 66 66 66 66 90 55 48 89 e5 48 83 ec 20 f6 05 d6 a3 02 00 04
> RIP [<ffffffffa0442169>] l2cap_do_send+0x29/0x120 [bluetooth]
> RSP <ffff8801ec987ad8>
> CR2: 0000000000000000
>
> Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
> ---
> net/bluetooth/l2cap_core.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 6ace116..cfc7c69 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2434,6 +2434,11 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> if (IS_ERR(skb))
> return PTR_ERR(skb);
>
> + if (chan->state != BT_CONNECTED) {
> + kfree_skb(skb);
> + return -ENOTCONN;
> + }
> +
> l2cap_do_send(chan, skb);
> return len;
> }
> @@ -2483,6 +2488,11 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> if (IS_ERR(skb))
> return PTR_ERR(skb);
>
> + if (chan->state != BT_CONNECTED) {
> + kfree_skb(skb);
> + return -ENOTCONN;
> + }
> +
> l2cap_do_send(chan, skb);
> err = len;
> break;
Acked-by: Johan Hedberg <johan.hedberg@intel.com>
However, I'd consider adding code comment explaining the lock was
released and reacquired and the state needs to therefore be rechecked.
Johan
^ permalink raw reply
* [PATCH] android/hal-ipc-api: Fix Add Set Volume command struct packing
From: Grzegorz Kolodziejczyk @ 2014-02-25 15:50 UTC (permalink / raw)
To: linux-bluetooth
This adds missed packed struct attribute to hal-ipc command.
---
android/hal-msg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 7c6110a..ee08c37 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -987,7 +987,7 @@ struct hal_cmd_avrcp_register_notification {
#define HAL_OP_AVRCP_SET_VOLUME 0x0a
struct hal_cmd_avrcp_set_volume {
uint8_t value;
-};
+} __attribute__((packed));
#define HAL_EV_AVRCP_REMOTE_FEATURES 0x81
struct hal_ev_avrcp_remote_features {
--
1.8.5.2
^ permalink raw reply related
* [PATCH v2] Bluetooth: Fix NULL pointer dereference when sending data
From: Andrzej Kaczmarek @ 2014-02-25 16:16 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
When trying to allocate skb for new PDU, l2cap_chan is unlocked so we
can sleep waiting for memory as otherwise there's possible deadlock as
fixed in e454c84464. However, in a6a5568c03 lock was moved from socket
to channel level and it's no longer safe to just unlock and lock again
without checking l2cap_chan state since channel can be disconnected
when lock is not held.
This patch adds missing checks for l2cap_chan state when returning from
call which allocates skb.
Scenario is easily reproducible by running rfcomm-tester in a loop.
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffffa0442169>] l2cap_do_send+0x29/0x120 [bluetooth]
PGD 0
Oops: 0000 [#1] SMP
Modules linked in:
CPU: 7 PID: 4038 Comm: krfcommd Not tainted 3.14.0-rc2+ #15
Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A10 11/24/2011
task: ffff8802bdd731c0 ti: ffff8801ec986000 task.ti: ffff8801ec986000
RIP: 0010:[<ffffffffa0442169>] [<ffffffffa0442169>] l2cap_do_send+0x29/0x120
RSP: 0018:ffff8801ec987ad8 EFLAGS: 00010202
RAX: 0000000000000000 RBX: ffff8800c5796800 RCX: 0000000000000000
RDX: ffff880410e7a800 RSI: ffff8802b6c1da00 RDI: ffff8800c5796800
RBP: ffff8801ec987af8 R08: 00000000000000c0 R09: 0000000000000300
R10: 000000000000573b R11: 000000000000573a R12: ffff8802b6c1da00
R13: 0000000000000000 R14: ffff8802b6c1da00 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff88042dce0000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000041257c000 CR4: 00000000000407e0
Stack:
ffff8801ec987d78 ffff8800c5796800 ffff8801ec987d78 0000000000000000
ffff8801ec987ba8 ffffffffa0449e37 0000000000000004 ffff8801ec987af0
ffff8801ec987d40 0000000000000282 0000000000000000 ffffffff00000004
Call Trace:
[<ffffffffa0449e37>] l2cap_chan_send+0xaa7/0x1120 [bluetooth]
[<ffffffff81770100>] ? _raw_spin_unlock_bh+0x20/0x40
[<ffffffffa045188b>] l2cap_sock_sendmsg+0xcb/0x110 [bluetooth]
[<ffffffff81652b0f>] sock_sendmsg+0xaf/0xc0
[<ffffffff810a8381>] ? update_curr+0x141/0x200
[<ffffffff810a8961>] ? dequeue_entity+0x181/0x520
[<ffffffff81652b60>] kernel_sendmsg+0x40/0x60
[<ffffffffa04a8505>] rfcomm_send_frame+0x45/0x70 [rfcomm]
[<ffffffff810766f0>] ? internal_add_timer+0x20/0x50
[<ffffffffa04a8564>] rfcomm_send_cmd+0x34/0x60 [rfcomm]
[<ffffffffa04a8605>] rfcomm_send_disc+0x75/0xa0 [rfcomm]
[<ffffffffa04aacec>] rfcomm_run+0x8cc/0x1a30 [rfcomm]
[<ffffffffa04aa420>] ? rfcomm_check_accept+0xc0/0xc0 [rfcomm]
[<ffffffff8108e3a9>] kthread+0xc9/0xe0
[<ffffffff8108e2e0>] ? flush_kthread_worker+0xb0/0xb0
[<ffffffff817795fc>] ret_from_fork+0x7c/0xb0
[<ffffffff8108e2e0>] ? flush_kthread_worker+0xb0/0xb0
Code: 00 00 66 66 66 66 90 55 48 89 e5 48 83 ec 20 f6 05 d6 a3 02 00 04
RIP [<ffffffffa0442169>] l2cap_do_send+0x29/0x120 [bluetooth]
RSP <ffff8801ec987ad8>
CR2: 0000000000000000
Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
Acked-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2 added comment regarding why we need to check state again
net/bluetooth/l2cap_core.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 6ace116..7bd78c5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2434,6 +2434,14 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
if (IS_ERR(skb))
return PTR_ERR(skb);
+ /* Channel lock is released before requesting new skb and then
+ * reacquired thus we need to recheck channel state.
+ */
+ if (chan->state != BT_CONNECTED) {
+ kfree_skb(skb);
+ return -ENOTCONN;
+ }
+
l2cap_do_send(chan, skb);
return len;
}
@@ -2483,6 +2491,14 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
if (IS_ERR(skb))
return PTR_ERR(skb);
+ /* Channel lock is released before requesting new skb and then
+ * reacquired thus we need to recheck channel state.
+ */
+ if (chan->state != BT_CONNECTED) {
+ kfree_skb(skb);
+ return -ENOTCONN;
+ }
+
l2cap_do_send(chan, skb);
err = len;
break;
--
1.8.5.4
^ permalink raw reply related
* Re: [RFC v11 01/15] Bluetooth: Create hci_req_add_le_scan_disable helper
From: Andre Guedes @ 2014-02-25 17:16 UTC (permalink / raw)
To: Andre Guedes, linux-bluetooth@vger.kernel.org
In-Reply-To: <20140225055717.GA776@localhost.P-661HNU-F1>
Hi Johan,
On Tue, Feb 25, 2014 at 2:57 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Andre,
>
> On Mon, Feb 24, 2014, Andre Guedes wrote:
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -1128,6 +1128,7 @@ void hci_req_add(struct hci_request *req, u16 opcode, u32 plen,
>> void hci_req_add_ev(struct hci_request *req, u16 opcode, u32 plen,
>> const void *param, u8 event);
>> void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
>> +void hci_req_add_le_scan_disable(struct hci_request *req);
>
> It doesn't feel right to put this straight next to the generic async
> request API. I'd at least have an empty line between the generic API and
> this new function.
I'll add an empty line then.
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -3318,7 +3318,6 @@ static void le_scan_disable_work(struct work_struct *work)
>> {
>> struct hci_dev *hdev = container_of(work, struct hci_dev,
>> le_scan_disable.work);
>> - struct hci_cp_le_set_scan_enable cp;
>> struct hci_request req;
>> int err;
>>
>> @@ -3326,9 +3325,7 @@ static void le_scan_disable_work(struct work_struct *work)
>>
>> hci_req_init(&req, hdev);
>>
>> - memset(&cp, 0, sizeof(cp));
>> - cp.enable = LE_SCAN_DISABLE;
>> - hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
>> + hci_req_add_le_scan_disable(&req);
>>
>> err = hci_req_run(&req, le_scan_disable_work_complete);
>> if (err)
>> @@ -4872,3 +4869,12 @@ static void hci_cmd_work(struct work_struct *work)
>> }
>> }
>> }
>> +
>> +void hci_req_add_le_scan_disable(struct hci_request *req)
>> +{
>> + struct hci_cp_le_set_scan_enable cp;
>> +
>> + memset(&cp, 0, sizeof(cp));
>> + cp.enable = LE_SCAN_DISABLE;
>> + hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
>> +}
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 25b8b27..c722914 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -3526,7 +3526,6 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
>> struct hci_cp_remote_name_req_cancel cp;
>> struct inquiry_entry *e;
>> struct hci_request req;
>> - struct hci_cp_le_set_scan_enable enable_cp;
>> int err;
>>
>> BT_DBG("%s", hdev->name);
>> @@ -3562,10 +3561,7 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
>> } else {
>> cancel_delayed_work(&hdev->le_scan_disable);
>>
>> - memset(&enable_cp, 0, sizeof(enable_cp));
>> - enable_cp.enable = LE_SCAN_DISABLE;
>> - hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE,
>> - sizeof(enable_cp), &enable_cp);
>> + hci_req_add_le_scan_disable(&req);
>> }
>>
>> break;
>
> You've missed one place in clean_up_hci_state() in mgmt.c (which was
> added by the recent "graceful power down" patch set.
I'll fix it.
Thanks,
Andre
^ permalink raw reply
* Re: [RFC v11 06/15] Bluetooth: Introduce LE auto connection infrastructure
From: Andre Guedes @ 2014-02-25 17:16 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org
In-Reply-To: <20140225042337.GA27803@localhost.P-661HNU-F1>
Hi Johan,
On Tue, Feb 25, 2014 at 1:23 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Andre,
>
> On Mon, Feb 24, 2014, Andre Guedes wrote:
>> + if (hci_update_random_address(&req, false, &own_addr_type))
>> + return;
>
> Don't we want require_privacy = true here?
require_privacy = true enables non-resolvable address. Since this is a
passive scanning (no SCAN_REQ is sent), I don't think this is
required.
>> + if (addr_type == ADDR_LE_DEV_PUBLIC)
>> + bdaddr_type = BDADDR_LE_PUBLIC;
>> + else
>> + bdaddr_type = BDADDR_LE_RANDOM;
>> +
>> + conn = hci_connect(hdev, LE_LINK, addr, bdaddr_type, BT_SECURITY_LOW,
>> + HCI_AT_NO_BONDING);
>
> All hci_connect is is a very short wrapper to hci_connect_acl/le. The
> hci_connect_le function in turn does the following:
>
> /* Convert from L2CAP channel address type to HCI address type */
> if (dst_type == BDADDR_LE_PUBLIC)
> dst_type = ADDR_LE_DEV_PUBLIC;
> else
> dst_type = ADDR_LE_DEV_RANDOM;
>
> So it seems a bit silly to do this kind of back and forth conversions.
> Instead, I think the hci_connect API should take the address type that's
> used for struct hci_conn, i.e. the HCI one (not the L2CAP/mgmt one).
>
> Also, since pretty much all places that are calling hci_connect pass an
> explicit value as the link type I think the most sensible thing to do
> would be to completely remove this function and export hci_connect_le &
> hci_connect_acl from hci_conn.c.
I agree. I can do this refactoring in the next version.
BR,
Andre
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix advertising address type when toggling connectable
From: Marcel Holtmann @ 2014-02-25 17:26 UTC (permalink / raw)
To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393333842-8380-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> When the connectable setting is toggled using mgmt_set_connectable the
> HCI_CONNECTABLE flag will only be set once the related HCI commands
> succeed. When determining what kind of advertising to do we need to
> therefore also check whether there is a pending Set Connectable command
> in addition to the current flag value.
>
> The enable_advertising function was already taking care of this for the
> advertising type with the help of the get_adv_type function, but was
> failing to do the same for the address type selection. This patch
> converts the get_adv_type function to be more generic in that it returns
> the expected connectable state and updates the enable_advertising
> function to use the return value both for the advertising type as well
> as the advertising address type.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 25b8b278debd..a02828ad4c0e 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -817,10 +817,9 @@ static void update_class(struct hci_request *req)
> hci_req_add(req, HCI_OP_WRITE_CLASS_OF_DEV, sizeof(cod), cod);
> }
>
> -static u8 get_adv_type(struct hci_dev *hdev)
> +static bool get_connectable(struct hci_dev *hdev)
> {
> struct pending_cmd *cmd;
> - bool connectable;
>
> /* If there's a pending mgmt command the flag will not yet have
> * it's final value, so check for this first.
> @@ -828,12 +827,10 @@ static u8 get_adv_type(struct hci_dev *hdev)
> cmd = mgmt_pending_find(MGMT_OP_SET_CONNECTABLE, hdev);
> if (cmd) {
> struct mgmt_mode *cp = cmd->param;
> - connectable = !!cp->val;
> - } else {
> - connectable = test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
> + return cp->val;
> }
>
> - return connectable ? LE_ADV_IND : LE_ADV_NONCONN_IND;
> + return test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
> }
>
> static void enable_advertising(struct hci_request *req)
> @@ -841,9 +838,10 @@ static void enable_advertising(struct hci_request *req)
> struct hci_dev *hdev = req->hdev;
> struct hci_cp_le_set_adv_param cp;
> u8 own_addr_type, enable = 0x01;
> - bool require_privacy;
> + bool connectable, require_privacy;
>
> - require_privacy = !test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
> + connectable = get_connectable(hdev);
> + require_privacy = !connectable;
>
> if (hci_update_random_address(req, require_privacy, &own_addr_type) < 0)
> return;
can we use a comment here to explain require_privacy like we do with the other time it is used. the two variables are too much. And then call it like this ..address(req, !connectable, &own..).
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix advertising address type when toggling connectable
From: Johan Hedberg @ 2014-02-25 17:32 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <84898D84-C149-4ABE-8CFE-55FB2E261CF6@holtmann.org>
Hi Marcel,
On Tue, Feb 25, 2014, Marcel Holtmann wrote:
> > - bool require_privacy;
> > + bool connectable, require_privacy;
> >
> > - require_privacy = !test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
> > + connectable = get_connectable(hdev);
> > + require_privacy = !connectable;
> >
> > if (hci_update_random_address(req, require_privacy, &own_addr_type) < 0)
> > return;
>
> can we use a comment here to explain require_privacy like we do with
> the other time it is used. the two variables are too much. And then
> call it like this ..address(req, !connectable, &own..).
Sure. v2 coming in a few minutes.
Johan
^ permalink raw reply
* [PATCH v2] Bluetooth: Fix advertising address type when toggling connectable
From: johan.hedberg @ 2014-02-25 17:36 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@intel.com>
When the connectable setting is toggled using mgmt_set_connectable the
HCI_CONNECTABLE flag will only be set once the related HCI commands
succeed. When determining what kind of advertising to do we need to
therefore also check whether there is a pending Set Connectable command
in addition to the current flag value.
The enable_advertising function was already taking care of this for the
advertising type with the help of the get_adv_type function, but was
failing to do the same for the address type selection. This patch
converts the get_adv_type function to be more generic in that it returns
the expected connectable state and updates the enable_advertising
function to use the return value both for the advertising type as well
as the advertising address type.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 25b8b278debd..16782b5860c5 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -817,10 +817,9 @@ static void update_class(struct hci_request *req)
hci_req_add(req, HCI_OP_WRITE_CLASS_OF_DEV, sizeof(cod), cod);
}
-static u8 get_adv_type(struct hci_dev *hdev)
+static bool get_connectable(struct hci_dev *hdev)
{
struct pending_cmd *cmd;
- bool connectable;
/* If there's a pending mgmt command the flag will not yet have
* it's final value, so check for this first.
@@ -828,12 +827,10 @@ static u8 get_adv_type(struct hci_dev *hdev)
cmd = mgmt_pending_find(MGMT_OP_SET_CONNECTABLE, hdev);
if (cmd) {
struct mgmt_mode *cp = cmd->param;
- connectable = !!cp->val;
- } else {
- connectable = test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
+ return cp->val;
}
- return connectable ? LE_ADV_IND : LE_ADV_NONCONN_IND;
+ return test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
}
static void enable_advertising(struct hci_request *req)
@@ -841,17 +838,21 @@ static void enable_advertising(struct hci_request *req)
struct hci_dev *hdev = req->hdev;
struct hci_cp_le_set_adv_param cp;
u8 own_addr_type, enable = 0x01;
- bool require_privacy;
+ bool connectable;
- require_privacy = !test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
+ connectable = get_connectable(hdev);
- if (hci_update_random_address(req, require_privacy, &own_addr_type) < 0)
+ /* If we're connectable set require_privacy to false as we
+ * shouldn't use NRPAs in that case. If we're non-connectable
+ * however we can allow NRPAs to be used.
+ */
+ if (hci_update_random_address(req, !connectable, &own_addr_type) < 0)
return;
memset(&cp, 0, sizeof(cp));
cp.min_interval = __constant_cpu_to_le16(0x0800);
cp.max_interval = __constant_cpu_to_le16(0x0800);
- cp.type = get_adv_type(hdev);
+ cp.type = connectable ? LE_ADV_IND : LE_ADV_NONCONN_IND;
cp.own_address_type = own_addr_type;
cp.channel_map = hdev->le_adv_channel_map;
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Fix NULL pointer dereference when sending data
From: Marcel Holtmann @ 2014-02-25 17:41 UTC (permalink / raw)
To: Johan Hedberg
Cc: Andrzej Kaczmarek,
bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <20140225145226.GA13119@localhost.P-661HNU-F1>
Hi Johan,
>> When trying to allocate skb for new PDU, l2cap_chan is unlocked so we
>> can sleep waiting for memory as otherwise there's possible deadlock as
>> fixed in e454c84464. However, in a6a5568c03 lock was moved from socket
>> to channel level and it's no longer safe to just unlock and lock again
>> without checking l2cap_chan state since channel can be disconnected
>> when lock is not held.
>>
>> This patch adds missing checks for l2cap_chan state when returning from
>> call which allocates skb.
>>
>> Scenario is easily reproducible by running rfcomm-tester in a loop.
>>
>> BUG: unable to handle kernel NULL pointer dereference at (null)
>> IP: [<ffffffffa0442169>] l2cap_do_send+0x29/0x120 [bluetooth]
>> PGD 0
>> Oops: 0000 [#1] SMP
>> Modules linked in:
>> CPU: 7 PID: 4038 Comm: krfcommd Not tainted 3.14.0-rc2+ #15
>> Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A10 11/24/2011
>> task: ffff8802bdd731c0 ti: ffff8801ec986000 task.ti: ffff8801ec986000
>> RIP: 0010:[<ffffffffa0442169>] [<ffffffffa0442169>] l2cap_do_send+0x29/0x120
>> RSP: 0018:ffff8801ec987ad8 EFLAGS: 00010202
>> RAX: 0000000000000000 RBX: ffff8800c5796800 RCX: 0000000000000000
>> RDX: ffff880410e7a800 RSI: ffff8802b6c1da00 RDI: ffff8800c5796800
>> RBP: ffff8801ec987af8 R08: 00000000000000c0 R09: 0000000000000300
>> R10: 000000000000573b R11: 000000000000573a R12: ffff8802b6c1da00
>> R13: 0000000000000000 R14: ffff8802b6c1da00 R15: 0000000000000000
>> FS: 0000000000000000(0000) GS:ffff88042dce0000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 0000000000000000 CR3: 000000041257c000 CR4: 00000000000407e0
>> Stack:
>> ffff8801ec987d78 ffff8800c5796800 ffff8801ec987d78 0000000000000000
>> ffff8801ec987ba8 ffffffffa0449e37 0000000000000004 ffff8801ec987af0
>> ffff8801ec987d40 0000000000000282 0000000000000000 ffffffff00000004
>> Call Trace:
>> [<ffffffffa0449e37>] l2cap_chan_send+0xaa7/0x1120 [bluetooth]
>> [<ffffffff81770100>] ? _raw_spin_unlock_bh+0x20/0x40
>> [<ffffffffa045188b>] l2cap_sock_sendmsg+0xcb/0x110 [bluetooth]
>> [<ffffffff81652b0f>] sock_sendmsg+0xaf/0xc0
>> [<ffffffff810a8381>] ? update_curr+0x141/0x200
>> [<ffffffff810a8961>] ? dequeue_entity+0x181/0x520
>> [<ffffffff81652b60>] kernel_sendmsg+0x40/0x60
>> [<ffffffffa04a8505>] rfcomm_send_frame+0x45/0x70 [rfcomm]
>> [<ffffffff810766f0>] ? internal_add_timer+0x20/0x50
>> [<ffffffffa04a8564>] rfcomm_send_cmd+0x34/0x60 [rfcomm]
>> [<ffffffffa04a8605>] rfcomm_send_disc+0x75/0xa0 [rfcomm]
>> [<ffffffffa04aacec>] rfcomm_run+0x8cc/0x1a30 [rfcomm]
>> [<ffffffffa04aa420>] ? rfcomm_check_accept+0xc0/0xc0 [rfcomm]
>> [<ffffffff8108e3a9>] kthread+0xc9/0xe0
>> [<ffffffff8108e2e0>] ? flush_kthread_worker+0xb0/0xb0
>> [<ffffffff817795fc>] ret_from_fork+0x7c/0xb0
>> [<ffffffff8108e2e0>] ? flush_kthread_worker+0xb0/0xb0
>> Code: 00 00 66 66 66 66 90 55 48 89 e5 48 83 ec 20 f6 05 d6 a3 02 00 04
>> RIP [<ffffffffa0442169>] l2cap_do_send+0x29/0x120 [bluetooth]
>> RSP <ffff8801ec987ad8>
>> CR2: 0000000000000000
>>
>> Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
>> ---
>> net/bluetooth/l2cap_core.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 6ace116..cfc7c69 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -2434,6 +2434,11 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>> if (IS_ERR(skb))
>> return PTR_ERR(skb);
>>
>> + if (chan->state != BT_CONNECTED) {
>> + kfree_skb(skb);
>> + return -ENOTCONN;
>> + }
>> +
>> l2cap_do_send(chan, skb);
>> return len;
>> }
>> @@ -2483,6 +2488,11 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>> if (IS_ERR(skb))
>> return PTR_ERR(skb);
>>
>> + if (chan->state != BT_CONNECTED) {
>> + kfree_skb(skb);
>> + return -ENOTCONN;
>> + }
>> +
>> l2cap_do_send(chan, skb);
>> err = len;
>> break;
>
> Acked-by: Johan Hedberg <johan.hedberg@intel.com>
>
> However, I'd consider adding code comment explaining the lock was
> released and reacquired and the state needs to therefore be rechecked.
I agree. This needs comments in both cases. Especially since we are doing the same for the other check when using ERTM.
Regards
Marcel
^ permalink raw reply
* Re: [RFC v11 06/15] Bluetooth: Introduce LE auto connection infrastructure
From: Marcel Holtmann @ 2014-02-25 17:43 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CACJA=fW-A8GbT55_W43Xk50bu5nzwkPjxDB7fuMBKf0h=mmD7Q@mail.gmail.com>
Hi Andre,
>>> + if (hci_update_random_address(&req, false, &own_addr_type))
>>> + return;
>>
>> Don't we want require_privacy = true here?
>
> require_privacy = true enables non-resolvable address. Since this is a
> passive scanning (no SCAN_REQ is sent), I don't think this is
> required.
that is correct, it is not strictly speaking needed. However I would prefer we set this to true anyway and add a proper comment for it like we do with the other places.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2] Bluetooth: Fix advertising address type when toggling connectable
From: Marcel Holtmann @ 2014-02-25 17:48 UTC (permalink / raw)
To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393349802-5831-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> When the connectable setting is toggled using mgmt_set_connectable the
> HCI_CONNECTABLE flag will only be set once the related HCI commands
> succeed. When determining what kind of advertising to do we need to
> therefore also check whether there is a pending Set Connectable command
> in addition to the current flag value.
>
> The enable_advertising function was already taking care of this for the
> advertising type with the help of the get_adv_type function, but was
> failing to do the same for the address type selection. This patch
> converts the get_adv_type function to be more generic in that it returns
> the expected connectable state and updates the enable_advertising
> function to use the return value both for the advertising type as well
> as the advertising address type.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 25b8b278debd..16782b5860c5 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -817,10 +817,9 @@ static void update_class(struct hci_request *req)
> hci_req_add(req, HCI_OP_WRITE_CLASS_OF_DEV, sizeof(cod), cod);
> }
>
> -static u8 get_adv_type(struct hci_dev *hdev)
> +static bool get_connectable(struct hci_dev *hdev)
> {
> struct pending_cmd *cmd;
> - bool connectable;
>
> /* If there's a pending mgmt command the flag will not yet have
> * it's final value, so check for this first.
> @@ -828,12 +827,10 @@ static u8 get_adv_type(struct hci_dev *hdev)
> cmd = mgmt_pending_find(MGMT_OP_SET_CONNECTABLE, hdev);
> if (cmd) {
> struct mgmt_mode *cp = cmd->param;
> - connectable = !!cp->val;
> - } else {
> - connectable = test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
> + return cp->val;
> }
>
> - return connectable ? LE_ADV_IND : LE_ADV_NONCONN_IND;
> + return test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
> }
>
> static void enable_advertising(struct hci_request *req)
> @@ -841,17 +838,21 @@ static void enable_advertising(struct hci_request *req)
> struct hci_dev *hdev = req->hdev;
> struct hci_cp_le_set_adv_param cp;
> u8 own_addr_type, enable = 0x01;
> - bool require_privacy;
> + bool connectable;
>
> - require_privacy = !test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
> + connectable = get_connectable(hdev);
>
> - if (hci_update_random_address(req, require_privacy, &own_addr_type) < 0)
> + /* If we're connectable set require_privacy to false as we
> + * shouldn't use NRPAs in that case. If we're non-connectable
> + * however we can allow NRPAs to be used.
> + */
> + if (hci_update_random_address(req, !connectable, &own_addr_type) < 0)
> return;
I really do not want to be pedantic, but this comment is misleading. It is a bit hard to understand. Also I am not sure if it NRPA or URPA. I think using an acronym here is confusing.
/* Set require_privacy to true only when non-connectable advertising
* is used. In that case it is fine to use an unresolvable private
* address.
*/
Regards
Marcel
^ permalink raw reply
* [PATCH v3] Bluetooth: Fix advertising address type when toggling connectable
From: johan.hedberg @ 2014-02-25 17:56 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@intel.com>
When the connectable setting is toggled using mgmt_set_connectable the
HCI_CONNECTABLE flag will only be set once the related HCI commands
succeed. When determining what kind of advertising to do we need to
therefore also check whether there is a pending Set Connectable command
in addition to the current flag value.
The enable_advertising function was already taking care of this for the
advertising type with the help of the get_adv_type function, but was
failing to do the same for the address type selection. This patch
converts the get_adv_type function to be more generic in that it returns
the expected connectable state and updates the enable_advertising
function to use the return value both for the advertising type as well
as the advertising address type.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 25b8b278debd..d6e269287cfc 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -817,10 +817,9 @@ static void update_class(struct hci_request *req)
hci_req_add(req, HCI_OP_WRITE_CLASS_OF_DEV, sizeof(cod), cod);
}
-static u8 get_adv_type(struct hci_dev *hdev)
+static bool get_connectable(struct hci_dev *hdev)
{
struct pending_cmd *cmd;
- bool connectable;
/* If there's a pending mgmt command the flag will not yet have
* it's final value, so check for this first.
@@ -828,12 +827,10 @@ static u8 get_adv_type(struct hci_dev *hdev)
cmd = mgmt_pending_find(MGMT_OP_SET_CONNECTABLE, hdev);
if (cmd) {
struct mgmt_mode *cp = cmd->param;
- connectable = !!cp->val;
- } else {
- connectable = test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
+ return cp->val;
}
- return connectable ? LE_ADV_IND : LE_ADV_NONCONN_IND;
+ return test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
}
static void enable_advertising(struct hci_request *req)
@@ -841,17 +838,21 @@ static void enable_advertising(struct hci_request *req)
struct hci_dev *hdev = req->hdev;
struct hci_cp_le_set_adv_param cp;
u8 own_addr_type, enable = 0x01;
- bool require_privacy;
+ bool connectable;
- require_privacy = !test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
+ connectable = get_connectable(hdev);
- if (hci_update_random_address(req, require_privacy, &own_addr_type) < 0)
+ /* Set require_privacy to true only when non-connectable
+ * advertising is used. In that case it is fine to use a
+ * non-resolvable private address.
+ */
+ if (hci_update_random_address(req, !connectable, &own_addr_type) < 0)
return;
memset(&cp, 0, sizeof(cp));
cp.min_interval = __constant_cpu_to_le16(0x0800);
cp.max_interval = __constant_cpu_to_le16(0x0800);
- cp.type = get_adv_type(hdev);
+ cp.type = connectable ? LE_ADV_IND : LE_ADV_NONCONN_IND;
cp.own_address_type = own_addr_type;
cp.channel_map = hdev->le_adv_channel_map;
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH v2] Bluetooth: Fix advertising address type when toggling connectable
From: Johan Hedberg @ 2014-02-25 17:56 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1700F292-DB01-4DA1-AF5D-445535741ACA@holtmann.org>
Hi Marcel,
On Tue, Feb 25, 2014, Marcel Holtmann wrote:
> > - if (hci_update_random_address(req, require_privacy, &own_addr_type) < 0)
> > + /* If we're connectable set require_privacy to false as we
> > + * shouldn't use NRPAs in that case. If we're non-connectable
> > + * however we can allow NRPAs to be used.
> > + */
> > + if (hci_update_random_address(req, !connectable, &own_addr_type) < 0)
> > return;
>
> I really do not want to be pedantic, but this comment is misleading.
> It is a bit hard to understand. Also I am not sure if it NRPA or URPA.
It's "non-resolvable" according to the core specification.
> I think using an acronym here is confusing.
>
> /* Set require_privacy to true only when non-connectable advertising
> * is used. In that case it is fine to use an unresolvable private
> * address.
> */
v3 has been sent.
Johan
^ permalink raw reply
* Re: [PATCH v2] Bluetooth: Fix NULL pointer dereference when sending data
From: Marcel Holtmann @ 2014-02-25 18:04 UTC (permalink / raw)
To: Andrzej Kaczmarek; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393344982-6756-1-git-send-email-andrzej.kaczmarek@tieto.com>
Hi Andrzej,
> When trying to allocate skb for new PDU, l2cap_chan is unlocked so we
> can sleep waiting for memory as otherwise there's possible deadlock as
> fixed in e454c84464. However, in a6a5568c03 lock was moved from socket
> to channel level and it's no longer safe to just unlock and lock again
> without checking l2cap_chan state since channel can be disconnected
> when lock is not held.
>
> This patch adds missing checks for l2cap_chan state when returning from
> call which allocates skb.
>
> Scenario is easily reproducible by running rfcomm-tester in a loop.
>
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffffa0442169>] l2cap_do_send+0x29/0x120 [bluetooth]
> PGD 0
> Oops: 0000 [#1] SMP
> Modules linked in:
> CPU: 7 PID: 4038 Comm: krfcommd Not tainted 3.14.0-rc2+ #15
> Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A10 11/24/2011
> task: ffff8802bdd731c0 ti: ffff8801ec986000 task.ti: ffff8801ec986000
> RIP: 0010:[<ffffffffa0442169>] [<ffffffffa0442169>] l2cap_do_send+0x29/0x120
> RSP: 0018:ffff8801ec987ad8 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: ffff8800c5796800 RCX: 0000000000000000
> RDX: ffff880410e7a800 RSI: ffff8802b6c1da00 RDI: ffff8800c5796800
> RBP: ffff8801ec987af8 R08: 00000000000000c0 R09: 0000000000000300
> R10: 000000000000573b R11: 000000000000573a R12: ffff8802b6c1da00
> R13: 0000000000000000 R14: ffff8802b6c1da00 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff88042dce0000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 000000041257c000 CR4: 00000000000407e0
> Stack:
> ffff8801ec987d78 ffff8800c5796800 ffff8801ec987d78 0000000000000000
> ffff8801ec987ba8 ffffffffa0449e37 0000000000000004 ffff8801ec987af0
> ffff8801ec987d40 0000000000000282 0000000000000000 ffffffff00000004
> Call Trace:
> [<ffffffffa0449e37>] l2cap_chan_send+0xaa7/0x1120 [bluetooth]
> [<ffffffff81770100>] ? _raw_spin_unlock_bh+0x20/0x40
> [<ffffffffa045188b>] l2cap_sock_sendmsg+0xcb/0x110 [bluetooth]
> [<ffffffff81652b0f>] sock_sendmsg+0xaf/0xc0
> [<ffffffff810a8381>] ? update_curr+0x141/0x200
> [<ffffffff810a8961>] ? dequeue_entity+0x181/0x520
> [<ffffffff81652b60>] kernel_sendmsg+0x40/0x60
> [<ffffffffa04a8505>] rfcomm_send_frame+0x45/0x70 [rfcomm]
> [<ffffffff810766f0>] ? internal_add_timer+0x20/0x50
> [<ffffffffa04a8564>] rfcomm_send_cmd+0x34/0x60 [rfcomm]
> [<ffffffffa04a8605>] rfcomm_send_disc+0x75/0xa0 [rfcomm]
> [<ffffffffa04aacec>] rfcomm_run+0x8cc/0x1a30 [rfcomm]
> [<ffffffffa04aa420>] ? rfcomm_check_accept+0xc0/0xc0 [rfcomm]
> [<ffffffff8108e3a9>] kthread+0xc9/0xe0
> [<ffffffff8108e2e0>] ? flush_kthread_worker+0xb0/0xb0
> [<ffffffff817795fc>] ret_from_fork+0x7c/0xb0
> [<ffffffff8108e2e0>] ? flush_kthread_worker+0xb0/0xb0
> Code: 00 00 66 66 66 66 90 55 48 89 e5 48 83 ec 20 f6 05 d6 a3 02 00 04
> RIP [<ffffffffa0442169>] l2cap_do_send+0x29/0x120 [bluetooth]
> RSP <ffff8801ec987ad8>
> CR2: 0000000000000000
>
> Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
> Acked-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> v2 added comment regarding why we need to check state again
>
> net/bluetooth/l2cap_core.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v3] Bluetooth: Fix advertising address type when toggling connectable
From: Marcel Holtmann @ 2014-02-25 18:04 UTC (permalink / raw)
To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393350991-8327-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> When the connectable setting is toggled using mgmt_set_connectable the
> HCI_CONNECTABLE flag will only be set once the related HCI commands
> succeed. When determining what kind of advertising to do we need to
> therefore also check whether there is a pending Set Connectable command
> in addition to the current flag value.
>
> The enable_advertising function was already taking care of this for the
> advertising type with the help of the get_adv_type function, but was
> failing to do the same for the address type selection. This patch
> converts the get_adv_type function to be more generic in that it returns
> the expected connectable state and updates the enable_advertising
> function to use the return value both for the advertising type as well
> as the advertising address type.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/9] shared: Add skeleton of hfp_at parser
From: Marcel Holtmann @ 2014-02-25 18:46 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393320227-10931-2-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
> It will parse AT frames in HFP profile implementation. Use can register
> handlers for specified prefixes. Callbacks will be called with one of
> hfp_cmd_type. Default handler will have prefix NULL, and type passed
> to callback will be always HFP_AT_UNKNOWN.
> ---
> src/shared/hfp_at.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp_at.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 93 insertions(+)
> create mode 100644 src/shared/hfp_at.c
> create mode 100644 src/shared/hfp_at.h
>
> diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
> new file mode 100644
> index 0000000..bd1899b
> --- /dev/null
> +++ b/src/shared/hfp_at.c
> @@ -0,0 +1,50 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2014 Intel Corporation. All rights reserved.
> + *
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +#include <stdbool.h>
> +#include <string.h>
> +#include <ctype.h>
> +
> +#include "src/shared/util.h"
> +#include "src/shared/hfp_at.h"
> +
> +struct hfp_at {
> + const struct hfp_at_handler *cmd_handlers;
> +};
> +
> +struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
> +{
> + struct hfp_at *hfp_at;
> +
> + hfp_at = new0(struct hfp_at, 1);
> + if (!hfp_at)
> + return NULL;
> +
> + hfp_at->cmd_handlers = handlers;
> +
> + return hfp_at;
> +}
> +
> +void hfp_at_free(struct hfp_at *hfp_at)
> +{
> + free(hfp_at);
> +}
> diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
> new file mode 100644
> index 0000000..fe074f2
> --- /dev/null
> +++ b/src/shared/hfp_at.h
> @@ -0,0 +1,43 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2014 Intel Corporation. All rights reserved.
> + *
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +struct hfp_at;
> +
> +enum hfp_cmd_type {
> + HFP_AT_READ,
> + HFP_AT_SET,
> + HFP_AT_TEST,
> + HFP_AT_COMMAND,
> + HFP_AT_UNKNOWN
> +};
> +
> +typedef void (*hfp_at_cmd_cb)(struct hfp_at *hfp_at, enum hfp_cmd_type type,
> + const char *at, void *user_data);
> +
> +struct hfp_at_handler {
> + const char *prefix;
> + hfp_at_cmd_cb cb;
> +};
> +
> +struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
> +void hfp_at_free(struct hfp_at *hfp_at);
so I rather see this integrated into hfp_gw natively. No need for new files or a new structure with all new resource allocation.
Lets just use hfp_gw_set_command_handler for the default case where we have no specific handler to parse the data since that will be needed for unknown AT command callback anyway.
And then add hfp_gw_register_command(hfp, prefix, callback, user_data, destroy) with a matching unregister function. Use a large table does not really makes sense to me. Individual commands similar to GAtServer works seems more reasonable and lot more flexible.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/9] shared/hfp_at: Add skeleton of hfp_at_process_data
From: Marcel Holtmann @ 2014-02-25 18:56 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393320227-10931-4-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
> It will handle AT frames and recognize their commands. It will also
> call proper callback. If no callback was found, default handler
> will be called.
> ---
> src/shared/hfp_at.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp_at.h | 3 +
> 2 files changed, 178 insertions(+)
>
> diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
> index bd1899b..eff0a38 100644
> --- a/src/shared/hfp_at.c
> +++ b/src/shared/hfp_at.c
> @@ -29,8 +29,183 @@
>
> struct hfp_at {
> const struct hfp_at_handler *cmd_handlers;
> + int offset;
> };
>
> +/* Last cmd handler should have prefix NULL and will be returned if no other
> + * will be found
> + */
> +static const struct hfp_at_handler *hfp_at_find_cb(struct hfp_at *hfp_at,
> + char *pref, uint8_t len)
> +{
> + const struct hfp_at_handler *handler = hfp_at->cmd_handlers;
> +
> + while (handler->prefix) {
> + if (strlen(handler->prefix) == len)
> + if (!memcmp(handler->prefix, pref, len))
> + break;
> +
> + handler++;
> + }
> +
> + return handler;
> +}
> +
> +static void hfp_at_call_handler(struct hfp_at *hfp_at, char *prefix, int len,
> + const char *data, void *user_data,
> + enum hfp_cmd_type type)
> +{
> + const struct hfp_at_handler *handler;
> +
> + handler = hfp_at_find_cb(hfp_at, prefix, len);
> +
> + if (handler->prefix) {
> + handler->cb(hfp_at, type, data, user_data);
> + return;
> + }
> +
> + /* We set offset to 0 because it is unknown command */
> + hfp_at->offset = 0;
> + handler->cb(hfp_at, HFP_AT_UNKNOWN, data, user_data);
> +}
> +
> +static bool hfp_at_process_basic(struct hfp_at *hfp_at, const char *data,
> + void *user_data)
> +{
> + const char *prefix = data + hfp_at->offset;
> + enum hfp_cmd_type type;
> + char lookup_prefix[4];
> + uint8_t pref_len = 0;
> + bool equal = false;
> + int i;
> +
> + /* Check if first sign is character */
> + if (isalpha(prefix[pref_len])) {
> + /* Handle S-parameter prefix */
> + if (toupper(prefix[pref_len]) == 'S') {
> + do {
> + pref_len++;
> + } while (isdigit(prefix[pref_len]));
> + /*S-parameter must be followed with number*/
> + if (pref_len == 1)
> + pref_len--;
> + } else {
> + /* It's just one-character prefix */
> + pref_len++;
> + }
> + } else if (prefix[pref_len] == '&' && isalpha(prefix[pref_len + 1])) {
> + /* Two-signed prefix starting with & */
> + pref_len = 2;
> + }
I am not sure we actually need this. HFP is pretty much only a subset. I am fine with not bothering with any of the details of V250 if we do not have to. Let Java deal with it if they want to.
> +
> + if (pref_len == 0 || pref_len > sizeof(lookup_prefix))
> + return false;
> +
> + for (i = 0; i < pref_len; i++)
> + lookup_prefix[i] = toupper(prefix[i]);
> +
> + hfp_at->offset += pref_len;
> +
> + if (lookup_prefix[0] == 'D') {
> + type = HFP_AT_SET;
> +
> + goto done;
> + }
> +
> + if (data[hfp_at->offset] == '=') {
> + hfp_at->offset++;
> + equal = true;
> + }
> +
> + if (data[hfp_at->offset] == '?') {
> + hfp_at->offset++;
> +
> + if (equal)
> + type = HFP_AT_TEST;
> + else
> + type = HFP_AT_READ;
> + } else if (equal) {
> + type = HFP_AT_SET;
> + } else {
> + type = HFP_AT_COMMAND;
> + }
> +
> +done:
> + hfp_at_call_handler(hfp_at, lookup_prefix, pref_len, data, user_data,
> + type);
> +
> + return true;
> +}
> +
> +static bool hfp_at_process_extended(struct hfp_at *hfp_at, const char *data,
> + void *user_data)
> +{
> + const char *prefix = data + hfp_at->offset;
> + const char *separators = ";?=";
> + enum hfp_cmd_type type;
> + char lookup_prefix[18];
> + bool equal = false;
> + uint8_t pref_len;
> + int i;
> +
> + /* Lookup for first separator */
> + pref_len = strcspn(prefix, separators);
> +
> + if (pref_len > 17 || pref_len < 2)
> + return false;
> +
> + for (i = 0; i < pref_len; i++)
> + lookup_prefix[i] = toupper(prefix[i]);
> +
> + hfp_at->offset += pref_len;
> +
> + if (data[hfp_at->offset] == '=') {
> + hfp_at->offset++;
> + equal = true;
> + }
> +
> + if (data[hfp_at->offset] == '?') {
> + hfp_at->offset++;
> +
> + if (equal)
> + type = HFP_AT_TEST;
> + else
> + type = HFP_AT_READ;
> + } else if (equal) {
> + type = HFP_AT_SET;
> + } else {
> + type = HFP_AT_COMMAND;
> + }
Still trying to figure out why we need a variable equal here. Might want to restructure the code a little bit.
Also do not forget to check the length of your string. Remote devices can send you garbage.
I also fail to see what difference between basic and extended is. Seems like a lot of the basic code is shared.
> +
> + hfp_at_call_handler(hfp_at, lookup_prefix, pref_len, data, user_data,
> + type);
> +
> + return true;
> +}
> +
> +static bool hfp_at_is_extended_cmd(struct hfp_at *hfp_at, const char *data)
> +{
> + /* Extended commands names always begin with character "+" V250 5.4.1*/
> + return data[hfp_at->offset] == '+';
> +}
Lets not go overboard with functions that serve only a single purpose. Do that inline in the code.
> +
> +bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
> + void *user_data)
> +{
> + if (strlen(data) < 3)
> + return false;
> +
> + if (strncmp(data, "AT", 2))
> + return false;
actually they way I read is that AT and at are valid. However not At and not aT. We would also need to strip any leading whitespaces. This should be done on level down if I have not already implemented it.
> +
> + hfp_at->offset = 2;
> +
> + if (hfp_at_is_extended_cmd(hfp_at, data))
> + return hfp_at_process_extended(hfp_at, data, user_data);
> + else
> + return hfp_at_process_basic(hfp_at, data, user_data);
> +}
> +
> struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
> {
> struct hfp_at *hfp_at;
> diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
> index fe074f2..0b628b5 100644
> --- a/src/shared/hfp_at.h
> +++ b/src/shared/hfp_at.h
> @@ -39,5 +39,8 @@ struct hfp_at_handler {
> hfp_at_cmd_cb cb;
> };
>
> +bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
> + void *user_data);
> +
> struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
> void hfp_at_free(struct hfp_at *hfp_at);
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH] Bluetooth: Ignore IRKs with no Identity Address
From: johan.hedberg @ 2014-02-25 20:24 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@intel.com>
The Core Specification (4.1) leaves room for sending an SMP Identity
Address Information PDU with an all-zeros BD_ADDR value. This
essentially means that we would not have an Identity Address for the
device and the only means of identifying it would be the IRK value
itself.
Due to lack of any known implementations behaving like this it's best to
keep our implementation as simple as possible as far as handling such
situations is concerned. This patch updates the Identity Address
Information handler function to simply ignore the IRK received from such
a device.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/smp.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 79a80f44c832..50355d045992 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -1003,6 +1003,19 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
skb_pull(skb, sizeof(*info));
+ /* Strictly speaking the Core Specification (4.1) allows sending
+ * an empty address which would force us to rely on just the IRK
+ * as "identity information". However, since such
+ * implementations are not known of and in order to not over
+ * complicate our implementation, simply pretend that we never
+ * received an IRK for such a device.
+ */
+ if (!bacmp(&info->bdaddr, BDADDR_ANY)) {
+ BT_ERR("Ignoring IRK with no identity address");
+ smp_distribute_keys(conn, 1);
+ return 0;
+ }
+
bacpy(&smp->id_addr, &info->bdaddr);
smp->id_addr_type = info->addr_type;
--
1.8.5.3
^ 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