Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: Remove device OOB data if it was discovered in band
From: Szymon Janc @ 2012-11-19 14:46 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

OOB authentication mechanism should be used only if pairing process
has been activated by previous OOB information exchange (Core Spec
4.0 , vol. 1, Part A, 5.1.4.3). Stored OOB data for specific device
should be removed if that device was discovered in band later on.

Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
---

This could also be done by userspace but would require calling remove remote
OOB data mgmt command for every device found. Userspace could also track for
which devices OOB data were added but this could be problematic as OOB data
persists userspace restart. Doing it in kernel seems better.


 net/bluetooth/hci_event.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9f5c5f2..cda5bac 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1946,6 +1946,8 @@ static void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	for (; num_rsp; num_rsp--, info++) {
 		bool name_known, ssp;
 
+		hci_remove_remote_oob_data(hdev, &info->bdaddr);
+
 		bacpy(&data.bdaddr, &info->bdaddr);
 		data.pscan_rep_mode	= info->pscan_rep_mode;
 		data.pscan_period_mode	= info->pscan_period_mode;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/5] adapter: Remove not needed variable from btd_adapter_get_mode
From: Szymon Janc @ 2012-11-19 15:02 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

address is no longer used and can be removed.
---
 src/adapter.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 9ecc0fa..d208cfa 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2180,10 +2180,6 @@ void btd_adapter_get_mode(struct btd_adapter *adapter, uint8_t *mode,
 						uint16_t *discoverable_timeout,
 						gboolean *pairable)
 {
-	char address[18];
-
-	ba2str(&adapter->bdaddr, address);
-
 	if (mode)
 		*mode = adapter->mode;
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/5] adapter: Remove not needed variable from btd_adapter_start
From: Szymon Janc @ 2012-11-19 15:02 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1353337366-21590-1-git-send-email-szymon.janc@tieto.com>

address is no longer used and can be removed.
---
 src/adapter.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index d208cfa..8b599da 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2259,9 +2259,6 @@ void adapter_connect_list_remove(struct btd_adapter *adapter,
 void btd_adapter_start(struct btd_adapter *adapter)
 {
 	struct session_req *req;
-	char address[18];
-
-	ba2str(&adapter->bdaddr, address);
 
 	adapter->off_requested = FALSE;
 	adapter->up = TRUE;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 3/5] adapter: Minor whitespace fix in btd_adapter_set_class
From: Szymon Janc @ 2012-11-19 15:02 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1353337366-21590-1-git-send-email-szymon.janc@tieto.com>

Indent with tab not spaces.
---
 src/adapter.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/adapter.c b/src/adapter.c
index 8b599da..5c17a6e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -181,7 +181,7 @@ static gboolean process_auth_queue(gpointer user_data);
 int btd_adapter_set_class(struct btd_adapter *adapter, uint8_t major,
 							uint8_t minor)
 {
-       return mgmt_set_dev_class(adapter->dev_id, major, minor);
+	return mgmt_set_dev_class(adapter->dev_id, major, minor);
 }
 
 static const char *mode2str(uint8_t mode)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 4/5] device: Remove not really used auto_id member from btd_device
From: Szymon Janc @ 2012-11-19 15:02 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1353337366-21590-1-git-send-email-szymon.janc@tieto.com>

auto_id is not used anymore as LE device connect is now done by
adapter autoconnect list.
---
 src/device.c |    9 ---------
 1 file changed, 9 deletions(-)

diff --git a/src/device.c b/src/device.c
index bc78fa7..44e58ac 100644
--- a/src/device.c
+++ b/src/device.c
@@ -174,7 +174,6 @@ struct btd_device {
 	GSList		*attios;
 	GSList		*attios_offline;
 	guint		attachid;		/* Attrib server attach */
-	guint		auto_id;		/* Auto connect source id */
 
 	gboolean	connected;
 	gboolean	profiles_connected;	/* Profile level connected */
@@ -346,9 +345,6 @@ static void device_free(gpointer user_data)
 	if (device->discov_timer)
 		g_source_remove(device->discov_timer);
 
-	if (device->auto_id)
-		g_source_remove(device->auto_id);
-
 	DBG("%p", device);
 
 	if (device->authr)
@@ -4027,11 +4023,6 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 	if (device->attios != NULL || device->attios_offline != NULL)
 		return TRUE;
 
-	if (device->auto_id) {
-		g_source_remove(device->auto_id);
-		device->auto_id = 0;
-	}
-
 	attio_cleanup(device);
 
 	return TRUE;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 5/5] device: Fix minor whitespace issues
From: Szymon Janc @ 2012-11-19 15:02 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1353337366-21590-1-git-send-email-szymon.janc@tieto.com>

---
 src/device.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/device.c b/src/device.c
index 44e58ac..4b5363f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -191,7 +191,7 @@ struct btd_device {
 
 	gint		ref;
 
-	GIOChannel      *att_io;
+	GIOChannel	*att_io;
 	guint		cleanup_id;
 	guint		store_id;
 };
@@ -1506,7 +1506,6 @@ static const GDBusSignalTable device_signals[] = {
 	{ }
 };
 
-
 static const GDBusPropertyTable device_properties[] = {
 	{ "Address", "s", dev_property_get_address },
 	{ "Name", "s", dev_property_get_name, NULL, dev_property_exists_name },
@@ -4037,4 +4036,3 @@ void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
 	device_set_product(device, product_id);
 	device_set_version(device, product_ver);
 }
-
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/4] neard: Use ENONET error when adapter is not enabled
From: Szymon Janc @ 2012-11-19 15:03 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This results in returning error 'Disabled' instead of 'No such Device'.
Will allow neard to properly set power state of Bluetooth carrier.
---
 plugins/neard.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/neard.c b/plugins/neard.c
index 8f8381c..1576be5 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -380,7 +380,7 @@ static int check_adapter(struct btd_adapter *adapter)
 	btd_adapter_get_mode(adapter, NULL, NULL, &pairable);
 
 	if (!pairable || !adapter_get_agent(adapter))
-		return -ENOENT;
+		return -ENONET;
 
 	if (!btd_adapter_ssp_enabled(adapter))
 		return -ENOTSUP;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/4] neard: Move device object checking into separate helper
From: Szymon Janc @ 2012-11-19 15:03 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1353337438-21865-1-git-send-email-szymon.janc@tieto.com>

This will also be used by 'nokia.com:bt' handler.
---
 plugins/neard.c |   47 +++++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/plugins/neard.c b/plugins/neard.c
index 1576be5..8319445 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -248,13 +248,38 @@ static void bonding_complete(struct btd_adapter *adapter,
 		error("D-Bus send failed");
 }
 
+static int check_device(struct btd_adapter *adapter, const char *address)
+{
+	struct btd_device *device;
+
+	device = adapter_find_device(adapter, address);
+
+	/* If already paired */
+	if (device && device_is_paired(device)) {
+		DBG("already paired");
+		return 1;
+	}
+
+	/* Pairing in progress... */
+	if (device && device_is_bonding(device, NULL)) {
+		DBG("pairing in progress");
+		return -EINPROGRESS;
+	}
+
+	/* If we have unpaired device hanging around, purge it */
+	if (device)
+		adapter_remove_device(adapter, device, TRUE);
+
+	return 0;
+}
+
 /* returns 1 if pairing is not needed */
 static int process_eir(struct btd_adapter *adapter, uint8_t *eir, size_t size,
 							bdaddr_t *remote)
 {
-	struct btd_device *device;
 	struct eir_data eir_data;
 	char remote_address[18];
+	int ret;
 
 	DBG("size %zu", size);
 
@@ -267,26 +292,12 @@ static int process_eir(struct btd_adapter *adapter, uint8_t *eir, size_t size,
 
 	DBG("hci%u remote:%s", adapter_get_dev_id(adapter), remote_address);
 
-	device = adapter_find_device(adapter, remote_address);
-
-	/* If already paired do nothing */
-	if (device && device_is_paired(device)) {
-		DBG("already paired");
+	ret = check_device (adapter, remote_address);
+	if (ret != 0) {
 		eir_data_free(&eir_data);
-		return 1;
+		return ret;
 	}
 
-	/* Pairing in progress... */
-	if (device && device_is_bonding(device, NULL)) {
-		DBG("pairing in progress");
-		eir_data_free(&eir_data);
-		return -EINPROGRESS;
-	}
-
-	/* If we have unpaired device hanging around, purge it */
-	if (device)
-		adapter_remove_device(adapter, device, TRUE);
-
 	/* store OOB data */
 	if (eir_data.class != 0)
 		write_remote_class(adapter_get_address(adapter),
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 3/4] neard: Add support for nokia.com:bt type
From: Szymon Janc @ 2012-11-19 15:03 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1353337438-21865-1-git-send-email-szymon.janc@tieto.com>

This adds support for parsing nokia.com:bt type supported by some Nokia
NFC enabled devices. This is not fully documented Nokia extension so
to keep implementation sane and simple only PushOOB method suports
this type so only static handover is possible.
---
 plugins/neard.c |  198 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 196 insertions(+), 2 deletions(-)

diff --git a/plugins/neard.c b/plugins/neard.c
index 8319445..4627f52 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -322,6 +322,194 @@ static int process_eir(struct btd_adapter *adapter, uint8_t *eir, size_t size,
 	return 0;
 }
 
+/*
+ * This is (barely documented) Nokia extension format, most work was done by
+ * reverse engineering.
+ *
+ * Binary format varies among different devices, type depends on first byte
+ * 0x00 - BT address not reversed, 16 bytes authentication data (all zeros)
+ * 0x01 - BT address not reversed, 16 bytes authentication data (4 digit PIN,
+ *        padded with zeros)
+ * 0x02 - BT address not reversed, 16 bytes authentication data (not sure if
+ *        16 digit PIN or link key?, Nokia refers to it as ' Public Key')
+ * 0x10 - BT address reversed, no authentication data
+ * 0x24 - BT address not reversed, 4 bytes authentication data (4 digit PIN)
+ *
+ * General structure:
+ * 1 byte  - marker
+ * 6 bytes - BT address (reversed or not, depends on marker)
+ * 3 bytes - Class of Device
+ * 0, 4 or 16 bytes - authentication data, interpretation depends on marker
+ * 1 bytes - name length
+ * N bytes - name
+ */
+
+struct nokia_com_bt {
+	bdaddr_t address;
+	uint32_t cod;
+	uint8_t pin[16];
+	int pin_len;
+	char *name;
+};
+
+static int process_nokia_long (void *data, size_t size, uint8_t marker,
+						struct nokia_com_bt *nokia)
+{
+	struct {
+		bdaddr_t address;
+		uint8_t class[3];
+		uint8_t authentication[16];
+		uint8_t name_len;
+		uint8_t name[0];
+	}__attribute__((packed)) *n = data;
+
+	if (size != sizeof(*n) + n->name_len)
+		return -EINVAL;
+
+	/* address is not reverted */
+	baswap(&nokia->address, &n->address);
+
+	nokia->cod = n->class[0] | (n->class[1] << 8) | (n->class[2] << 16);
+
+	if (n->name_len > 0)
+		nokia->name = g_strndup((char *)n->name, n->name_len);
+
+	if (marker == 0x01) {
+		memcpy(nokia->pin, n->authentication, 4);
+		nokia->pin_len = 4;
+	} else if (marker == 0x02) {
+		memcpy(nokia->pin, n->authentication, 16);
+		nokia->pin_len = 16;
+	}
+
+	return 0;
+}
+
+static int process_nokia_short (void *data, size_t size,
+						struct nokia_com_bt *nokia)
+{
+	struct {
+		bdaddr_t address;
+		uint8_t class[3];
+		uint8_t authentication[4];
+		uint8_t name_len;
+		uint8_t name[0];
+	}__attribute__((packed)) *n = data;
+
+	if (size != sizeof(*n) + n->name_len)
+		return -EINVAL;
+
+	/* address is not reverted */
+	baswap(&nokia->address, &n->address);
+
+	nokia->cod = n->class[0] | (n->class[1] << 8) | (n->class[2] << 16);
+
+	if (n->name_len > 0)
+		nokia->name = g_strndup((char *)n->name, n->name_len);
+
+	memcpy(nokia->pin, n->authentication, 4);
+	nokia->pin_len = 4;
+
+	return 0;
+}
+
+static int process_nokia_extra_short (void *data, size_t size,
+						struct nokia_com_bt *nokia)
+{
+	struct {
+		bdaddr_t address;
+		uint8_t class[3];
+		uint8_t name_len;
+		uint8_t name[0];
+	}__attribute__((packed)) *n = data;
+
+	if (size != sizeof(*n) + n->name_len)
+		return -EINVAL;
+
+	bacpy(&nokia->address, &n->address);
+
+	nokia->cod = n->class[0] | (n->class[1] << 8) | (n->class[2] << 16);
+
+	if (n->name_len > 0)
+		nokia->name = g_strndup((char *)n->name, n->name_len);
+
+	return 0;
+}
+
+/* returns 1 if pairing is not needed */
+static int process_nokia_com_bt(struct btd_adapter *adapter, void *data,
+						size_t size, bdaddr_t *remote)
+{
+	uint8_t *marker;
+	struct nokia_com_bt nokia;
+	int ret;
+	char remote_address[18];
+
+	/* Support this only for PushOOB */
+	if (!remote)
+		return -EOPNOTSUPP;
+
+	marker = data++;
+	size --;
+
+	DBG("marker: 0x%.2x  size: %zu", *marker, size);
+
+	memset(&nokia, 0, sizeof(nokia));
+
+	switch (*marker) {
+	case 0x00:
+	case 0x01:
+	case 0x02:
+		ret = process_nokia_long(data, size, *marker, &nokia);
+		break;
+	case 0x10:
+		ret = process_nokia_extra_short(data, size, &nokia);
+		break;
+	case 0x24:
+		ret = process_nokia_short(data, size, &nokia);
+		break;
+	default:
+		info("Not supported Nokia NFC extension (0x%.2x)", *marker);
+		ret = -EPROTONOSUPPORT;
+		break;
+	}
+
+	if (ret < 0)
+		return ret;
+
+	ba2str(&nokia.address, remote_address);
+	DBG("hci%u remote:%s", adapter_get_dev_id(adapter), remote_address);
+
+	ret = check_device(adapter, remote_address);
+	if (ret != 0) {
+		g_free(nokia.name);
+		return ret;
+	}
+
+	DBG("hci%u remote:%s", adapter_get_dev_id(adapter), remote_address);
+
+	if (nokia.name) {
+		btd_event_remote_name(adapter_get_address(adapter), remote,
+								nokia.name);
+		g_free(nokia.name);
+	}
+
+	if (nokia.cod != 0)
+		write_remote_class(adapter_get_address(adapter), remote,
+								nokia.cod);
+
+	if (nokia.pin_len > 0) {
+		/* TODO
+		 * Handle PIN, for now only discovery mode and 'common' PINs
+		 * that might be provided by agent will work correctly.
+		 */
+	}
+
+	bacpy(remote, &nokia.address);
+
+	return 0;
+}
+
 static int process_params(DBusMessage *msg, struct btd_adapter *adapter,
 							bdaddr_t *remote)
 {
@@ -371,8 +559,14 @@ static int process_params(DBusMessage *msg, struct btd_adapter *adapter,
 
 		return process_eir(adapter, eir, size, remote);
 	} else if (strcasecmp(key, "nokia.com:bt") == 0) {
-		/* TODO add support for Nokia BT 2.0 proprietary stuff */
-		return -ENOTSUP;
+		DBusMessageIter array;
+		uint8_t *data;
+		int size;
+
+		dbus_message_iter_recurse(&value, &array);
+		dbus_message_iter_get_fixed_array(&array, &data, &size);
+
+		return process_nokia_com_bt(adapter, data, size, remote);
 	}
 
 	return -EINVAL;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 4/4] neard: Append hash/randomizer in EIR only if remote provided it
From: Szymon Janc @ 2012-11-19 15:03 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1353337438-21865-1-git-send-email-szymon.janc@tieto.com>

Read local OOB data for RequestOOB reply only if remote also provided
hash and randomizer in EIR. This will allow for faster reply when only
discovery is done OOB. It is also required to pass NFC handover test
related to Bluetooth just-works pairing.
---
 plugins/neard.c |   89 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 55 insertions(+), 34 deletions(-)

diff --git a/plugins/neard.c b/plugins/neard.c
index 4627f52..f8d023c 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -158,6 +158,46 @@ unregister:
 							AGENT_INTERFACE);
 }
 
+static DBusMessage *create_request_oob_reply(struct btd_adapter *adapter,
+							uint8_t *hash,
+							uint8_t *randomizer,
+							DBusMessage *msg)
+{
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	uint8_t eir[NFC_OOB_EIR_MAX];
+	uint8_t *peir = eir;
+	int len;
+
+	len = eir_create_oob(adapter_get_address(adapter),
+				btd_adapter_get_name(adapter),
+				btd_adapter_get_class(adapter), hash,
+				randomizer, main_opts.did_vendor,
+				main_opts.did_product, main_opts.did_version,
+				main_opts.did_source,
+				btd_adapter_get_services(adapter), eir);
+
+	reply = dbus_message_new_method_return(msg);
+	if (!reply)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_VARIANT_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
+				&dict);
+
+	dict_append_array(&dict, "EIR", DBUS_TYPE_BYTE, &peir, len);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
 static void read_local_complete(struct btd_adapter *adapter, uint8_t *hash,
 					uint8_t *randomizer, void *user_data)
 {
@@ -177,39 +217,10 @@ static void read_local_complete(struct btd_adapter *adapter, uint8_t *hash,
 		return;
 	}
 
-	if (hash && randomizer) {
-		int len;
-		uint8_t eir[NFC_OOB_EIR_MAX];
-		uint8_t *peir = eir;
-		DBusMessageIter iter;
-		DBusMessageIter dict;
-
-		len = eir_create_oob(adapter_get_address(adapter),
-				btd_adapter_get_name(adapter),
-				btd_adapter_get_class(adapter), hash,
-				randomizer, main_opts.did_vendor,
-				main_opts.did_product, main_opts.did_version,
-				main_opts.did_source,
-				btd_adapter_get_services(adapter), eir);
-
-		reply = dbus_message_new_method_return(msg);
-
-		dbus_message_iter_init_append(reply, &iter);
-
-		dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-					DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-					DBUS_TYPE_STRING_AS_STRING
-					DBUS_TYPE_VARIANT_AS_STRING
-					DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
-					&dict);
-
-		dict_append_array(&dict, "EIR", DBUS_TYPE_BYTE, &peir, len);
-
-		dbus_message_iter_close_container(&iter, &dict);
-
-	} else {
+	if (hash && randomizer)
+		reply = create_request_oob_reply(adapter, hash, randomizer, msg);
+	else
 		reply = error_reply(msg, EIO);
-	}
 
 	dbus_message_unref(msg);
 
@@ -273,7 +284,7 @@ static int check_device(struct btd_adapter *adapter, const char *address)
 	return 0;
 }
 
-/* returns 1 if pairing is not needed */
+/* returns 1 if action (pairing or reading local data) is not needed */
 static int process_eir(struct btd_adapter *adapter, uint8_t *eir, size_t size,
 							bdaddr_t *remote)
 {
@@ -317,9 +328,16 @@ static int process_eir(struct btd_adapter *adapter, uint8_t *eir, size_t size,
 	if (remote)
 		bacpy(remote, &eir_data.addr);
 
+	/*
+	 * In RequestOOB reply append local hash and randomizer only if
+	 * received EIR also contained it.
+	 */
+	if (!remote && !eir_data.hash)
+		ret = 1;
+
 	eir_data_free(&eir_data);
 
-	return 0;
+	return ret;
 }
 
 /*
@@ -651,6 +669,9 @@ static DBusMessage *request_oob(DBusConnection *conn, DBusMessage *msg,
 	if (ret < 0)
 		return error_reply(msg, -ret);
 
+	if (ret == 1)
+		return create_request_oob_reply(adapter, NULL, NULL, msg);
+
 	ret = btd_adapter_read_local_oob_data(adapter);
 	if (ret < 0)
 		return error_reply(msg, -ret);
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH] Bluetooth: Remove device OOB data if it was discovered in band
From: Johan Hedberg @ 2012-11-19 15:05 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1353336407-10863-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Mon, Nov 19, 2012, Szymon Janc wrote:
> OOB authentication mechanism should be used only if pairing process
> has been activated by previous OOB information exchange (Core Spec
> 4.0 , vol. 1, Part A, 5.1.4.3). Stored OOB data for specific device
> should be removed if that device was discovered in band later on.
> 
> Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> ---
> 
> This could also be done by userspace but would require calling remove remote
> OOB data mgmt command for every device found. Userspace could also track for
> which devices OOB data were added but this could be problematic as OOB data
> persists userspace restart. Doing it in kernel seems better.
> 
> 
>  net/bluetooth/hci_event.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 9f5c5f2..cda5bac 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1946,6 +1946,8 @@ static void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  	for (; num_rsp; num_rsp--, info++) {
>  		bool name_known, ssp;
>  
> +		hci_remove_remote_oob_data(hdev, &info->bdaddr);
> +
>  		bacpy(&data.bdaddr, &info->bdaddr);
>  		data.pscan_rep_mode	= info->pscan_rep_mode;
>  		data.pscan_period_mode	= info->pscan_period_mode;

Why would you do this only in hci_inquiry_result_evt? What about
hci_extended_inquiry_result_evt and hci_inquiry_result_with_rssi_evt?
Maybe hci_inquiry_cache_update would be a better function to put this
into.

Johan

^ permalink raw reply

* Re: [PATCH BlueZ 2/4] gdbus: Automatically register '/' path
From: Luiz Augusto von Dentz @ 2012-11-19 15:08 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CAMOw1v43vHHseyKKXK6t2sooS6dFkQdwbF4nOcCUGojpgbmGzQ@mail.gmail.com>

Hi Lucas,

On Mon, Nov 19, 2012 at 3:18 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> On Mon, Nov 19, 2012 at 10:49 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Lucas,
>>
>> On Mon, Nov 19, 2012 at 2:06 PM, Lucas De Marchi
>> <lucas.demarchi@profusion.mobi> wrote:
>>> Hi Luiz,
>>>
>>> On Mon, Nov 19, 2012 at 6:46 AM, Luiz Augusto von Dentz
>>> <luiz.dentz@gmail.com> wrote:
>>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>>
>>>> This makes g_dbus_setup_bus to automatically register '/' path so
>>>> user application that don't export any interface on '/' will have it
>>>> registered for ObjectManager.
>>>>
>>>> Note that it is now required to call g_dbus_close before exit.
>>>
>>> I'm no really fan of this. Why do we need to register '/' now?  If we
>>> are not going to support ObjectManager interfaces in subtrees, it
>>> would be easier to just move the ObjectManager to the shortest path
>>> registered rather than always registering '/'.
>>
>> Maybe I should have made clear this in the description, if you look at
>> the spec it suggest that each sub-tree root should implement
>> ObjectManager, the current implementation does that but if you think
>> about it probably doesn't make sense to have sub-trees because that
>> would creates extra round trips that ObjectManager was meant to
>
> HUmn... I think you are a bit confused now.
>
> 1 entire tree can be managed by a single ObjectManager, or multiple
> ones if there are objects that are not interesting to everybody and
> only a few types of clients would be interested in the objects changed
> beyond that path.
>
> Example:
>
> /org/bluez
>   - ObjectManager
> /org/bluez/adapter/bla/bli
>   - MyInterfaceWithLotsOfObjects
>
> If you put only 1 ObjectManager it means you get called for each
> change in /org/bluez/adapter/bla/bli even though most of the clients
> will not be interested in that. Therefore the spec allows you to
> attach another ObjectManager in /org/bluez/adapter/[...]  so the first
> ObjectManager only send updates and "manage" the objects until the
> path containing another ObjectManager.
>
> I do think we don't really need that feature right now. I think we are
> fine with a single ObjectManager in "/", but always registering "/"
> IMO is not the right solution.

As far I know /org/bluez/adapter/bla/bli is a child of /org/bluez thus
you don't need another ObjectManager, anyway the point is that
multiple ObjectManagers for the same connection might not make sense
as it will create more round trips to discover the object tree e.g:
/org/bluez vs /org/somethingelse

There is also the problem of supporting changing the root, the spec
doesn't mention anything about that even though it says it can support
sub-trees thus the application could in theory register a new root
e.g. /org at any point which basically invalidates the entire
sub-trees which I don't think would be convenient for anyone watching
it.

>> prevent, also Im not completely sure now but I remember someone
>> mentioning the '/' is kind special and should always be available no
>> matter what, so by registering '/' directly on g_dbus_setup_bus we
>> guarantee we have it always and that no sub-tree is going to be
>> generated since '/' is the root no matter what path the user code
>> register.
>
> Kind of true... what really needs to be treated are calls to
> Introspectable interface on "/". This is used by tools like d-feet to
> navigate the tree (looking for the "<node ...>" tags) -- I *think*
> this is also used by some bindings like python's. If you look in other
> projects around, they prefer using the project's namespace rather than
> "/"  (examples: systemd, udisks, policykit, dconf).
>
> So, if we register "/", but not an Introspectable interface in there,
> it means we will break tools like d-feet. If we don't register it,
> libdbus will handle the messages to Introspectable (this is why other
> projects don't even bother about "/").

Yep, I assume it was more convenient to have in '/' because it would
basically work in any case, but we can default to something else it
just a matter to have this set directly when we do g_dbus_setup_bus so
it is available since the beginning.

--
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH] Bluetooth: Remove device OOB data if it was discovered in band
From: Szymon Janc @ 2012-11-19 15:17 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20121119150537.GA10702@x220>

On Monday 19 of November 2012 17:05:37 Johan Hedberg wrote:
> Hi Szymon,

Hi Johan,

> 
> On Mon, Nov 19, 2012, Szymon Janc wrote:
> > OOB authentication mechanism should be used only if pairing process
> > has been activated by previous OOB information exchange (Core Spec
> > 4.0 , vol. 1, Part A, 5.1.4.3). Stored OOB data for specific device
> > should be removed if that device was discovered in band later on.
> > 
> > Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> > ---
> > 
> > This could also be done by userspace but would require calling remove remote
> > OOB data mgmt command for every device found. Userspace could also track for
> > which devices OOB data were added but this could be problematic as OOB data
> > persists userspace restart. Doing it in kernel seems better.
> > 
> > 
> >  net/bluetooth/hci_event.c |    2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 9f5c5f2..cda5bac 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -1946,6 +1946,8 @@ static void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >  	for (; num_rsp; num_rsp--, info++) {
> >  		bool name_known, ssp;
> >  
> > +		hci_remove_remote_oob_data(hdev, &info->bdaddr);
> > +
> >  		bacpy(&data.bdaddr, &info->bdaddr);
> >  		data.pscan_rep_mode	= info->pscan_rep_mode;
> >  		data.pscan_period_mode	= info->pscan_period_mode;
> 
> Why would you do this only in hci_inquiry_result_evt? What about
> hci_extended_inquiry_result_evt and hci_inquiry_result_with_rssi_evt?
> Maybe hci_inquiry_cache_update would be a better function to put this
> into.

Yes, I guess hci_inquiry_cache_update will be the right place to put this.
Will send V2. 

Thx!

> Johan

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: [PATCH BlueZ 2/4] gdbus: Automatically register '/' path
From: Lucas De Marchi @ 2012-11-19 15:38 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZ+oP-2YarvFndGCrUeGAqb=Nt7CxQ=1ULasaRXqvRy68A@mail.gmail.com>

On Mon, Nov 19, 2012 at 1:08 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Lucas,
>
> On Mon, Nov 19, 2012 at 3:18 PM, Lucas De Marchi
> <lucas.demarchi@profusion.mobi> wrote:
>> On Mon, Nov 19, 2012 at 10:49 AM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> Hi Lucas,
>>>
>>> On Mon, Nov 19, 2012 at 2:06 PM, Lucas De Marchi
>>> <lucas.demarchi@profusion.mobi> wrote:
>>>> Hi Luiz,
>>>>
>>>> On Mon, Nov 19, 2012 at 6:46 AM, Luiz Augusto von Dentz
>>>> <luiz.dentz@gmail.com> wrote:
>>>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>>>
>>>>> This makes g_dbus_setup_bus to automatically register '/' path so
>>>>> user application that don't export any interface on '/' will have it
>>>>> registered for ObjectManager.
>>>>>
>>>>> Note that it is now required to call g_dbus_close before exit.
>>>>
>>>> I'm no really fan of this. Why do we need to register '/' now?  If we
>>>> are not going to support ObjectManager interfaces in subtrees, it
>>>> would be easier to just move the ObjectManager to the shortest path
>>>> registered rather than always registering '/'.
>>>
>>> Maybe I should have made clear this in the description, if you look at
>>> the spec it suggest that each sub-tree root should implement
>>> ObjectManager, the current implementation does that but if you think
>>> about it probably doesn't make sense to have sub-trees because that
>>> would creates extra round trips that ObjectManager was meant to
>>
>> HUmn... I think you are a bit confused now.
>>
>> 1 entire tree can be managed by a single ObjectManager, or multiple
>> ones if there are objects that are not interesting to everybody and
>> only a few types of clients would be interested in the objects changed
>> beyond that path.
>>
>> Example:
>>
>> /org/bluez
>>   - ObjectManager
>> /org/bluez/adapter/bla/bli
>>   - MyInterfaceWithLotsOfObjects
>>
>> If you put only 1 ObjectManager it means you get called for each
>> change in /org/bluez/adapter/bla/bli even though most of the clients
>> will not be interested in that. Therefore the spec allows you to
>> attach another ObjectManager in /org/bluez/adapter/[...]  so the first
>> ObjectManager only send updates and "manage" the objects until the
>> path containing another ObjectManager.
>>
>> I do think we don't really need that feature right now. I think we are
>> fine with a single ObjectManager in "/", but always registering "/"
>> IMO is not the right solution.
>
> As far I know /org/bluez/adapter/bla/bli is a child of /org/bluez thus
> you don't need another ObjectManager, anyway the point is that

Sure we don't need it, but it was made that way so projects could
actually use multiple object managers for the reasons pointed out in
the previous email. Projects could even have separate ObjectManagers
for completely different things, like one in /org/bluez/bla and
another in /org/bluez/bli instead of 1 in /org/bluez. But it's an
application decision where to put the ObjectManager.

If all projects using gdbus decide to have a single ObjectManager on
"/" because it's simpler and fit the needs for everybody, then fine.
I'd suggest to even register it earlier than it is. Otherwise we might
choose to let the application (bluez, connman, ofono, etc) decide
where to put it, by adding a "g_dbus_attach_object_manager(conn,
path)" or the like.


> multiple ObjectManagers for the same connection might not make sense
> as it will create more round trips to discover the object tree e.g:
> /org/bluez vs /org/somethingelse

I'm not arguing about one object manager per object path component.

>
> There is also the problem of supporting changing the root, the spec
> doesn't mention anything about that even though it says it can support
> sub-trees thus the application could in theory register a new root
> e.g. /org at any point which basically invalidates the entire
> sub-trees which I don't think would be convenient for anyone watching
> it.

then it's a misuse of object manager by the application, not a problem
with the spec. We need to define in the API where the object
manager(s) will be and not randomly adding/removing them.

Lucas De Marchi

^ permalink raw reply

* [PATCH 0/2] Add support for SCO defered setup in test
From: Frédéric Dalleau @ 2012-11-19 15:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau

This adds support for deferred setup in userspace test tools for SCO
connections.

Regards,
Frédéric

Frédéric Dalleau (2):
  scotest: Add deferred setup option
  btiotest: Enable deferred setup for sco sockets

 test/btiotest.c |   22 +++++++++++++++-----
 test/scotest.c  |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 72 insertions(+), 10 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* [PATCH 1/2] scotest: Add deferred setup option
From: Frédéric Dalleau @ 2012-11-19 15:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1353340244-16443-1-git-send-email-frederic.dalleau@linux.intel.com>

---
 test/scotest.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/test/scotest.c b/test/scotest.c
index de65edf..a40e395 100644
--- a/test/scotest.c
+++ b/test/scotest.c
@@ -57,6 +57,8 @@ static long data_size = 672;
 
 static bdaddr_t bdaddr;
 
+static int defer_setup = 0;
+
 static float tv2fl(struct timeval tv)
 {
 	return (float)tv.tv_sec + (float)(tv.tv_usec/1000000.0);
@@ -147,6 +149,14 @@ static void do_listen(void (*handler)(int sk))
 		goto error;
 	}
 
+	/* Enable deferred setup */
+	if (defer_setup && setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
+				&defer_setup, sizeof(defer_setup)) < 0) {
+		syslog(LOG_ERR, "Can't enable deferred setup : %s (%d)",
+							strerror(errno), errno);
+		goto error;
+	}
+
 	/* Listen for connections */
 	if (listen(sk, 10)) {
 		syslog(LOG_ERR,"Can not listen on the socket: %s (%d)",
@@ -181,8 +191,10 @@ static void do_listen(void (*handler)(int sk))
 		if (getsockopt(nsk, SOL_SCO, SCO_CONNINFO, &conn, &optlen) < 0) {
 			syslog(LOG_ERR, "Can't get SCO connection information: %s (%d)",
 							strerror(errno), errno);
-			close(nsk);
-			goto error;
+			if (!defer_setup) {
+				close(nsk);
+				goto error;
+			}
 		}
 
 		ba2str(&addr.sco_bdaddr, ba);
@@ -190,6 +202,18 @@ static void do_listen(void (*handler)(int sk))
 			ba, conn.hci_handle,
 			conn.dev_class[2], conn.dev_class[1], conn.dev_class[0]);
 
+		/* Handle deferred setup */
+		if (defer_setup) {
+			syslog(LOG_INFO, "Waiting for %d seconds",
+							abs(defer_setup) - 1);
+			sleep(abs(defer_setup) - 1);
+
+			if (defer_setup < 0) {
+				close(nsk);
+				goto error;
+			}
+		}
+
 		handler(nsk);
 
 		syslog(LOG_INFO, "Disconnect");
@@ -207,6 +231,15 @@ static void dump_mode(int sk)
 {
 	int len;
 
+	if (defer_setup) {
+		len = read(sk, buf, sizeof(buf));
+		if (len < 0)
+			syslog(LOG_ERR, "Initial read error: %s (%d)",
+						strerror(errno), errno);
+		else
+			syslog(LOG_INFO, "Initial bytes %d", len);
+	}
+
 	syslog(LOG_INFO,"Receiving ...");
 	while ((len = read(sk, buf, data_size)) > 0)
 		syslog(LOG_INFO, "Recevied %d bytes", len);
@@ -216,6 +249,16 @@ static void recv_mode(int sk)
 {
 	struct timeval tv_beg,tv_end,tv_diff;
 	long total;
+	int len;
+
+	if (defer_setup) {
+		len = read(sk, buf, sizeof(buf));
+		if (len < 0)
+			syslog(LOG_ERR, "Initial read error: %s (%d)",
+						strerror(errno), errno);
+		else
+			syslog(LOG_INFO, "Initial bytes %d", len);
+	}
 
 	syslog(LOG_INFO, "Receiving ...");
 
@@ -328,14 +371,17 @@ static void usage(void)
 {
 	printf("scotest - SCO testing\n"
 		"Usage:\n");
-	printf("\tscotest <mode> [-b bytes] [bd_addr]\n");
+	printf("\tscotest <mode> [options] [bd_addr]\n");
 	printf("Modes:\n"
 		"\t-d dump (server)\n"
 		"\t-c reconnect (client)\n"
 		"\t-m multiple connects (client)\n"
 		"\t-r receive (server)\n"
 		"\t-s connect and send (client)\n"
-		"\t-n connect and be silent (client)\n");
+		"\t-n connect and be silent (client)\n"
+		"Options:\n"
+		"\t[-b bytes]\n"
+		"\t[-W seconds] enable deferred setup\n");
 }
 
 int main(int argc ,char *argv[])
@@ -343,7 +389,7 @@ int main(int argc ,char *argv[])
 	struct sigaction sa;
 	int opt, sk, mode = RECV;
 
-	while ((opt=getopt(argc,argv,"rdscmnb:")) != EOF) {
+	while ((opt = getopt(argc, argv, "rdscmnb:W:")) != EOF) {
 		switch(opt) {
 		case 'r':
 			mode = RECV;
@@ -373,6 +419,10 @@ int main(int argc ,char *argv[])
 			data_size = atoi(optarg);
 			break;
 
+		case 'W':
+			defer_setup = atoi(optarg);
+			break;
+
 		default:
 			usage();
 			exit(1);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/2] btiotest: Enable deferred setup for sco sockets
From: Frédéric Dalleau @ 2012-11-19 15:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1353340244-16443-1-git-send-email-frederic.dalleau@linux.intel.com>

---
 test/btiotest.c |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/test/btiotest.c b/test/btiotest.c
index 4cafcc3..593bafc 100644
--- a/test/btiotest.c
+++ b/test/btiotest.c
@@ -461,24 +461,35 @@ static void sco_connect(const char *src, const char *dst, gint disconn)
 	}
 }
 
-static void sco_listen(const char *src, gint disconn)
+static void sco_listen(const char *src, gboolean defer, gint reject,
+				gint disconn, gint accept)
 {
 	struct io_data *data;
+	BtIOConnect conn;
+	BtIOConfirm cfm;
 	GIOChannel *sco_srv;
 	GError *err = NULL;
 
 	printf("Listening for SCO connections\n");
 
-	data = io_data_new(NULL, -1, disconn, -1);
+	if (defer) {
+		conn = NULL;
+		cfm = confirm_cb;
+	} else {
+		conn = connect_cb;
+		cfm = NULL;
+	}
+
+	data = io_data_new(NULL, reject, disconn, accept);
 
 	if (src)
-		sco_srv = bt_io_listen(connect_cb, NULL, data,
+		sco_srv = bt_io_listen(conn, cfm, data,
 					(GDestroyNotify) io_data_unref,
 					&err,
 					BT_IO_OPT_SOURCE, src,
 					BT_IO_OPT_INVALID);
 	else
-		sco_srv = bt_io_listen(connect_cb, NULL, data,
+		sco_srv = bt_io_listen(conn, cfm, data,
 					(GDestroyNotify) io_data_unref,
 					&err, BT_IO_OPT_INVALID);
 
@@ -588,7 +599,8 @@ int main(int argc, char *argv[])
 		if (argc > 1)
 			sco_connect(opt_dev, argv[1], opt_disconn);
 		else
-			sco_listen(opt_dev, opt_disconn);
+			sco_listen(opt_dev, opt_defer, opt_reject,
+					opt_disconn, opt_accept);
 	}
 
 	signal(SIGTERM, sig_term);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 0/2] sco: BT_DEFER_SETUP for SCO sockets
From: Frédéric Dalleau @ 2012-11-19 16:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau

Hi,

This is a better view about what can be done to implement DEFER_SETUP on SCO
sockets. hci layer get some changes since previous behavior was to accept all
SCO connections.
The ugly hci_proto_defer has been removed and replaced by an additional flag
parameter to hci_proto_connect_ind.
Regarding testing, I'm still stuck by a txt timeout issue however, if the
delay between accept and recv is set to 0, then the connection is working fine.
And I even managed to get the sco connection established correctly once. So I
believed this emulator problem.
Now I'm trying without emulator.

Let me know what you think.

Best regards,
Frédéric


Frédéric Dalleau (2):
  Bluetooth: Add BT_DEFER_SETUP option to sco socket
  Bluetooth: Implement deferred sco socket setup

 include/net/bluetooth/hci_core.h |    9 +++--
 net/bluetooth/hci_event.c        |   53 +++++++++++++++++++++++++++---
 net/bluetooth/sco.c              |   67 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 119 insertions(+), 10 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* [PATCH 1/2] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Frédéric Dalleau @ 2012-11-19 16:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1353342958-25303-1-git-send-email-frederic.dalleau@linux.intel.com>

This option will set the BT_SK_DEFER_SETUP bit in socket flags.
---
 net/bluetooth/sco.c |   32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 450cdcd..c6678f2 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -666,12 +666,31 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char
 {
 	struct sock *sk = sock->sk;
 	int err = 0;
+	u32 opt;
 
 	BT_DBG("sk %p", sk);
 
 	lock_sock(sk);
 
 	switch (optname) {
+
+	case BT_DEFER_SETUP:
+		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (get_user(opt, (u32 __user *) optval)) {
+			err = -EFAULT;
+			break;
+		}
+
+		if (opt)
+			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
+		else
+			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -753,6 +772,19 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
 	lock_sock(sk);
 
 	switch (optname) {
+
+	case BT_DEFER_SETUP:
+		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags),
+			     (u32 __user *) optval))
+			err = -EFAULT;
+
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/2] Bluetooth: Implement deferred sco socket setup
From: Frédéric Dalleau @ 2012-11-19 16:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1353342958-25303-1-git-send-email-frederic.dalleau@linux.intel.com>

In order to authenticate and configure an incoming SCO connection, the
BT_DEFER_SETUP option was added. This option is intended to defer reply
to Connect Request on SCO sockets.
When a connection is requested, the listening socket is unblocked but
the effective connection setup happens only on first recv. Any send
between accept and recv fails with -ENOTCONN.
---
 include/net/bluetooth/hci_core.h |    9 ++++---
 net/bluetooth/hci_event.c        |   53 +++++++++++++++++++++++++++++++++++---
 net/bluetooth/sco.c              |   35 ++++++++++++++++++++++---
 3 files changed, 87 insertions(+), 10 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ef5b85d..8fceb35 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -376,7 +376,7 @@ extern int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt);
 extern int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb,
 			      u16 flags);
 
-extern int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr);
+extern int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags);
 extern void sco_connect_cfm(struct hci_conn *hcon, __u8 status);
 extern void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason);
 extern int sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb);
@@ -577,6 +577,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst);
 int hci_conn_del(struct hci_conn *conn);
 void hci_conn_hash_flush(struct hci_dev *hdev);
 void hci_conn_check_pending(struct hci_dev *hdev);
+void hci_conn_accept(struct hci_conn *conn, int mask);
 
 struct hci_chan *hci_chan_create(struct hci_conn *conn);
 void hci_chan_del(struct hci_chan *chan);
@@ -779,8 +780,10 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 #define lmp_host_le_br_capable(dev) ((dev)->host_features[0] & LMP_HOST_LE_BREDR)
 
 /* ----- HCI protocols ----- */
+#define HCI_PROTO_DEFER             0x01
+
 static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
-								__u8 type)
+						__u8 type, __u8 *flags)
 {
 	switch (type) {
 	case ACL_LINK:
@@ -788,7 +791,7 @@ static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
 
 	case SCO_LINK:
 	case ESCO_LINK:
-		return sco_connect_ind(hdev, bdaddr);
+		return sco_connect_ind(hdev, bdaddr, flags);
 
 	default:
 		BT_ERR("unknown link type %d", type);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9f5c5f2..f883ef5 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2047,15 +2047,54 @@ unlock:
 	hci_conn_check_pending(hdev);
 }
 
+void hci_conn_accept(struct hci_conn *conn, int mask)
+{
+	struct hci_dev *hdev = conn->hdev;
+
+	BT_DBG("conn %p", conn);
+
+	if (!lmp_esco_capable(hdev)) {
+		struct hci_cp_accept_conn_req cp;
+
+		conn->state = BT_CONFIG;
+		bacpy(&cp.bdaddr, &conn->dst);
+
+		if (lmp_rswitch_capable(hdev) && (mask & HCI_LM_MASTER))
+			cp.role = 0x00; /* Become master */
+		else
+			cp.role = 0x01; /* Remain slave */
+
+		hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp),
+			     &cp);
+	} else /* lmp_esco_capable(hdev)) */ {
+		struct hci_cp_accept_sync_conn_req cp;
+
+		conn->state = BT_CONFIG;
+		bacpy(&cp.bdaddr, &conn->dst);
+		cp.pkt_type = cpu_to_le16(conn->pkt_type);
+
+		cp.tx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
+		cp.rx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
+		cp.max_latency    = __constant_cpu_to_le16(0xffff);
+		cp.content_format = cpu_to_le16(hdev->voice_setting);
+		cp.retrans_effort = 0xff;
+
+		hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
+			     sizeof(cp), &cp);
+	}
+}
+
 static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_conn_request *ev = (void *) skb->data;
 	int mask = hdev->link_mode;
+	__u8 flags = 0;
 
 	BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr,
 	       ev->link_type);
 
-	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
+	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type,
+				      &flags);
 
 	if ((mask & HCI_LM_ACCEPT) &&
 	    !hci_blacklist_lookup(hdev, &ev->bdaddr)) {
@@ -2081,12 +2120,13 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		}
 
 		memcpy(conn->dev_class, ev->dev_class, 3);
-		conn->state = BT_CONNECT;
 
 		hci_dev_unlock(hdev);
 
-		if (ev->link_type == ACL_LINK || !lmp_esco_capable(hdev)) {
+		if (ev->link_type == ACL_LINK ||
+		    (!(flags & HCI_PROTO_DEFER) && !lmp_esco_capable(hdev))) {
 			struct hci_cp_accept_conn_req cp;
+			conn->state = BT_CONNECT;
 
 			bacpy(&cp.bdaddr, &ev->bdaddr);
 
@@ -2097,8 +2137,9 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 			hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp),
 				     &cp);
-		} else {
+		} else if (!(flags & HCI_PROTO_DEFER)) {
 			struct hci_cp_accept_sync_conn_req cp;
+			conn->state = BT_CONNECT;
 
 			bacpy(&cp.bdaddr, &ev->bdaddr);
 			cp.pkt_type = cpu_to_le16(conn->pkt_type);
@@ -2111,6 +2152,10 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 			hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
 				     sizeof(cp), &cp);
+		} else {
+			conn->state = BT_CONNECT2;
+			hci_proto_connect_cfm(conn, 0);
+			hci_conn_put(conn);
 		}
 	} else {
 		/* Connection rejected */
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index c6678f2..77210bf 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -397,6 +397,7 @@ static void sco_sock_init(struct sock *sk, struct sock *parent)
 
 	if (parent) {
 		sk->sk_type = parent->sk_type;
+		bt_sk(sk)->flags = bt_sk(parent)->flags;
 		security_sk_clone(parent, sk);
 	}
 }
@@ -662,6 +663,28 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	return err;
 }
 
+static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
+			      struct msghdr *msg, size_t len, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct sco_pinfo *pi = sco_pi(sk);
+
+	lock_sock(sk);
+
+	if (sk->sk_state == BT_CONNECT2 &&
+			test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
+		hci_conn_accept(pi->conn->hcon, 0);
+		sk->sk_state = BT_CONFIG;
+
+		release_sock(sk);
+		return 0;
+	}
+
+	release_sock(sk);
+
+	return bt_sock_recvmsg(iocb, sock, msg, len, flags);
+}
+
 static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen)
 {
 	struct sock *sk = sock->sk;
@@ -906,7 +929,10 @@ static void sco_conn_ready(struct sco_conn *conn)
 		hci_conn_hold(conn->hcon);
 		__sco_chan_add(conn, sk, parent);
 
-		sk->sk_state = BT_CONNECTED;
+		if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags))
+			sk->sk_state = BT_CONNECT2;
+		else
+			sk->sk_state = BT_CONNECTED;
 
 		/* Wake up parent */
 		parent->sk_data_ready(parent, 1);
@@ -919,7 +945,7 @@ done:
 }
 
 /* ----- SCO interface with lower layer (HCI) ----- */
-int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
+int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
 {
 	struct sock *sk;
 	struct hlist_node *node;
@@ -936,6 +962,9 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
 		if (!bacmp(&bt_sk(sk)->src, &hdev->bdaddr) ||
 		    !bacmp(&bt_sk(sk)->src, BDADDR_ANY)) {
 			lm |= HCI_LM_ACCEPT;
+
+			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags))
+				*flags |= HCI_PROTO_DEFER;
 			break;
 		}
 	}
@@ -1024,7 +1053,7 @@ static const struct proto_ops sco_sock_ops = {
 	.accept		= sco_sock_accept,
 	.getname	= sco_sock_getname,
 	.sendmsg	= sco_sock_sendmsg,
-	.recvmsg	= bt_sock_recvmsg,
+	.recvmsg	= sco_sock_recvmsg,
 	.poll		= bt_sock_poll,
 	.ioctl		= bt_sock_ioctl,
 	.mmap		= sock_no_mmap,
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 1/5] adapter: Remove not needed variable from btd_adapter_get_mode
From: Johan Hedberg @ 2012-11-19 18:32 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1353337366-21590-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Mon, Nov 19, 2012, Szymon Janc wrote:
> address is no longer used and can be removed.
> ---
>  src/adapter.c |    4 ----
>  1 file changed, 4 deletions(-)

All five patches have been applied. Thanks.

Johan

^ permalink raw reply

* [PATCH 0/3] Fix corrupted SDP response for sequence size > 256
From: Bart Westgeest @ 2012-11-19 19:04 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bart Westgeest

This patchset includes minor refactoring and a bug fix. The bug
is exposed when a data sequence grows beyond 256 bytes, in this case the
complete sequence data is written, but the size is truncated to one
byte, resulting in a corrupted SDP response.

RE: Feedback on potential fix for issue while advertising Feature List

Bart Westgeest (3):
  sdp: Inlined single use of function sdp_set_data_type
  sdp: Limit side effects of sdp_get_data_type and sdp_get_data_size
  sdp: Upgrade datatype SEQ8 to SEQ16 when data size is greater than
    256

 lib/sdp.c |   56 +++++++++++++++++++++++---------------------------------
 1 file changed, 23 insertions(+), 33 deletions(-)

-- 
1.7.10.4


^ permalink raw reply

* [PATCH 1/3] sdp: Inlined single use of function sdp_set_data_type
From: Bart Westgeest @ 2012-11-19 19:04 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bart Westgeest
In-Reply-To: <1353351872-10628-1-git-send-email-bart@elbrys.com>

Inlining single use of sdp_set_data_type to improve code readability,
since the function was doing more than just setting the data type.
---
 lib/sdp.c |   18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index 1a5bf01..dba3f59 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -666,18 +666,6 @@ static int sdp_get_data_type(sdp_buf_t *buf, uint8_t dtd)
 	return data_type;
 }
 
-static int sdp_set_data_type(sdp_buf_t *buf, uint8_t dtd)
-{
-	int data_type = 0;
-	uint8_t *p = buf->data + buf->data_size;
-
-	*p = dtd;
-	data_type = sdp_get_data_type(buf, dtd);
-	buf->data_size += data_type;
-
-	return data_type;
-}
-
 void sdp_set_attrid(sdp_buf_t *buf, uint16_t attr)
 {
 	uint8_t *p = buf->data;
@@ -815,9 +803,13 @@ int sdp_gen_pdu(sdp_buf_t *buf, sdp_data_t *d)
 	uint128_t u128;
 	uint8_t *seqp = buf->data + buf->data_size;
 
-	pdu_size = sdp_set_data_type(buf, dtd);
+	pdu_size = sdp_get_data_type(buf, dtd);
+	buf->data_size += pdu_size;
+
 	data_size = sdp_get_data_size(buf, d);
 
+	*seqp = dtd;
+
 	switch (dtd) {
 	case SDP_DATA_NIL:
 		break;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 2/3] sdp: Limit side effects of sdp_get_data_type and sdp_get_data_size
From: Bart Westgeest @ 2012-11-19 19:04 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bart Westgeest
In-Reply-To: <1353351872-10628-1-git-send-email-bart@elbrys.com>

Remove modification of buf->buf_size in 'get' functions. Data is
still indirectly modified due to recursive nature of code.

Renamed sdp_get_data_type to sdp_get_data_type_size.
---
 lib/sdp.c |   26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index dba3f59..026163e 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -633,37 +633,32 @@ void sdp_set_seq_len(uint8_t *ptr, uint32_t length)
 	}
 }
 
-static int sdp_get_data_type(sdp_buf_t *buf, uint8_t dtd)
+static int sdp_get_data_type_size(uint8_t dtd)
 {
-	int data_type = 0;
-
-	data_type += sizeof(uint8_t);
+	int size = sizeof(uint8_t);
 
 	switch (dtd) {
 	case SDP_SEQ8:
 	case SDP_TEXT_STR8:
 	case SDP_URL_STR8:
 	case SDP_ALT8:
-		data_type += sizeof(uint8_t);
+		size += sizeof(uint8_t);
 		break;
 	case SDP_SEQ16:
 	case SDP_TEXT_STR16:
 	case SDP_URL_STR16:
 	case SDP_ALT16:
-		data_type += sizeof(uint16_t);
+		size += sizeof(uint16_t);
 		break;
 	case SDP_SEQ32:
 	case SDP_TEXT_STR32:
 	case SDP_URL_STR32:
 	case SDP_ALT32:
-		data_type += sizeof(uint32_t);
+		size += sizeof(uint32_t);
 		break;
 	}
 
-	if (!buf->data)
-		buf->buf_size += data_type;
-
-	return data_type;
+	return size;
 }
 
 void sdp_set_attrid(sdp_buf_t *buf, uint16_t attr)
@@ -762,9 +757,6 @@ static int sdp_get_data_size(sdp_buf_t *buf, sdp_data_t *d)
 		break;
 	}
 
-	if (!buf->data)
-		buf->buf_size += data_size;
-
 	return data_size;
 }
 
@@ -783,8 +775,8 @@ static int sdp_gen_buffer(sdp_buf_t *buf, sdp_data_t *d)
 	/* attribute length */
 	buf->buf_size += sizeof(uint8_t) + sizeof(uint16_t);
 
-	sdp_get_data_type(buf, d->dtd);
-	sdp_get_data_size(buf, d);
+	buf->buf_size += sdp_get_data_type_size(d->dtd);
+	buf->buf_size += sdp_get_data_size(buf, d);
 
 	if (buf->buf_size > UCHAR_MAX && d->dtd == SDP_SEQ8)
 		buf->buf_size += sizeof(uint8_t);
@@ -803,7 +795,7 @@ int sdp_gen_pdu(sdp_buf_t *buf, sdp_data_t *d)
 	uint128_t u128;
 	uint8_t *seqp = buf->data + buf->data_size;
 
-	pdu_size = sdp_get_data_type(buf, dtd);
+	pdu_size = sdp_get_data_type_size(dtd);
 	buf->data_size += pdu_size;
 
 	data_size = sdp_get_data_size(buf, d);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 3/3] sdp: Upgrade datatype SEQ8 to SEQ16 when data size is greater than 256
From: Bart Westgeest @ 2012-11-19 19:04 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bart Westgeest
In-Reply-To: <1353351872-10628-1-git-send-email-bart@elbrys.com>

Fixes a bug where the complete sequence data is written, but the size
is truncated to one byte.
---
 lib/sdp.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index 026163e..ceb1192 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -786,23 +786,29 @@ static int sdp_gen_buffer(sdp_buf_t *buf, sdp_data_t *d)
 
 int sdp_gen_pdu(sdp_buf_t *buf, sdp_data_t *d)
 {
-	uint32_t pdu_size = 0, data_size = 0;
+	uint32_t pdu_size, data_size;
 	unsigned char *src = NULL, is_seq = 0, is_alt = 0;
-	uint8_t dtd = d->dtd;
 	uint16_t u16;
 	uint32_t u32;
 	uint64_t u64;
 	uint128_t u128;
 	uint8_t *seqp = buf->data + buf->data_size;
+	uint32_t orig_data_size = buf->data_size;
 
-	pdu_size = sdp_get_data_type_size(dtd);
+recalculate:
+	pdu_size = sdp_get_data_type_size(d->dtd);
 	buf->data_size += pdu_size;
 
 	data_size = sdp_get_data_size(buf, d);
+	if (data_size > UCHAR_MAX && d->dtd == SDP_SEQ8) {
+		buf->data_size = orig_data_size;
+		d->dtd = SDP_SEQ16;
+		goto recalculate;
+	}
 
-	*seqp = dtd;
+	*seqp = d->dtd;
 
-	switch (dtd) {
+	switch (d->dtd) {
 	case SDP_DATA_NIL:
 		break;
 	case SDP_UINT8:
@@ -884,7 +890,7 @@ int sdp_gen_pdu(sdp_buf_t *buf, sdp_data_t *d)
 		if (src && buf->buf_size >= buf->data_size + data_size) {
 			memcpy(buf->data + buf->data_size, src, data_size);
 			buf->data_size += data_size;
-		} else if (dtd != SDP_DATA_NIL) {
+		} else if (d->dtd != SDP_DATA_NIL) {
 			SDPDBG("Gen PDU : Can't copy from invalid source or dest\n");
 		}
 	}
-- 
1.7.10.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox