* [PATCH 04/10] Bluetooth: Refactor valid LTK data testing into its own function
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358515558-17861-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch refactors valid LTK data testing into its own function. This
will help keep the code readable since there are several tests still
missing that need to be done on the LTK data.
---
net/bluetooth/mgmt.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5388151..3634907 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2701,6 +2701,13 @@ done:
return err;
}
+static bool ltk_is_valid(struct mgmt_ltk_info *key)
+{
+ if (key->master != 0x00 && key->master != 0x01)
+ return false;
+ return true;
+}
+
static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
void *cp_data, u16 len)
{
@@ -2729,7 +2736,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
struct mgmt_ltk_info *key = &cp->keys[i];
u8 type;
- if (key->master != 0x00 && key->master != 0x01) {
+ if (!ltk_is_valid(key)) {
hci_smp_ltks_clear(hdev);
err = cmd_status(sk, hdev->id,
MGMT_OP_LOAD_LONG_TERM_KEYS,
--
1.7.10.4
^ permalink raw reply related
* [PATCH 03/10] Bluetooth: Fix checking for proper key->master value in Load LTKs
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358515558-17861-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The allowed values for the key->master parameter in the Load LTKs
command are 0x00 and 0x01. If there is a key in the list with some other
value the command should fail with a proper invalid params response.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a050eee..5388151 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2729,6 +2729,14 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
struct mgmt_ltk_info *key = &cp->keys[i];
u8 type;
+ if (key->master != 0x00 && key->master != 0x01) {
+ hci_smp_ltks_clear(hdev);
+ err = cmd_status(sk, hdev->id,
+ MGMT_OP_LOAD_LONG_TERM_KEYS,
+ MGMT_STATUS_INVALID_PARAMS);
+ goto unlock;
+ }
+
if (key->master)
type = HCI_SMP_LTK;
else
@@ -2743,6 +2751,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
err = cmd_complete(sk, hdev->id, MGMT_OP_LOAD_LONG_TERM_KEYS, 0,
NULL, 0);
+unlock:
hci_dev_unlock(hdev);
return err;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 02/10] Bluetooth: Fix returning proper mgmt status for Load LTKs
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358515558-17861-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
Failures of mgmt commands should be indicated with valid mgmt status
codes, and EINVAL is not one of them. Instead MGMT_STATUS_INVALID_PARAMS
should be returned.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d9b042e..a050eee 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2716,7 +2716,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
BT_ERR("load_keys: expected %u bytes, got %u bytes",
len, expected_len);
return cmd_status(sk, hdev->id, MGMT_OP_LOAD_LONG_TERM_KEYS,
- EINVAL);
+ MGMT_STATUS_INVALID_PARAMS);
}
BT_DBG("%s key_count %u", hdev->name, key_count);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 01/10] Bluetooth: Fix checking for correct mgmt_load_link_keys parameters
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358515558-17861-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The debug_keys parameter is only allowed to have the values 0x00 and
0x01. Any other value should result in a proper command status with
MGMT_STATUS_INVALID_PARAMS.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 36b2310..d9b042e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1519,6 +1519,10 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
MGMT_STATUS_INVALID_PARAMS);
}
+ if (cp->debug_keys != 0x00 && cp->debug_keys != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_LOAD_LINK_KEYS,
+ MGMT_STATUS_INVALID_PARAMS);
+
BT_DBG("%s debug_keys %u key_count %u", hdev->name, cp->debug_keys,
key_count);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
To: linux-bluetooth
Hi,
This patch set is intended to be applied after Szymon's "Bluetooth: Fix
pair device command reply if adapter is powered off" patch.
This set includes fixes to improve the validity checks on parameters we
receive from user space in mgmt commands. Each of these fixes has had
its own test case added to the user space mgmt-tester and has been
verified to work.
Johan
^ permalink raw reply
* RE: bluez: atomic operations
From: Kling, Andreas @ 2013-01-18 12:51 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1358400337.3059.11.camel@aeonflux>
Hi Marcel
>Hi Andy,
>
>> using gcc atomic buildins breaks support for older gcc/platforms.
>>
>> src/bluetoothd-adapter.o: In function `btd_adapter_unref':
>> adapter.c:(.text+0x63f8): undefined reference to `__sync_sub_and_fetch_4=
'
>>
>> I suggest to use glib g_atomic_* group of functions.
>> They fall back to traditional locking if buildins are not available.
>
>the GLib atomic API functions are not really stable. The GLib people for
>some reason decided to break their own API/ABI guarantee. So we moved
>away from using them. And I have no intention to get back to them.
>
>Long term, we are moving away from GLib and want to switch to ELL. So
>that means gcc atomics are required. My advise would be to get a modern
>platform and not rely on workarounds.
>
>Regards
>
>Marcel
many thanks for your answer.
unfortunately a modern platform is not an option for me, hopefully changes =
some day.
so I will maintain another local patch (yieha) :P
ELL? sounds interesting... will have a look at it.
regards
andy=
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix pair device command reply if adapter is powered off
From: Johan Hedberg @ 2013-01-18 12:35 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1358509687-7194-1-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Fri, Jan 18, 2013, Szymon Janc wrote:
> According to Bluetooth Management API specification Pair Device Command
> should generate command complete event on both success and failure.
> This fix replying with command status (which lacks address info) when
> adapter is powered off.
>
> Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> ---
> net/bluetooth/mgmt.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Acked-by: Johan Hedberg <johan.hedberg@intel.com>
Johan
^ permalink raw reply
* Re: [PATCH 0/5] MAP client: Add missing parameters for GetMessageListing
From: Luiz Augusto von Dentz @ 2013-01-18 12:12 UTC (permalink / raw)
To: Christian Fetzer; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1358258204-4090-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
Hi Christian,
On Tue, Jan 15, 2013 at 3:56 PM, Christian Fetzer
<christian.fetzer@oss.bmw-carit.de> wrote:
> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>
> This patchset adds the missing parameters Text, AttachmentSize and Status
> as well as the filter SubjectLength.
>
> Christian Fetzer (5):
> obexd: Add parameter SubjectLength to map_list_messages
> obexd: Move parse_size function in map.c
> obexd: Add parameter Text to GetMessageListing response
> obexd: Add parameter AttachmentSize to GetMessageListing response
> obexd: Add parameter Status to GetMessageListing response
>
> doc/obex-api.txt | 20 +++++++++++--
> obexd/client/map.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 101 insertions(+), 6 deletions(-)
>
> --
> 1.8.1
All 5 patches have been pushed upstream, just need to do a minor fix
to SubjectLength type since D-Bus don't really have uint8 I modified
it to byte.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Auto reconnect heartrate LE device
From: Damjan Cvetko @ 2013-01-18 12:06 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org
Hi.
Talked to jhe on IRC regarding this some days ago. I have a Polar heart rate monitor, and with the latest code it work well enough. However when the device goes out of range, it is disconnected from bluetoothd and does not reconnect when coming back into range:
bluetoothd[2898]: src/adapter.c:dev_disconnected() Device 00:22:D0:00:00:64 disconnected, reason 0
...
bluetoothd[2898]: src/device.c:attrib_disconnected_cb() Automatic connection disabled
I wanted to add auto reconnection, and this 1 line fix is what I came up with. It works in most cases, but sometimes bluetoothd fals into a loop.
...
"src/adapter.c:adapter_set_discovering() hci0 restarting discovery: disc_sessions 2"
"src/adapter.c:start_discovery_complete() Busy (0x0a)"
"src/adapter.c:adapter_set_discovering() hci0 restarting discovery: disc_sessions 2"
...
Still looking into that part, could also be unrelated. Here is the patch.
Best,
Damjan
---
Set auto connect for heartrate device.
diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 5c56d3f..1788d4f 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -801,6 +801,8 @@ static int heartrate_device_register(struct btd_device *device,
hr->attioid = btd_device_add_attio_callback(device, attio_connected_cb,
attio_disconnected_cb, hr);
+ device_set_auto_connect(device, TRUE);
+
return 0;
}
^ permalink raw reply related
* [PATCH 2/2] Adapter: Add workaround for device pairing kernel bug
From: Szymon Janc @ 2013-01-18 11:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1358509895-7460-1-git-send-email-szymon.janc@tieto.com>
Some kernels reply to device pairing command with bogus command status
instead of command complete event if adapter was not powered.
Command status event doesn't provide remote device address and type
needed to properly complete bonding request.
Pass possibly missing data as user_data to mgmt_send and use it in
pair_device_complete function if needed.
---
src/adapter.c | 38 +++++++++++++++++++++++++++++++++++---
1 file changed, 35 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 1a5b1ad..c413242 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4643,17 +4643,40 @@ static void adapter_bonding_complete(struct btd_adapter *adapter,
check_oob_bonding_complete(adapter, bdaddr, status);
}
+struct pair_device_data {
+ struct btd_adapter *adapter;
+ bdaddr_t bdaddr;
+ uint8_t addr_type;
+};
+
+static void free_pair_device_data(void *user_data)
+{
+ struct pair_device_data *data = user_data;
+
+ g_free(data);
+}
+
static void pair_device_complete(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
const struct mgmt_rp_pair_device *rp = param;
- struct btd_adapter *adapter = user_data;
+ struct pair_device_data *data = user_data;
DBG("%s (0x%02x)", mgmt_errstr(status), status);
+ /*
+ * Workaround for a kernel bug
+ *
+ * Broken kernels may reply to device pairing command with command
+ * status instead of command complete event e.g. if adapter was not
+ * powered.
+ */
if (status != MGMT_STATUS_SUCCESS && length < sizeof(*rp)) {
error("Pair device failed: %s (0x%02x)",
mgmt_errstr(status), status);
+
+ adapter_bonding_complete(data->adapter, &data->bdaddr,
+ data->addr_type, status);
return;
}
@@ -4662,7 +4685,7 @@ static void pair_device_complete(uint8_t status, uint16_t length,
return;
}
- adapter_bonding_complete(adapter, &rp->addr.bdaddr, rp->addr.type,
+ adapter_bonding_complete(data->adapter, &rp->addr.bdaddr, rp->addr.type,
status);
}
@@ -4671,6 +4694,7 @@ int adapter_create_bonding(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
{
struct mgmt_cp_pair_device cp;
char addr[18];
+ struct pair_device_data *data;
suspend_discovery(adapter);
@@ -4683,13 +4707,21 @@ int adapter_create_bonding(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
cp.addr.type = addr_type;
cp.io_cap = io_cap;
+ data = g_new0(struct pair_device_data, 1);
+ data->adapter = adapter;
+ bacpy(&data->bdaddr, bdaddr);
+ data->addr_type = addr_type;
+
if (mgmt_send(adapter->mgmt, MGMT_OP_PAIR_DEVICE,
adapter->dev_id, sizeof(cp), &cp,
- pair_device_complete, adapter, NULL) > 0)
+ pair_device_complete, data,
+ free_pair_device_data) > 0)
return 0;
error("Failed to pair %s for hci%u", addr, adapter->dev_id);
+ free_pair_device_data(data);
+
return -EIO;
}
--
1.8.0.3
^ permalink raw reply related
* [PATCH 1/2] tools: Add mgmt test case for pair device while powered off
From: Szymon Janc @ 2013-01-18 11:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
This tests if kernel is responding with proper command complete event.
---
tools/mgmt-tester.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index cb02480..5c72d00 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -916,6 +916,20 @@ static const struct generic_data add_uuid32_test_1 = {
.expect_hci_len = sizeof(write_eir_uuid32_hci),
};
+static const char pair_device_param[] = {
+ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x00, 0x00 };
+static const char pair_device_rsp[] = {
+ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x00 };
+
+static const struct generic_data pair_device_not_powered_test_1 = {
+ .send_opcode = MGMT_OP_PAIR_DEVICE,
+ .send_param = pair_device_param,
+ .send_len = sizeof(pair_device_param),
+ .expect_status = MGMT_STATUS_NOT_POWERED,
+ .expect_param = pair_device_rsp,
+ .expect_len = sizeof(pair_device_rsp),
+};
+
static void setup_powered_callback(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
@@ -1414,5 +1428,8 @@ int main(int argc, char *argv[])
test_bredr("Add UUID - UUID-32 1", &add_uuid32_test_1, setup_ssp,
test_command_generic);
+ test_bredr("Pair Device - Not Powered 1",
+ &pair_device_not_powered_test_1, NULL,
+ test_command_generic);
return tester_run();
}
--
1.8.0.3
^ permalink raw reply related
* [PATCH] Bluetooth: Fix pair device command reply if adapter is powered off
From: Szymon Janc @ 2013-01-18 11:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
According to Bluetooth Management API specification Pair Device Command
should generate command complete event on both success and failure.
This fix replying with command status (which lacks address info) when
adapter is powered off.
Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
---
net/bluetooth/mgmt.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fc171f2..3aa8c38 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1939,11 +1939,15 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("");
+ memset(&rp, 0, sizeof(rp));
+ bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
+ rp.addr.type = cp->addr.type;
+
hci_dev_lock(hdev);
if (!hdev_is_powered(hdev)) {
- err = cmd_status(sk, hdev->id, MGMT_OP_PAIR_DEVICE,
- MGMT_STATUS_NOT_POWERED);
+ err = cmd_complete(sk, hdev->id, MGMT_OP_PAIR_DEVICE,
+ MGMT_STATUS_NOT_POWERED, &rp, sizeof(rp));
goto unlock;
}
@@ -1960,10 +1964,6 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
conn = hci_connect(hdev, LE_LINK, &cp->addr.bdaddr,
cp->addr.type, sec_level, auth_type);
- memset(&rp, 0, sizeof(rp));
- bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
- rp.addr.type = cp->addr.type;
-
if (IS_ERR(conn)) {
int status;
--
1.8.0.3
^ permalink raw reply related
* Re: [PATCH v2] core: Fix allow registering same custom property
From: Luiz Augusto von Dentz @ 2013-01-18 9:08 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1358356034-2622-1-git-send-email-frederic.dalleau@linux.intel.com>
Hi Frederic,
On Wed, Jan 16, 2013 at 7:07 PM, Frédéric Dalleau
<frederic.dalleau@linux.intel.com> wrote:
> With several adapters, MediaEndpoint custom property is only created for the
> first adapter. When a new connection happens on another adapter
> btd_profile_custom_property_exists fails because the custom property doesn't
> exist for this last adapter. In case of HFP ofono fails with error: "Media
> Endpoint missing error".
>
> This patch allows the creation of several custom property with same name that
> can be distinguished based on a 'key'. When querying the property, additional
> checking must happen. For example endpoint_properties_exists, compares
> btd_adapter.
> 'NULL' key maintains existing behavior.
> ---
> profiles/audio/media.c | 5 +++--
> src/profile.c | 22 ++++++++++++++++------
> src/profile.h | 6 ++++--
> 3 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index 4b91656..60a5da5 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -195,7 +195,7 @@ static void media_endpoint_remove(struct media_endpoint *endpoint)
>
> if (media_adapter_find_endpoint(adapter, NULL, NULL,
> endpoint->uuid) == NULL)
> - btd_profile_remove_custom_prop(endpoint->uuid,
> + btd_profile_remove_custom_prop(adapter, endpoint->uuid,
> "MediaEndpoints");
>
> media_endpoint_destroy(endpoint);
> @@ -740,7 +740,8 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
> endpoint, NULL);
>
> if (media_adapter_find_endpoint(adapter, NULL, NULL, uuid) == NULL) {
> - btd_profile_add_custom_prop(uuid, "a{sv}", "MediaEndpoints",
> + btd_profile_add_custom_prop(adapter, uuid, "a{sv}",
> + "MediaEndpoints",
> endpoint_properties_exists,
> endpoint_properties_get,
> adapter);
> diff --git a/src/profile.c b/src/profile.c
> index b0c8500..7d48cfc 100644
> --- a/src/profile.c
> +++ b/src/profile.c
> @@ -572,6 +572,7 @@ struct ext_record {
> };
>
> struct btd_profile_custom_property {
> + const void *key;
> char *uuid;
> char *type;
> char *name;
> @@ -2208,7 +2209,8 @@ static const GDBusMethodTable methods[] = {
> { }
> };
>
> -static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
> +static struct btd_profile_custom_property *find_custom_prop(const char *key,
> + const char *uuid,
> const char *name)
> {
> GSList *l;
> @@ -2216,6 +2218,9 @@ static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
> for (l = custom_props; l; l = l->next) {
> struct btd_profile_custom_property *prop = l->data;
>
> + if (prop->key != key)
> + continue;
> +
> if (strcasecmp(prop->uuid, uuid) != 0)
> continue;
>
> @@ -2226,7 +2231,8 @@ static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
> return NULL;
> }
>
> -bool btd_profile_add_custom_prop(const char *uuid, const char *type,
> +bool btd_profile_add_custom_prop(const void *key, const char *uuid,
> + const char *type,
> const char *name,
> btd_profile_prop_exists exists,
> btd_profile_prop_get get,
> @@ -2234,12 +2240,15 @@ bool btd_profile_add_custom_prop(const char *uuid, const char *type,
> {
> struct btd_profile_custom_property *prop;
>
> - prop = find_custom_prop(uuid, name);
> - if (prop != NULL)
> + prop = find_custom_prop(key, uuid, name);
> + if (prop != NULL) {
> + DBG("Custom property %s already exists", name);
> return false;
> + }
>
> prop = g_new0(struct btd_profile_custom_property, 1);
>
> + prop->key = key;
> prop->uuid = g_strdup(uuid);
> prop->type = g_strdup(type);
> prop->name = g_strdup(name);
> @@ -2263,11 +2272,12 @@ static void free_property(gpointer data)
> g_free(p);
> }
>
> -bool btd_profile_remove_custom_prop(const char *uuid, const char *name)
> +bool btd_profile_remove_custom_prop(const void *key, const char *uuid,
> + const char *name)
> {
> struct btd_profile_custom_property *prop;
>
> - prop = find_custom_prop(uuid, name);
> + prop = find_custom_prop(key, uuid, name);
> if (prop == NULL)
> return false;
>
> diff --git a/src/profile.h b/src/profile.h
> index d858925..0764f6f 100644
> --- a/src/profile.h
> +++ b/src/profile.h
> @@ -67,12 +67,14 @@ typedef bool (*btd_profile_prop_get)(const char *uuid,
> DBusMessageIter *iter,
> void *user_data);
>
> -bool btd_profile_add_custom_prop(const char *uuid, const char *type,
> +bool btd_profile_add_custom_prop(const void *key, const char *uuid,
> + const char *type,
> const char *name,
> btd_profile_prop_exists exists,
> btd_profile_prop_get get,
> void *user_data);
> -bool btd_profile_remove_custom_prop(const char *uuid, const char *name);
> +bool btd_profile_remove_custom_prop(const void *key, const char *uuid,
> + const char *name);
>
> void btd_profile_init(void);
> void btd_profile_cleanup(void);
> --
> 1.7.9.5
Im afraid you are trying to fix in the wrong place, indeed there is a
problem when a custom property is already registered it cannot
register another time for a different adapter, but what we should be
doing is checking the list of adapters in endpoint_properties_exists,
you just have to find by btd_adapter instead of using user_data for
that, in fact the user_data should be probably NULL as we have the
list of adapters global in media.c
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH] bluetooth: btmrvl_sdio: look for sd8688 firmware in proper location
From: Marcel Holtmann @ 2013-01-18 7:58 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Bing Zhao, Ben Hutchings, David Woodhouse, Gustavo Padovan,
Johan Hedberg, libertas-dev, linux-bluetooth, linux-kernel
In-Reply-To: <1358494643-29765-1-git-send-email-lkundrak@v3.sk>
Hi Lubumir,
proper commit message with explanation here please.
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> drivers/bluetooth/btmrvl_sdio.c | 8 ++++----
> drivers/bluetooth/btmrvl_sdio.h | 6 ++++--
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
> index 3f4bfc8..bc27d01 100644
> --- a/drivers/bluetooth/btmrvl_sdio.c
> +++ b/drivers/bluetooth/btmrvl_sdio.c
> @@ -83,8 +83,8 @@ static const struct btmrvl_sdio_card_reg btmrvl_reg_87xx = {
> };
>
> static const struct btmrvl_sdio_device btmrvl_sdio_sd8688 = {
> - .helper = "sd8688_helper.bin",
> - .firmware = "sd8688.bin",
> + .helper = "mrvl/sd8688_helper.bin",
> + .firmware = "mrvl/sd8688.bin",
> .reg = &btmrvl_reg_8688,
> .sd_blksz_fw_dl = 64,
> };
> @@ -1179,7 +1179,7 @@ MODULE_AUTHOR("Marvell International Ltd.");
> MODULE_DESCRIPTION("Marvell BT-over-SDIO driver ver " VERSION);
> MODULE_VERSION(VERSION);
> MODULE_LICENSE("GPL v2");
> -MODULE_FIRMWARE("sd8688_helper.bin");
> -MODULE_FIRMWARE("sd8688.bin");
> +MODULE_FIRMWARE("mrvl/sd8688_helper.bin");
> +MODULE_FIRMWARE("mrvl/sd8688.bin");
> MODULE_FIRMWARE("mrvl/sd8787_uapsta.bin");
> MODULE_FIRMWARE("mrvl/sd8797_uapsta.bin");
> diff --git a/drivers/bluetooth/btmrvl_sdio.h b/drivers/bluetooth/btmrvl_sdio.h
> index 43d35a6..4a5a019 100644
> --- a/drivers/bluetooth/btmrvl_sdio.h
> +++ b/drivers/bluetooth/btmrvl_sdio.h
> @@ -84,7 +84,9 @@ struct btmrvl_sdio_card {
> struct sdio_func *func;
> u32 ioport;
> const char *helper;
> + const char *helper2;
> const char *firmware;
> + const char *firmware2;
And please clear out the patch from left-overs.
> const struct btmrvl_sdio_card_reg *reg;
> u16 sd_blksz_fw_dl;
> u8 rx_unit;
> @@ -92,8 +94,8 @@ struct btmrvl_sdio_card {
> };
>
> struct btmrvl_sdio_device {
> - const char *helper;
> - const char *firmware;
> + const char *helper, *helper2;
> + const char *firmware, *firmware2;
And here as well.
Regards
Marcel
^ permalink raw reply
* [PATCH] libertas sdio: look for 8688 firmware in common location
From: Lubomir Rintel @ 2013-01-18 7:39 UTC (permalink / raw)
To: Dan Williams
Cc: Marcel Holtmann, Bing Zhao, Ben Hutchings, David Woodhouse,
Gustavo Padovan, Johan Hedberg, libertas-dev, linux-bluetooth,
linux-kernel, Lubomir Rintel
In-Reply-To: <1357771504.12030.59.camel@dcbw.foobar.com>
sd8688 is not only used by libertas WiFi, but shared with btmrvl bluetooth as
well.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/net/wireless/libertas/if_sdio.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 739309e..be16a76 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -85,6 +85,7 @@ static const struct lbs_fw_table fw_table[] = {
{ MODEL_8686, "libertas/sd8686_v9_helper.bin", "libertas/sd8686_v9.bin" },
{ MODEL_8686, "libertas/sd8686_v8_helper.bin", "libertas/sd8686_v8.bin" },
{ MODEL_8686, "sd8686_helper.bin", "sd8686.bin" },
+ { MODEL_8688, "mrvl/sd8688_helper.bin", "mrvl/sd8688.bin" },
{ MODEL_8688, "libertas/sd8688_helper.bin", "libertas/sd8688.bin" },
{ MODEL_8688, "sd8688_helper.bin", "sd8688.bin" },
{ 0, NULL, NULL }
@@ -99,6 +100,8 @@ MODULE_FIRMWARE("libertas/sd8686_v8_helper.bin");
MODULE_FIRMWARE("libertas/sd8686_v8.bin");
MODULE_FIRMWARE("sd8686_helper.bin");
MODULE_FIRMWARE("sd8686.bin");
+MODULE_FIRMWARE("mrvl/sd8688_helper.bin");
+MODULE_FIRMWARE("mrvl/sd8688.bin");
MODULE_FIRMWARE("libertas/sd8688_helper.bin");
MODULE_FIRMWARE("libertas/sd8688.bin");
MODULE_FIRMWARE("sd8688_helper.bin");
--
1.7.1
^ permalink raw reply related
* [PATCH] bluetooth: btmrvl_sdio: look for sd8688 firmware in proper location
From: Lubomir Rintel @ 2013-01-18 7:37 UTC (permalink / raw)
To: Marcel Holtmann, Bing Zhao
Cc: Ben Hutchings, David Woodhouse, Gustavo Padovan, Johan Hedberg,
libertas-dev, linux-bluetooth, linux-kernel, Lubomir Rintel
In-Reply-To: <477F20668A386D41ADCC57781B1F70430D13FDB783@SC-VEXCH1.marvell.com>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/bluetooth/btmrvl_sdio.c | 8 ++++----
drivers/bluetooth/btmrvl_sdio.h | 6 ++++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 3f4bfc8..bc27d01 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -83,8 +83,8 @@ static const struct btmrvl_sdio_card_reg btmrvl_reg_87xx = {
};
static const struct btmrvl_sdio_device btmrvl_sdio_sd8688 = {
- .helper = "sd8688_helper.bin",
- .firmware = "sd8688.bin",
+ .helper = "mrvl/sd8688_helper.bin",
+ .firmware = "mrvl/sd8688.bin",
.reg = &btmrvl_reg_8688,
.sd_blksz_fw_dl = 64,
};
@@ -1179,7 +1179,7 @@ MODULE_AUTHOR("Marvell International Ltd.");
MODULE_DESCRIPTION("Marvell BT-over-SDIO driver ver " VERSION);
MODULE_VERSION(VERSION);
MODULE_LICENSE("GPL v2");
-MODULE_FIRMWARE("sd8688_helper.bin");
-MODULE_FIRMWARE("sd8688.bin");
+MODULE_FIRMWARE("mrvl/sd8688_helper.bin");
+MODULE_FIRMWARE("mrvl/sd8688.bin");
MODULE_FIRMWARE("mrvl/sd8787_uapsta.bin");
MODULE_FIRMWARE("mrvl/sd8797_uapsta.bin");
diff --git a/drivers/bluetooth/btmrvl_sdio.h b/drivers/bluetooth/btmrvl_sdio.h
index 43d35a6..4a5a019 100644
--- a/drivers/bluetooth/btmrvl_sdio.h
+++ b/drivers/bluetooth/btmrvl_sdio.h
@@ -84,7 +84,9 @@ struct btmrvl_sdio_card {
struct sdio_func *func;
u32 ioport;
const char *helper;
+ const char *helper2;
const char *firmware;
+ const char *firmware2;
const struct btmrvl_sdio_card_reg *reg;
u16 sd_blksz_fw_dl;
u8 rx_unit;
@@ -92,8 +94,8 @@ struct btmrvl_sdio_card {
};
struct btmrvl_sdio_device {
- const char *helper;
- const char *firmware;
+ const char *helper, *helper2;
+ const char *firmware, *firmware2;
const struct btmrvl_sdio_card_reg *reg;
u16 sd_blksz_fw_dl;
};
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] Bluetooth: btmrvl_sdio: look for sd8688 firmware in alternate place
From: Lubomir Rintel @ 2013-01-18 7:33 UTC (permalink / raw)
To: Marcel Holtmann, David Woodhouse, Ben Hutchings
Cc: Bing Zhao, libertas-dev@lists.infradead.org,
linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
linux-kernel@vger.kernel.org
In-Reply-To: <1357713345.1806.44.camel@aeonflux>
On Tue, 2013-01-08 at 22:35 -0800, Marcel Holtmann wrote:
> Hi Lubomir,
>
> > > > > linux-firmware ships the sd8688* firmware images that are shared with
> > > > > libertas_sdio WiFi driver under libertas/. libertas_sdio looks in both places
> > > > > and so should we.
> > > > >
> > > > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > > > ---
> > > > > drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++--
> > > > > drivers/bluetooth/btmrvl_sdio.h | 6 ++++--
> > > > > 2 files changed, 26 insertions(+), 4 deletions(-)
> > > >
> > > > NAK from me on this one. I do not want the driver to check two
> > > > locations. That is what userspace can work around.
> > > >
> > > > If we want to unify the location between the WiFi driver and the
> > > > Bluetooth driver, I am fine with that, but seriously, just pick one over
> > > > the other. I do not care which one.
> > >
> > > The unified location is mrvl/ directory.
> > >
> > > We can probably move SD8688 firmware & helper binaries to mrvl/ and have both drivers grab the images there?
> >
> > That would break existing setups, wouldn't it?
> >
> > I was under impression (commit 3d32a58b) that we care about
> > compatibility here. Do we?
>
> that is what symlinks are for.
David, Ben: please pull the following branch then:
git pull git://github.com/lkundrak/linux-firmware.git sd8688-move
Thank you!
^ permalink raw reply
* Re: [PATCH 3/4] Bluetooth: Fix using system-global workqueue when not necessary
From: Gustavo Padovan @ 2013-01-18 5:07 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1358195633-29303-4-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
* Johan Hedberg <johan.hedberg@gmail.com> [2013-01-14 22:33:52 +0200]:
> From: Johan Hedberg <johan.hedberg@intel.com>
>
> There's a per-HCI device workqueue (hdev->workqueue) that should be used
> for general per-HCI device work (except hdev->req_workqueue that's for
> hci_request() related work). This patch fixes places using the
> system-global work queue and makes them use the hdev->workqueue instead.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_core.c | 4 ++--
> net/bluetooth/mgmt.c | 3 ++-
> 2 files changed, 4 insertions(+), 3 deletions(-)
Patches 1 to 3 have been applied to bluetooth-next. Thanks.
Gustavo
^ permalink raw reply
* Thoughts about LE scanning
From: Marcel Holtmann @ 2013-01-17 22:02 UTC (permalink / raw)
To: linux-bluetooth
Hello,
so I have been looking into how we handle LE scanning in central role a
little bit. And I am not really happy with what we do right now. Instead
of just adding new profiles, we need to take one step back and get the
basics right.
The one thing that I stumbled about is that we are trying to use the
Start Discovery management command to scan for connectable device that
we are paired with. And it is all triggered by bluetoothd. It tries to
get this done right, but fails badly at it. Trying to do this ends up in
a convoluted solution that will just break down eventually.
So Start Discovery and Stop Discovery management commands are for
finding new devices. They are for finding devices that we want to pair
with. They are not for checking if a paired device is in range or
signals connection requests to us. It is called discovery for a reason.
It might have looked like a good idea to just re-use these two commands,
but what I am seeing when working with multiple paired LE devices is
just a big mess. The amount of code that is needed to keep track of
states between running D-Bus commands for discovery, discovery triggered
management commands, scanning triggered management commands etc. is not
a good idea.
I am failing to understand why we tried to solve this inside bluetoothd
and not just let the kernel take full control here. We are loading the
list of paired LE devices at controller power on anyway. So the kernel
does know about our paired devices. It can control sensible scan
intervals and also sync a device discovery with scanning for connection
triggers from know remote devices. It also can sensible disconnect on
idle and scan for other devices that requests connections.
What I think we should have is a management command that allows us to
load device specific scan parameter configuration into the kernel. And
then the kernel will execute this on our behalf. We need to decouple the
commands for device discovery from the commands that scan for known
devices.
Seems also we should actually implement the scan parameters service as a
core feature of bluetoothd and not just a plugin. For me it looks like
it essential that we allow different behavior for different devices.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/5] Bluetooth: Add option for SCO socket mode
From: Frédéric Dalleau @ 2013-01-17 15:36 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Frédéric Dalleau, linux-bluetooth
In-Reply-To: <1358431772.3059.13.camel@aeonflux>
On Thu, Jan 17, 2013 at 06:09:32AM -0800, Marcel Holtmann wrote:
Hi Marcel,
> I do not like this enhanced magic. How is this suppose to work? And it
> is also not explained in the commit message. Or used in this patch.
sco_options_enhanced is not definitive, this patch is meant to describe
different parameters for the socket option that could be selected based on the
mode field. To ensure compatibility accross versions, the size of the socket
options structure can be checked.
The idea is to read supported codecs list using mgmt API. Then,it is possible
to select some codecs and the enhanced accept/setup sync conn are sent.
It does not make sense to get this option since the codec is negociated
externally.
If this mode is removed, we fallback on version 1 of RFC (except at this time,
sco_options.mode was sco_options.codec). And it is it is easy to add new modes
to select latency, or packet types without breaking existing API.
Thanks,
Frédéric
^ permalink raw reply
* Re: [PATCH 4/5] Bluetooth: Parameters for outgoing SCO connections
From: Marcel Holtmann @ 2013-01-17 14:17 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1358426389-25903-5-git-send-email-frederic.dalleau@linux.intel.com>
Hi Fred,
> In order to establish a transparent SCO connection, the correct settings must
> be specified in the SetupSynchronousConnection request. For that, a bit is set
> in ACL connection flags to set up the desired parameters. If this bit is not
> set, a legacy SCO connection will be requested.
> This patch uses T2 settings.
> ---
> include/net/bluetooth/hci_core.h | 3 +++
> net/bluetooth/hci_conn.c | 27 ++++++++++++++++++++-------
> net/bluetooth/sco.c | 4 ++--
> 3 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index cb5d131..9ef7fe0 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -438,6 +438,7 @@ enum {
> HCI_CONN_SSP_ENABLED,
> HCI_CONN_POWER_SAVE,
> HCI_CONN_REMOTE_OOB,
> + HCI_CONN_SCO_TRANSPARENT,
> };
>
> static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
> @@ -586,6 +587,8 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle);
>
> struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
> __u8 dst_type, __u8 sec_level, __u8 auth_type);
> +struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
> + u8 sec_level, u8 auth_type, u8 codec);
> int hci_conn_check_link_mode(struct hci_conn *conn);
> int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
> int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 25bfce0..e3f1fad 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -175,13 +175,22 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
> conn->attempt++;
>
> cp.handle = cpu_to_le16(handle);
> - 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.voice_setting = cpu_to_le16(hdev->voice_setting);
> - cp.retrans_effort = 0xff;
> +
> + if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
> + cp.pkt_type = __constant_cpu_to_le16(EDR_ESCO_MASK &
> + ~ESCO_2EV3);
> + cp.max_latency = __constant_cpu_to_le16(0x000d);
> + cp.voice_setting = __constant_cpu_to_le16(0x0003);
> + cp.retrans_effort = 0x02;
> + } else {
> + cp.pkt_type = cpu_to_le16(conn->pkt_type);
> + cp.max_latency = __constant_cpu_to_le16(0xffff);
> + cp.voice_setting = cpu_to_le16(hdev->voice_setting);
> + cp.retrans_effort = 0xff;
> + }
>
> hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
> }
> @@ -551,8 +560,9 @@ static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
> return acl;
> }
>
> -static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
> - bdaddr_t *dst, u8 sec_level, u8 auth_type)
> +struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
> + bdaddr_t *dst, u8 sec_level,
> + u8 auth_type, u8 codec)
> {
> struct hci_conn *acl;
> struct hci_conn *sco;
> @@ -575,6 +585,9 @@ static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
>
> hci_conn_hold(sco);
>
> + if (codec)
> + set_bit(HCI_CONN_SCO_TRANSPARENT, &sco->flags);
> +
> if (acl->state == BT_CONNECTED &&
> (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
> set_bit(HCI_CONN_POWER_SAVE, &acl->flags);
> @@ -605,7 +618,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
> return hci_connect_acl(hdev, dst, sec_level, auth_type);
> case SCO_LINK:
> case ESCO_LINK:
> - return hci_connect_sco(hdev, type, dst, sec_level, auth_type);
> + return hci_connect_sco(hdev, type, dst, sec_level, auth_type, 0);
> }
>
> return ERR_PTR(-EINVAL);
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 6a957a3..bee0b5c 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -176,8 +176,8 @@ static int sco_connect(struct sock *sk)
> else
> type = SCO_LINK;
>
> - hcon = hci_connect(hdev, type, dst, BDADDR_BREDR, BT_SECURITY_LOW,
> - HCI_AT_NO_BONDING);
> + hcon = hci_connect_sco(hdev, type, dst, BT_SECURITY_LOW,
> + HCI_AT_NO_BONDING, sco_pi(sk)->mode);
if we start doing this, then the security requirements for SCO channels
are pointless. They do not exists.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/5] Bluetooth: Use mode when accepting SCO connection
From: Marcel Holtmann @ 2013-01-17 14:15 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1358426389-25903-4-git-send-email-frederic.dalleau@linux.intel.com>
Hi Fred,
> When an incoming SCO connection is requested, check the selected mode, and
> reply appropriately. Mode should have been negotiated previously. For example,
> in case of HFP, the codec is negotiated using AT commands on the RFCOMM
> channel. This patch only changes replies for socket with defered setup enabled.
> ---
> include/net/bluetooth/hci_core.h | 2 +-
> net/bluetooth/hci_event.c | 21 +++++++++++++++++----
> net/bluetooth/sco.c | 2 +-
> 3 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 014a2ea..cb5d131 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -577,7 +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);
> +void hci_conn_accept(struct hci_conn *conn, int mask, int mode);
>
> struct hci_chan *hci_chan_create(struct hci_conn *conn);
> void hci_chan_del(struct hci_chan *chan);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 705078a..afa0104 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2047,7 +2047,7 @@ unlock:
> hci_conn_check_pending(hdev);
> }
>
> -void hci_conn_accept(struct hci_conn *conn, int mask)
> +void hci_conn_accept(struct hci_conn *conn, int mask, int mode)
> {
> struct hci_dev *hdev = conn->hdev;
>
> @@ -2074,9 +2074,22 @@ void hci_conn_accept(struct hci_conn *conn, int mask)
>
> 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;
> +
> + switch (mode) {
> + case 0:
> + cp.max_latency = __constant_cpu_to_le16(0xffff);
> + cp.content_format = cpu_to_le16(hdev->voice_setting);
> + cp.retrans_effort = 0xff;
> + break;
> + case 1:
> + if (conn->pkt_type & ESCO_2EV3)
> + cp.max_latency = __constant_cpu_to_le16(0x0008);
> + else
> + cp.max_latency = __constant_cpu_to_le16(0x000D);
> + cp.content_format = __constant_cpu_to_le16(0x0003);
> + cp.retrans_effort = 0x02;
> + break;
> + }
so what happens if someone sets mode == 0x02, then we just send some
random data. This reminds me, we need to do range checks for the
setsockopt call. Only valid modes are suppose to be allowed.
>
> hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
> sizeof(cp), &cp);
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 22ad5fa..6a957a3 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -666,7 +666,7 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>
> if (sk->sk_state == BT_CONNECT2 &&
> test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
> - hci_conn_accept(pi->conn->hcon, 0);
> + hci_conn_accept(pi->conn->hcon, 0, pi->mode);
> sk->sk_state = BT_CONFIG;
>
> release_sock(sk);
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/5] Bluetooth: Add setsockopt for SCO socket mode
From: Marcel Holtmann @ 2013-01-17 14:12 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1358426389-25903-3-git-send-email-frederic.dalleau@linux.intel.com>
Hi Fred,
> This patch implements setsockopt().
not acceptable commit message.
> ---
> net/bluetooth/sco.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index bdb21b2..22ad5fa 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -678,6 +678,47 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
> return bt_sock_recvmsg(iocb, sock, msg, len, flags);
> }
>
> +static int sco_sock_setsockopt_old(struct socket *sock, int optname,
> + char __user *optval, unsigned int optlen)
> +{
> + struct sock *sk = sock->sk;
> + struct sco_options opts;
> + int len, err = 0;
> +
> + BT_DBG("sk %p", sk);
> +
> + lock_sock(sk);
> +
> + switch (optname) {
> + case SCO_OPTIONS:
> + if (sk->sk_state != BT_OPEN &&
> + sk->sk_state != BT_BOUND &&
> + sk->sk_state != BT_CONNECT2) {
> + err = -EINVAL;
> + break;
> + }
> +
> + opts.mode = SCO_MODE_CVSD;
Don't we need to set a opts.mtu here as well? This is user input. So we
need to be 100% sure to verify it.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/5] Bluetooth: Add option for SCO socket mode
From: Marcel Holtmann @ 2013-01-17 14:09 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1358426389-25903-2-git-send-email-frederic.dalleau@linux.intel.com>
Hi Fred,
> This patch extends the current SCO socket option to add a 'mode' field. This
> field is intended to choose data type at runtime. Current modes are CVSD and
> transparent SCO, but adding new modes could allow support for CSA2 and fine
> tuning a sco connection, for example latency, bandwith, voice setting. Incoming
> connections will be setup during defered setup. Outgoing connections have to
> be setup before connect(). The selected type is stored in the sco socket info.
> This patch declares needed members and implements getsockopt().
> ---
> include/net/bluetooth/sco.h | 20 ++++++++++++++++++++
> net/bluetooth/sco.c | 3 +++
> 2 files changed, 23 insertions(+)
>
> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
> index 1e35c43..bc5d3d6 100644
> --- a/include/net/bluetooth/sco.h
> +++ b/include/net/bluetooth/sco.h
> @@ -41,8 +41,27 @@ struct sockaddr_sco {
>
> /* SCO socket options */
> #define SCO_OPTIONS 0x01
> +
> +#define SCO_MODE_CVSD 0x00
> +#define SCO_MODE_TRANSPARENT 0x01
> +#define SCO_MODE_ENHANCED 0x02
> +
I do not like this enhanced magic. How is this suppose to work? And it
is also not explained in the commit message. Or used in this patch.
Regards
Marcel
^ permalink raw reply
* [PATCH 5/5] Bluetooth: Fallback transparent SCO from T2 to T1
From: Frédéric Dalleau @ 2013-01-17 12:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1358426389-25903-1-git-send-email-frederic.dalleau@linux.intel.com>
When initiating a transparent eSCO connection, make use of T2 settings at
first try. T2 is the recommended settings from HFP 1.6 WideBand Speech. Upon
connection failure, try T1 settings.
To know which of T2 or T1 should be used, the connection attempt index is used.
T2 failure is detected if Synchronous Connection Complete Event fails with
error 0x0d. This error code has been found experimentally by sending a T2
request to a T1 only SCO listener. It means "Connection Rejected due to
Limited resource".
---
net/bluetooth/hci_conn.c | 11 ++++++++++-
net/bluetooth/hci_event.c | 1 +
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e3f1fad..d08f001 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -179,12 +179,21 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
cp.tx_bandwidth = __constant_cpu_to_le32(0x00001f40);
cp.rx_bandwidth = __constant_cpu_to_le32(0x00001f40);
- if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
+ if (conn->attempt == 1 &&
+ test_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
cp.pkt_type = __constant_cpu_to_le16(EDR_ESCO_MASK &
~ESCO_2EV3);
cp.max_latency = __constant_cpu_to_le16(0x000d);
cp.voice_setting = __constant_cpu_to_le16(0x0003);
cp.retrans_effort = 0x02;
+ } else if (conn->attempt == 2 &&
+ test_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
+ cp.pkt_type = __constant_cpu_to_le16(EDR_ESCO_MASK |
+ ESCO_EV3);
+ cp.max_latency = __constant_cpu_to_le16(0x0007);
+ cp.voice_setting = __constant_cpu_to_le16(0x0003);
+ cp.retrans_effort = 0x02;
+ clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags);
} else {
cp.pkt_type = cpu_to_le16(conn->pkt_type);
cp.max_latency = __constant_cpu_to_le16(0xffff);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index afa0104..ba4af46 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3319,6 +3319,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
hci_conn_add_sysfs(conn);
break;
+ case 0x0d: /* No resource available */
case 0x11: /* Unsupported Feature or Parameter Value */
case 0x1c: /* SCO interval rejected */
case 0x1a: /* Unsupported Remote Feature */
--
1.7.9.5
^ 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