Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Szymon & Gustavo,

On Wed, May 30, 2012, Szymon Janc wrote:
> > > Some devices e.g. SonyEricsson Xperia ray and arc S don't do SDP search
> > > before pairing. No L2CAP is connected so default HCI_DISCONN_TIMEOUT
> > > (2 seconds) timeout value is being used. This results in problems with
> > > legacy pairing as remote user has only few seconds to enter PIN before
> > > ACL is disconnected.
> > > 
> > > Increase disconnect timeout to HCI_PAIRING_TIMEOUT if SSP is disabled
> > > and no linkey exists.
> > 
> > does this only happen with SSP disabled?
> 
> Yes, I've seen this only with SSP disabled.
> After name request remote devices ask user to enter PIN and before user is
> able to do so (4 sec only) link is disconnected (cause we didn't get pin
> request event yet).
> 
> For SSP hci_conn_hold is called in hci_io_capa_request_evt so link is not
> disconnected.
> 
> > 
> > Also add the hcidump output to the commit message would be helpful.
> 
> hcidump for ssp disabled [1] and ssp enabled [2] below.
> 
> > > 
> > > Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> > > ---
> > >  net/bluetooth/hci_event.c |    7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > index ac86b65..98e8020 100644
> > > --- a/net/bluetooth/hci_event.c
> > > +++ b/net/bluetooth/hci_event.c
> > > @@ -1762,7 +1762,12 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> > >  		if (conn->type == ACL_LINK) {
> > >  			conn->state = BT_CONFIG;
> > >  			hci_conn_hold(conn);
> > > -			conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> > > +
> > > +			if (!hci_conn_ssp_enabled(conn) &&
> > > +			    !hci_find_link_key(hdev, &ev->bdaddr))
> 
> I could add also checking if connection is incoming to narrow case a bit further...
> 
> > > +				conn->disc_timeout = HCI_PAIRING_TIMEOUT;
> > > +			else
> > > +				conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> > 
> > You are also changing the timeout for a SDP search, for example, to
> > HCI_PAIRING_TIMEOUT and this is not good. I think we need to be smarter here,
> > we can't change the behaviour of things like SDP search here.
> 
> Maybe we could set timeout back to HCI_DICONN_TIMEOUT when l2cap is
> connected (or disconnected)?  That should cover SDP search case..

What happened to getting this patch upstream? To me it looks like a
definitely needed fix. After adding the fix to restore a sensible value
for disc_timeout after an L2CAP connect request either way and adding a
better explanation to the commit message (that we only get the PIN
request after user has entered one on the remote side, including a
hcidump of this) I think this should go upstream. If this had been
processed in a timely manner it could have made it to 3.5 but now it
seems too late for that (as it's not strictly speaking a regression from
3.4).

Johan