Re: [PATCH v4] Bluetooth: Override status if local user rejects pairing

[Johan Hedberg <johan.hedberg@xxxxxxxxx>]

Hi Jaganath,

On Sun, Jul 15, 2012, Johan Hedberg wrote:
> On Fri, Jul 13, 2012, Jaganath Kanakkassery wrote:
> > --- a/include/net/bluetooth/mgmt.h
> > +++ b/include/net/bluetooth/mgmt.h
> > @@ -42,6 +42,7 @@
> >  #define MGMT_STATUS_NOT_POWERED		0x0f
> >  #define MGMT_STATUS_CANCELLED		0x10
> >  #define MGMT_STATUS_INVALID_INDEX	0x11
> > +#define MGMT_STATUS_AUTH_REJECTED	0x12
> 
> Why this? You didn't have it in v3. The existing STATUS_REJECTED should
> be enough, right?
> 
> > @@ -2659,6 +2659,8 @@ static void hci_pin_code_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >  		else
> >  			secure = 0;
> >  
> > +		conn->auth_rejected = false;
> > +
> >  		mgmt_pin_code_request(hdev, &ev->bdaddr, secure);
> >  	}
> >  
> 
> I think it would be cleaner to set this to false immediately after the
> conn lookup in this function. Also, when HCI_PAIRABLE is not set you
> should set auth_rejected to true since that's also a local rejection
> (even though it's an automatic one).
> 
> > @@ -3139,6 +3141,8 @@ static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >  		else
> >  			cp.oob_data = 0x00;
> >  
> > +		conn->auth_rejected = false;
> > +
> >  		hci_send_cmd(hdev, HCI_OP_IO_CAPABILITY_REPLY,
> >  			     sizeof(cp), &cp);
> >  	} else {
> 
> Same here.

A couple more things:

You should also clear the variable in the link key request handler. If
we respond with a positive link key response but the remote side
responds with a negative one with SSP you will get a "PIN or Key
Missing" HCI error which must not be overwritten by the flag (wrongly)
being set.

Since we already have conn->flags let's add another one there instead of
adding a completely new bool struct member. I'd propose
HCI_CONN_AUTH_REJECTED.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html