[PATCH] Fix unpair device when disconnected for No Bonding

[PATCH] Fix unpair device when disconnected for No Bonding

[Dmitriy Paliy]

Fix Paired property of device to be false if 'No Bonding' authentication
is used. This property is set to 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 |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index ae6a0d5..e886448 100644
--- a/src/device.c
+++ b/src/device.c
@@ -830,6 +830,14 @@ void device_remove_connection(struct btd_device *device, DBusConnection *conn)
 		device->disconnects = g_slist_remove(device->disconnects, msg);
 	}
 
+	if (device_is_paired(device)) {
+		bdaddr_t src;
+
+		adapter_get_address(device->adapter, &src);
+		if (read_link_key(&src, &device->bdaddr, NULL, NULL))
+			device_set_paired(device, FALSE);
+	}
+
 	emit_property_changed(conn, device->path,
 					DEVICE_INTERFACE, "Connected",
 					DBUS_TYPE_BOOLEAN, &device->connected);
-- 
1.7.4.1

Re: [PATCH] Fix unpair device when disconnected for No Bonding

[Johan Hedberg]

Hi Dmitriy,

On Mon, Jun 20, 2011, Dmitriy Paliy wrote:
> Fix Paired property of device to be false if 'No Bonding' authentication
> is used. This property is set to 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 |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/src/device.c b/src/device.c
> index ae6a0d5..e886448 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -830,6 +830,14 @@ void device_remove_connection(struct btd_device *device, DBusConnection *conn)
>  		device->disconnects = g_slist_remove(device->disconnects, msg);
>  	}
>  
> +	if (device_is_paired(device)) {
> +		bdaddr_t src;
> +
> +		adapter_get_address(device->adapter, &src);
> +		if (read_link_key(&src, &device->bdaddr, NULL, NULL))
> +			device_set_paired(device, FALSE);
> +	}

Coding-style wise your read_link_key call should be checking for < 0 or
specifically for -ENOENT (you're calling the function as if it returned
a boolean value). However, I'm thinking we might just store the
no-bonding info in the device struct itself so you don't need to consult
the storage in this case. I.e. in addition to device_is_paired there'd
also be a device_is_bonded function which'd make the logic like:

	if (device_is_paired(device) && !device_is_bonded(device))
		device_set_paired(device, FALSE);

Thoughts? Have you checked that this works both with hciops and mgmtops?

Johan

Re: [PATCH] Fix unpair device when disconnected for No Bonding

[Dmitriy Paliy]

Hi Johan,

On Mon, Jun 20, 2011 at 12:31 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Coding-style wise your read_link_key call should be checking for < 0 or
> specifically for -ENOENT (you're calling the function as if it returned
> a boolean value). However, I'm thinking we might just store the
> no-bonding info in the device struct itself so you don't need to consult
> the storage in this case. I.e. in addition to device_is_paired there'd
> also be a device_is_bonded function which'd make the logic like:
>
>        if (device_is_paired(device) && !device_is_bonded(device))
>                device_set_paired(device, FALSE);
>
> Thoughts? Have you checked that this works both with hciops and mgmtops?

New ..is_bonded sounds a bit redundant since it is quite
straightforward to check it
from storage, but also ok.

I didnt touch mgmtops at all. Could you provide more inputs about what
exactly the
issue is?

BR,
Dmitriy

Re: [PATCH] Fix unpair device when disconnected for No Bonding

[Johan Hedberg]

Hi Dmitriy,

On Mon, Jun 20, 2011, Dmitriy Paliy wrote:
> On Mon, Jun 20, 2011 at 12:31 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> > Coding-style wise your read_link_key call should be checking for < 0 or
> > specifically for -ENOENT (you're calling the function as if it returned
> > a boolean value). However, I'm thinking we might just store the
> > no-bonding info in the device struct itself so you don't need to consult
> > the storage in this case. I.e. in addition to device_is_paired there'd
> > also be a device_is_bonded function which'd make the logic like:
> >
> >        if (device_is_paired(device) && !device_is_bonded(device))
> >                device_set_paired(device, FALSE);
> >
> > Thoughts? Have you checked that this works both with hciops and mgmtops?
> 
> New ..is_bonded sounds a bit redundant since it is quite
> straightforward to check it from storage, but also ok.

It's still a file-system access for something that we could easily have
available as a flag in runtime memory. Even if you did do the
file-system read it'd make the code more readable if you had this
wrapped into a device_is_bonded function.

> I didnt touch mgmtops at all. Could you provide more inputs about what
> exactly the issue is?

I didn't say that there is an issue. I just asked if you've checked that
this works with mgmtops since the signaling between the kernel,
adapter_ops and the core bluetooth daemon is quite different wrt pairing
in that case.

Johan