2011-07-11 12:20:05

by Dmitriy Paliy

[permalink] [raw]
Subject: [PATCH BlueZ v3 0/1] Fix unpair device when disconnected for No Bonding

Hi,

This version includes the fix regarding comment of Johan. Function
device_set_bonded is created and invoked when link key is stored to
file system which covers both pairing as initiator or acceptor cases.

BR,
Dmitriy



2011-07-11 17:39:34

by Dmitriy Paliy

[permalink] [raw]
Subject: Re: [PATCH BlueZ v3] Fix unpair device when disconnected for No Bonding

Hi Johan,

On Mon, Jul 11, 2011 at 3:52 PM, Johan Hedberg <[email protected]> wrote:
> Applied, and I also pushed a cleanup patch on top of it because
> device_remove_bonding was essentially useless. One thing that I'm
> thinking of is maybe it'd be better to get rid of the paired and bonded
> booleans have a single enum for them since they are dependent on each
> other and one combination of boolean values (!paired && bonded) is not
> possible. Maybe something like the following inside struct btd_device:
>
> ? ? ? ?enum {
> ? ? ? ? ? ? ? ?DEVICE_PAIRING_STATUS_NONE,
> ? ? ? ? ? ? ? ?DEVICE_PAIRING_STATUS_PAIRED,
> ? ? ? ? ? ? ? ?DEVICE_PAIRING_STATUS_BONDED,
> ? ? ? ?} pairing_status;
>
> Thoughts?

Sounds wise, I will check solution for hci_ops and mgmt_ops.

BR,
Dmitriy

2011-07-11 12:52:54

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH BlueZ v3] Fix unpair device when disconnected for No Bonding

Hi Dmitriy,

On Mon, Jul 11, 2011, Dmitriy Paliy wrote:
> Fix Paired device property if 'No Bonding' authentication is used. It is
> set to be false when device is disconnected and no link key is stored.
> Otherwise, there can be cases when device is still valid and being
> claimed as paired without available bonding information.
>
> For instanse, use of CreateDevice method call and obex client file
> transfer is such use case.
> ---
> src/device.c | 19 ++++++++++++++++++-
> src/device.h | 1 +
> src/event.c | 8 ++++++--
> 3 files changed, 25 insertions(+), 3 deletions(-)

Applied, and I also pushed a cleanup patch on top of it because
device_remove_bonding was essentially useless. One thing that I'm
thinking of is maybe it'd be better to get rid of the paired and bonded
booleans have a single enum for them since they are dependent on each
other and one combination of boolean values (!paired && bonded) is not
possible. Maybe something like the following inside struct btd_device:

enum {
DEVICE_PAIRING_STATUS_NONE,
DEVICE_PAIRING_STATUS_PAIRED,
DEVICE_PAIRING_STATUS_BONDED,
} pairing_status;

Thoughts?

Johan

2011-07-11 12:20:06

by Dmitriy Paliy

[permalink] [raw]
Subject: [PATCH BlueZ v3] Fix unpair device when disconnected for No Bonding

Fix Paired device property if 'No Bonding' authentication is used. It is
set to be false when device is disconnected and no link key is stored.
Otherwise, there can be cases when device is still valid and being
claimed as paired without available bonding information.

For instanse, use of CreateDevice method call and obex client file
transfer is such use case.
---
src/device.c | 19 ++++++++++++++++++-
src/device.h | 1 +
src/event.c | 8 ++++++--
3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/device.c b/src/device.c
index 82759a4..0230203 100644
--- a/src/device.c
+++ b/src/device.c
@@ -132,6 +132,7 @@ struct btd_device {
gboolean trusted;
gboolean paired;
gboolean blocked;
+ gboolean bonded;

gboolean authorizing;
gint ref;
@@ -824,6 +825,9 @@ void device_remove_connection(struct btd_device *device, DBusConnection *conn)
device->disconnects = g_slist_remove(device->disconnects, msg);
}

+ if (device_is_paired(device) && !device->bonded)
+ device_set_paired(device, FALSE);
+
emit_property_changed(conn, device->path,
DEVICE_INTERFACE, "Connected",
DBUS_TYPE_BOOLEAN, &device->connected);
@@ -906,8 +910,10 @@ struct btd_device *device_create(DBusConnection *conn,
if (read_blocked(&src, &device->bdaddr))
device_block(conn, device);

- if (read_link_key(&src, &device->bdaddr, NULL, NULL) == 0)
+ if (read_link_key(&src, &device->bdaddr, NULL, NULL) == 0) {
device->paired = TRUE;
+ device_set_bonded(device, TRUE);
+ }

return btd_device_ref(device);
}
@@ -958,6 +964,7 @@ void device_remove_bonding(struct btd_device *device)

/* Delete the link key from storage */
textfile_casedel(filename, dstaddr);
+ device_set_bonded(device, FALSE);

create_name(filename, PATH_MAX, STORAGEDIR, srcaddr,
"aliases");
@@ -1792,6 +1799,16 @@ void device_set_temporary(struct btd_device *device, gboolean temporary)
device->temporary = temporary;
}

+void device_set_bonded(struct btd_device *device, gboolean bonded)
+{
+ if (!device)
+ return;
+
+ DBG("bonded %d", bonded);
+
+ device->bonded = bonded;
+}
+
void device_set_type(struct btd_device *device, device_type_t type)
{
if (!device)
diff --git a/src/device.h b/src/device.h
index bd8a431..18b21ae 100644
--- a/src/device.h
+++ b/src/device.h
@@ -72,6 +72,7 @@ gboolean device_is_trusted(struct btd_device *device);
void device_set_paired(struct btd_device *device, gboolean paired);
void device_set_temporary(struct btd_device *device, gboolean temporary);
void device_set_type(struct btd_device *device, device_type_t type);
+void device_set_bonded(struct btd_device *device, gboolean bonded);
gboolean device_is_connected(struct btd_device *device);
DBusMessage *device_create_bonding(struct btd_device *device,
DBusConnection *conn, DBusMessage *msg,
diff --git a/src/event.c b/src/event.c
index 86a413e..d5462f6 100644
--- a/src/event.c
+++ b/src/event.c
@@ -410,8 +410,12 @@ int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer,

ret = write_link_key(local, peer, key, key_type, pin_length);

- if (ret == 0 && device_is_temporary(device))
- device_set_temporary(device, FALSE);
+ if (ret == 0) {
+ device_set_bonded(device, TRUE);
+
+ if (device_is_temporary(device))
+ device_set_temporary(device, FALSE);
+ }

return ret;
}
--
1.7.4.1