Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [RFC v2 5/9] Bluetooth: Add support for using the crypto subsystem
From: Vinicius Costa Gomes @ 2010-12-07 17:51 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20101207172713.GF2944@vigoh>

Hi Gustavo,

On 15:27 Tue 07 Dec, Gustavo F. Padovan wrote:
> Hi Vinicius,
> 
> * Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2010-12-06 18:43:48 -0300]:
> 
> > This will allow using the crypto subsystem for encrypting data. As SMP
> > (Security Manager Protocol) is implemented almost entirely on the host
> > side and the crypto module already implements the needed methods
> > (AES-128), it makes sense to use it.
> > 
> > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> > ---
> >  include/net/bluetooth/hci_core.h |    2 ++
> >  net/bluetooth/hci_core.c         |   10 ++++++++++
> >  2 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index 0687e2f..d0a9f5d 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -135,6 +135,8 @@ struct hci_dev {
> >  	__u32			req_status;
> >  	__u32			req_result;
> >  
> > +	struct crypto_blkcipher	*tfm;
> > +
> >  	struct inquiry_cache	inq_cache;
> >  	struct hci_conn_hash	conn_hash;
> >  	struct list_head	blacklist;
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 12c6735..b96c3dd 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -41,6 +41,7 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/notifier.h>
> >  #include <linux/rfkill.h>
> > +#include <linux/crypto.h>
> >  #include <net/sock.h>
> >  
> >  #include <asm/system.h>
> > @@ -961,6 +962,13 @@ int hci_register_dev(struct hci_dev *hdev)
> >  	if (!hdev->workqueue)
> >  		goto nomem;
> >  
> > +	hdev->tfm = crypto_alloc_blkcipher("ecb(aes)", 0, CRYPTO_ALG_ASYNC);
> > +	if (IS_ERR(hdev->tfm)) {
> > +		BT_ERR("Failed to load transform for ecb(aes): %ld",
> > +							PTR_ERR(hdev->tfm));
> > +		goto nomem;
> 
> You are leaking hdev->workqueue here.

Thanks, see below.

> 
> Also you will need to add CRYPTO_BLKCIPHER dependence in the Kconfig.
> Maybe we should add a CONFIG_BLUETOOTH_SMP, and just build with blkcipher
> in the case SMP was selected to be built.

Sounds fair. Another alternative is: instead of not being able to register the 
HCI device if the blockcypher allocation fails, we could reply "Pairing Not
Supported" at the SMP level. We would just need to document somewhere that the
crypto subsystem and support for AES are needed for SMP to work.

What do you think?

> 
> -- 
> Gustavo F. Padovan
> http://profusion.mobi


Cheers,
-- 
Vinicius

^ permalink raw reply

* Re: [RFC v2 7/9] Bluetooth: Add support for SMP confirmation checks
From: Gustavo F. Padovan @ 2010-12-07 17:41 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1291671832-13435-8-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

* Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2010-12-06 18:43:50 -0300]:

> This adds supports for verifying the confirmation value that the
> remote side has sent. This includes support for generating and sending
> the random value used to produce the confirmation value.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Can this be split in more than one patch? I'm getting lost.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [RFC v2 8/9] Bluetooth: Add support for LE Start Encryption
From: Gustavo F. Padovan @ 2010-12-07 17:38 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1291671832-13435-9-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

* Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2010-12-06 18:43:51 -0300]:

> This adds support for starting SMP Phase 2 Encryption, when the initial
> SMP negotiation is successful. This adds the LE Start Encryption and LE
> Long Term Key Request commands and related events.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  include/net/bluetooth/hci.h      |   34 +++++++++++++++++++
>  include/net/bluetooth/hci_core.h |    5 +++
>  net/bluetooth/hci_conn.c         |   47 ++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        |   67 ++++++++++++++++++++++++++++++++++++++
>  net/bluetooth/smp.c              |    8 ++++-
>  5 files changed, 160 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index dff6ded..e6bed3f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -626,6 +626,33 @@ struct hci_cp_le_create_conn {
>  
>  #define HCI_OP_LE_CREATE_CONN_CANCEL	0x200e
>  
> +#define HCI_OP_LE_START_ENC		0x2019
> +struct hci_cp_le_start_enc {
> +	__le16	handle;
> +	__u8	rand[8];
> +	__le16	ediv;
> +	__u8	ltk[16];
> +} __packed;
> +
> +#define HCI_OP_LE_LTK_REPLY		0x201a
> +struct hci_cp_le_ltk_reply {
> +	__le16	handle;
> +	__u8	ltk[16];
> +} __packed;
> +struct hci_rp_le_ltk_reply {
> +	__u8	status;
> +	__le16	handle;
> +} __packed;
> +
> +#define HCI_OP_LE_LTK_NEG_REPLY		0x201b
> +struct hci_cp_le_ltk_neg_reply {
> +	__le16	handle;
> +} __packed;
> +struct hci_rp_le_ltk_neg_reply {
> +	__u8	status;
> +	__le16	handle;
> +} __packed;
> +
>  /* ---- HCI Events ---- */
>  #define HCI_EV_INQUIRY_COMPLETE		0x01
>  
> @@ -897,6 +924,13 @@ struct hci_ev_le_conn_complete {
>  	__u8     clk_accurancy;
>  } __packed;
>  
> +#define HCI_EV_LE_LTK_REQ		0x05
> +struct hci_ev_le_ltk_req {
> +	__le16	handle;
> +	__u8	random[8];
> +	__le16	ediv;
> +} __packed;
> +
>  /* Internal events generated by Bluetooth stack */
>  #define HCI_EV_STACK_INTERNAL	0xfd
>  struct hci_ev_stack_internal {
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index d0a9f5d..c6c44eb 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -192,6 +192,7 @@ struct hci_conn {
>  	__u8             sec_level;
>  	__u8             power_save;
>  	__u16            disc_timeout;
> +	__u8		 ltk[16];
>  	unsigned long	 pend;
>  
>  	unsigned int	 sent;
> @@ -713,4 +714,8 @@ struct hci_sec_filter {
>  
>  void hci_req_complete(struct hci_dev *hdev, int result);
>  
> +void hci_le_start_enc(struct hci_conn *conn, u8 ltk[16]);
> +void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
> +void hci_le_ltk_neg_reply(struct hci_conn *conn);
> +
>  #endif /* __HCI_CORE_H */
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index edfb48b..f919ddb 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -183,6 +183,53 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
>  	hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
>  }
>  
> +void hci_le_start_enc(struct hci_conn *conn, u8 ltk[16])
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +	struct hci_cp_le_start_enc cp;
> +
> +	BT_DBG("%p", conn);
> +
> +	memset(&cp, 0, sizeof(cp));
> +
> +	cp.handle = cpu_to_le16(conn->handle);
> +	memcpy(cp.ltk, ltk, 16);
> +
> +	hci_send_cmd(hdev, HCI_OP_LE_START_ENC, sizeof(cp), &cp);
> +}
> +EXPORT_SYMBOL(hci_le_start_enc);
> +
> +void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16])
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +	struct hci_cp_le_ltk_reply cp;
> +
> +	BT_DBG("%p", conn);
> +
> +	memset(&cp, 0, sizeof(cp));
> +
> +	cp.handle = cpu_to_le16(conn->handle);
> +	memcpy(&cp.ltk, ltk, sizeof(ltk));
> +
> +	hci_send_cmd(hdev, HCI_OP_LE_LTK_REPLY, sizeof(cp), &cp);
> +}
> +EXPORT_SYMBOL(hci_le_ltk_reply);
> +
> +void hci_le_ltk_neg_reply(struct hci_conn *conn)
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +	struct hci_cp_le_ltk_neg_reply cp;
> +
> +	BT_DBG("%p", conn);
> +
> +	memset(&cp, 0, sizeof(cp));
> +
> +	cp.handle = cpu_to_le16(conn->handle);
> +
> +	hci_send_cmd(hdev, HCI_OP_LE_LTK_NEG_REPLY, sizeof(cp), &cp);
> +}
> +EXPORT_SYMBOL(hci_le_ltk_neg_reply);
> +
>  /* Device _must_ be locked */
>  void hci_sco_setup(struct hci_conn *conn, __u8 status)
>  {
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 55cdd6a..c90696f 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -559,6 +559,30 @@ static void hci_cc_le_read_buffer_size(struct hci_dev *hdev,
>  	hci_req_complete(hdev, rp->status);
>  }
>  
> +static void hci_cc_le_ltk_reply(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	struct hci_rp_le_ltk_reply *rp = (void *) skb->data;
> +
> +	BT_DBG("%s status 0x%x", hdev->name, rp->status);
> +
> +	if (rp->status)
> +		return;
> +
> +	hci_req_complete(hdev, rp->status);
> +}
> +
> +static void hci_cc_le_ltk_neg_reply(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	struct hci_rp_le_ltk_neg_reply *rp = (void *) skb->data;
> +
> +	BT_DBG("%s status 0x%x", hdev->name, rp->status);
> +
> +	if (rp->status)
> +		return;
> +
> +	hci_req_complete(hdev, rp->status);
> +}
> +
>  static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
>  {
>  	BT_DBG("%s status 0x%x", hdev->name, status);
> @@ -920,6 +944,11 @@ static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
>  	hci_dev_unlock(hdev);
>  }
>  
> +static void hci_cs_le_start_enc(struct hci_dev *hdev, u8 status)
> +{
> +	BT_DBG("%s status 0x%x", hdev->name, status);
> +}
> +
>  static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>  	__u8 status = *((__u8 *) skb->data);
> @@ -1440,6 +1469,14 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
>  		hci_cc_le_read_buffer_size(hdev, skb);
>  		break;
>  
> +	case HCI_OP_LE_LTK_REPLY:
> +		hci_cc_le_ltk_reply(hdev, skb);
> +		break;
> +
> +	case HCI_OP_LE_LTK_NEG_REPLY:
> +		hci_cc_le_ltk_neg_reply(hdev, skb);
> +		break;
> +
>  	default:
>  		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
>  		break;
> @@ -1510,6 +1547,10 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  		hci_cs_le_create_conn(hdev, ev->status);
>  		break;
>  
> +	case HCI_OP_LE_START_ENC:
> +		hci_cs_le_start_enc(hdev, ev->status);
> +		break;
> +
>  	default:
>  		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
>  		break;
> @@ -2013,6 +2054,28 @@ unlock:
>  	hci_dev_unlock(hdev);
>  }
>  
> +static inline void hci_le_ltk_request_evt(struct hci_dev *hdev,
> +						struct sk_buff *skb)
> +{
> +	struct hci_ev_le_ltk_req *ev = (void *) skb->data;
> +	struct hci_cp_le_ltk_reply cp;
> +	struct hci_conn *conn;
> +
> +	BT_DBG("%s handle %d", hdev->name, cpu_to_le16(ev->handle));
> +
> +	hci_dev_lock(hdev);
> +
> +	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
> +
> +	memset(&cp, 0, sizeof(cp));
> +	cp.handle = cpu_to_le16(conn->handle);
> +	memcpy(cp.ltk, conn->ltk, sizeof(conn->ltk));
> +
> +	hci_send_cmd(hdev, HCI_OP_LE_LTK_REPLY, sizeof(cp), &cp);
> +
> +	hci_dev_unlock(hdev);
> +}
> +
>  static inline void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>  	struct hci_ev_le_meta *le_ev = (void *) skb->data;
> @@ -2024,6 +2087,10 @@ static inline void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  		hci_le_conn_complete_evt(hdev, skb);
>  		break;
>  
> +	case HCI_EV_LE_LTK_REQ:
> +		hci_le_ltk_request_evt(hdev, skb);
> +		break;
> +
>  	default:
>  		break;
>  	}
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 7d7e8ad..d19b8a2 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -289,7 +289,8 @@ static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb
>  
>  static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
>  {
> -	struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
> +	struct hci_conn *hcon = conn->hcon;
> +	struct crypto_blkcipher *tfm = hcon->hdev->tfm;
>  	int ret;
>  	u8 k[16], key[16], res[16], random[16], confirm[16], buf[128];
>  
> @@ -297,6 +298,7 @@ static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
>  	skb_pull(skb, 16);
>  
>  	memset(k, 0, sizeof(k));
> +	memset(hcon->ltk, 0, sizeof(hcon->ltk));
>  
>  	if (conn->hcon->out)
>  		ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
> @@ -320,6 +322,9 @@ static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
>  
>  	if (conn->hcon->out) {
>  		smp_s1(tfm, k, random, conn->prnd, key);
> +		swap128(key, hcon->ltk);
> +
> +		hci_le_start_enc(conn->hcon, hcon->ltk);

You have hcon here, no need to use conn->hcon.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [RFC v2 5/9] Bluetooth: Add support for using the crypto subsystem
From: Gustavo F. Padovan @ 2010-12-07 17:27 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1291671832-13435-6-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

* Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2010-12-06 18:43:48 -0300]:

> This will allow using the crypto subsystem for encrypting data. As SMP
> (Security Manager Protocol) is implemented almost entirely on the host
> side and the crypto module already implements the needed methods
> (AES-128), it makes sense to use it.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    2 ++
>  net/bluetooth/hci_core.c         |   10 ++++++++++
>  2 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0687e2f..d0a9f5d 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -135,6 +135,8 @@ struct hci_dev {
>  	__u32			req_status;
>  	__u32			req_result;
>  
> +	struct crypto_blkcipher	*tfm;
> +
>  	struct inquiry_cache	inq_cache;
>  	struct hci_conn_hash	conn_hash;
>  	struct list_head	blacklist;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 12c6735..b96c3dd 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -41,6 +41,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/notifier.h>
>  #include <linux/rfkill.h>
> +#include <linux/crypto.h>
>  #include <net/sock.h>
>  
>  #include <asm/system.h>
> @@ -961,6 +962,13 @@ int hci_register_dev(struct hci_dev *hdev)
>  	if (!hdev->workqueue)
>  		goto nomem;
>  
> +	hdev->tfm = crypto_alloc_blkcipher("ecb(aes)", 0, CRYPTO_ALG_ASYNC);
> +	if (IS_ERR(hdev->tfm)) {
> +		BT_ERR("Failed to load transform for ecb(aes): %ld",
> +							PTR_ERR(hdev->tfm));
> +		goto nomem;

You are leaking hdev->workqueue here.

Also you will need to add CRYPTO_BLKCIPHER dependence in the Kconfig.
Maybe we should add a CONFIG_BLUETOOTH_SMP, and just build with blkcipher
in the case SMP was selected to be built.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* RE: EVT_NUM_COMP_PKTS and LE, GATT and SM
From: Brian Gix @ 2010-12-07 17:18 UTC (permalink / raw)
  To: 'Ville Tervo'; +Cc: linux-bluetooth, 'Vinicius Costa Gomes'
In-Reply-To: <20101207084704.GY874@null>

Hi Ville, Vinicius,

On Tue, Dec 07, 2010 at 12:47 AM -0800, Ville Tervo wrote:
> Hi Brian,
> 
> On Mon, Dec 06, 2010 at 04:35:51PM -0800, ext Brian Gix wrote:
> > Hi All,
> >
> > I've have encountered a problem while using the gatttool, where my
> write
> > commands get clobbered by the LE ACL being disconnected prior to the
> ATT
> > (fixed channel 4) WRITE_CMD being sent over the LE based ACL link.
> >
> > I believe this is fundamentally due to there being no dependency on
> the
> > EVT_NUM_COMP_PKTS event when gatttool decides that the WRITE_CMD has
> been
> > successfully sent.  There is multiple parts to this.
> >
> > 1. In User space, the WRITE_CMD pkt is written to the socket.
> Gatttool
> > erroneously considers a successful socket write as completion,
> disconnects
> > the socket and exits.

I would like to try Vinicius' patch that (may) address this, but again
I do not have access to infradead, although I think gitorious, which
you used before, was not a problem.

> >
> > 2. In Kernel space, the ACL packet is added to an ACL queue, which is
> > separate from the CMD queue, which can allow either the Disconnect
> request,
> > or the ACL packet to be sent over the H4 link to the baseband.
> >
> 
> And in usb case CMD and ACL are even using different endpoints.
> 
> > 3. In the baseband, due to LE clocking (and possible other baseband
> > activity) the ACL packet could be received first, and the Disconnect
> CMD
> > second, and still result in the connection being detached prior to Tx
> of the
> > ACL packet containing the ATT WRITE_CMD.
> >
> > This is not an issue with  any of the ATT READ/FIND/MTU or WRITE_REQ
> > transactions, because they require a response from the server.  I
> believe
> > for ATT, this problem is restricted to the WRITE_CMD only, due to
> it's
> > unacknowledged nature.
> 
> True.
> 
> > However, this will also be an issue with the LE Security Manager,
> because as
> > stated in the Core Spec v4.0, Vol 3, in the last paragraph of 3.6.1
> Key
> > Distribution on page 630 (of 656):
> >
> > 	> Key distribution is complete in the device sending the final
> key
> > when it receives
> > 	> the baseband acknowledgement for that key and is complete in
> the
> > receiving
> > 	> device when it receives the final key being distributed.
> >
> > This is intended to prevent exactly the kind of problem I am
> experiencing
> > with the ATT WRITE_CMD, and the acknowledgement from the baseband can
> only
> > be the EVT_NUM_COMP_PKTS event.
> >
> > While talking to my colleagues here, we were thinking that the
> cleanest
> > method to get this accomplished would be by using the "select" method
> with
> > the ATT socket, where the socket could be marked as non-writable by
> the
> > kernel driver until the EVT_NUM_COMP_PKTS is received. The User space
> code
> > could then wait for the socket to become writeable before issuing the
> socket
> > disconnect.
> >
> 
> How about implement waiting on l2cap_sock_shutdown() like for ERTM.
> User space
> could then call shutdown() before closing the socket so make sure the
> data is
> sent or timeout occurred.

If I understand l2cap_sock_shutdown() correctly, then I think this
is the correct solution.  The socket mode would be BASIC, and the
channel (dcid) would be fixed-4 (or perhaps fixed-5 or fixed-6 as well),
but instead of waiting for an ack, it would be waiting on the
EVT_NUM_COMP_PKTS, that indicates the number of outstanding ACL packets
on the connection is Zero, with the rest of the logic unchanged.


> 
> Would this be enough to solve the problem?
> 
> > If the Security manager is totally within the kernel, it probably
> does not
> > have to do as much work, however it does still need to wait on the
> > EVT_NUM_COMP_PKTS before disconnecting the remote device.
> >
> > Has anybody else observed this issue with ATT WRITE_CMD? It could be
> getting
> > exacerbated by slow (115Kbps) H4 links that I am using, however the
> hcidump
> > tool confirms that the disconnect happens prior to the
> EVT_NUM_COMP_PKTS.
> 
> I have seen this problem but didn't dig it deeper.
> 
> --
> Ville


^ permalink raw reply

* Re: [RFC v2 4/9] Bluetooth: simple SMP pairing negotiation
From: Gustavo F. Padovan @ 2010-12-07 16:39 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth, Anderson Briglia
In-Reply-To: <1291671832-13435-5-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

* Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2010-12-06 18:43:47 -0300]:

> From: Anderson Briglia <anderson.briglia@openbossa.org>
> 
> This implementation only exchanges SMP messages between the Host and the
> Remote. No keys are being generated. TK and STK generation will be
> provided in further patches.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  net/bluetooth/l2cap_core.c |    3 +-
>  net/bluetooth/smp.c        |  114 ++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 112 insertions(+), 5 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 674799c..da4f13d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4630,7 +4630,8 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
>  		break;
>  
>  	case L2CAP_CID_SMP:
> -		smp_sig_channel(conn, skb);
> +		if (smp_sig_channel(conn, skb))
> +			l2cap_conn_del(conn->hcon, 0x05);

So this could be in the previous patch instead of this one.

>  		break;
>  
>  	default:
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index e9dde5f..b25010f 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -64,6 +64,102 @@ static void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
>  	hci_send_acl(conn->hcon, skb, 0);
>  }
>  
> +static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
> +{
> +	struct smp_cmd_pairing *rp = (void *) skb->data;
> +
> +	BT_DBG("");

BT_DBG("conn %p", conn); is better. Same for the other functions below.

> +
> +	skb_pull(skb, sizeof(struct smp_cmd_pairing));
> +
> +	rp->io_capability = 0x00;
> +	rp->oob_flag = 0x00;
> +	rp->max_key_size = 16;
> +	rp->init_key_dist = 0x00;
> +	rp->resp_key_dist = 0x00;
> +	rp->auth_req &= 0x05;
> +
> +	smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
> +}
> +
> +static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
> +{
> +	struct smp_cmd_pairing_confirm cp;
> +
> +	BT_DBG("");
> +
> +	memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
> +
> +	smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
> +}
> +
> +static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
> +{
> +	BT_DBG("");

	BT_DBG("conn %p %s", conn, conn->hcon->out ? "master" : "slave"); 

Is better, no?

> +
> +	if (conn->hcon->out) {
> +		struct smp_cmd_pairing_random random;
> +
> +		BT_DBG("master");
> +
> +		memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
> +
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
> +								&random);
> +	} else {
> +		struct smp_cmd_pairing_confirm confirm;
> +
> +		BT_DBG("slave");
> +
> +		memset(&confirm, 0, sizeof(struct smp_cmd_pairing_confirm));
> +
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(confirm),
> +								&confirm);
> +	}
> +}
> +
> +static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
> +{
> +	struct smp_cmd_pairing_random cp;
> +
> +	BT_DBG("");
> +
> +	skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
> +
> +	/* FIXME: check if random matches */
> +
> +	if (conn->hcon->out) {
> +		BT_DBG("master");
> +		/* FIXME: start encryption */
> +	} else {
> +		BT_DBG("slave");
> +
> +		memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
> +
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp), &cp);
> +	}
> +}
> +
> +static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
> +{
> +	struct smp_cmd_security_req *rp = (void *) skb->data;
> +	struct smp_cmd_pairing cp;
> +
> +	BT_DBG("");
> +
> +	skb_pull(skb, sizeof(struct smp_cmd_security_req));
> +	memset(&cp, 0, sizeof(struct smp_cmd_pairing));
> +
> +	cp.io_capability = 0x00;
> +	cp.oob_flag = 0x00;
> +	cp.max_key_size = 16;
> +	cp.init_key_dist = 0x00;
> +	cp.resp_key_dist = 0x00;
> +	cp.auth_req = rp->auth_req & 0x05;
> +
> +	smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
> +}
> +
>  int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
>  {
>  	__u8 authreq;
> @@ -114,23 +210,33 @@ int smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
>  
>  	switch (code) {
>  	case SMP_CMD_PAIRING_REQ:
> -		reason = SMP_PAIRING_NOTSUPP;
> -		smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
> -		err = -1;
> +		smp_cmd_pairing_req(conn, skb);
>  		break;
>  
>  	case SMP_CMD_PAIRING_FAIL:
>  		break;
>  
>  	case SMP_CMD_PAIRING_RSP:
> +		smp_cmd_pairing_rsp(conn, skb);
> +		break;
> +
> +	case SMP_CMD_SECURITY_REQ:
> +		smp_cmd_security_req(conn, skb);
> +		break;
> +
>  	case SMP_CMD_PAIRING_CONFIRM:
> +		smp_cmd_pairing_confirm(conn, skb);
> +		break;
> +
>  	case SMP_CMD_PAIRING_RANDOM:
> +		smp_cmd_pairing_random(conn, skb);
> +		break;
> +
>  	case SMP_CMD_ENCRYPT_INFO:
>  	case SMP_CMD_MASTER_IDENT:
>  	case SMP_CMD_IDENT_INFO:
>  	case SMP_CMD_IDENT_ADDR_INFO:
>  	case SMP_CMD_SIGN_INFO:
> -	case SMP_CMD_SECURITY_REQ:
>  	default:
>  		BT_DBG("Unknown command code 0x%2.2x", code);
>  
> -- 
> 1.7.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [RFC v2 3/9] Bluetooth: Start SMP procedure
From: Gustavo F. Padovan @ 2010-12-07 16:11 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth, Anderson Briglia
In-Reply-To: <1291671832-13435-4-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

* Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2010-12-06 18:43:46 -0300]:

> From: Anderson Briglia <anderson.briglia@openbossa.org>
> 
> Start SMP procedure for LE connections. This modification intercepts l2cap
> received frames and call proper SMP functions to start the SMP procedure. By
> now, no keys are being used.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
> ---
>  net/bluetooth/l2cap_core.c |    7 +++++++
>  net/bluetooth/smp.c        |    2 +-
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 69e5f80..674799c 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -54,6 +54,7 @@
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
>  #include <net/bluetooth/l2cap.h>
> +#include <net/bluetooth/smp.h>
>  
>  #define VERSION "2.15"
>  
> @@ -642,6 +643,8 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
>  			l2cap_sock_clear_timer(sk);
>  			sk->sk_state = BT_CONNECTED;
>  			sk->sk_state_change(sk);
> +			if (smp_conn_security(conn, l2cap_pi(sk)->sec_level))
> +				BT_DBG("Insufficient security");
>  		}
>  
>  		if (sk->sk_type != SOCK_SEQPACKET &&
> @@ -4626,6 +4629,10 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
>  		l2cap_conless_channel(conn, psm, skb);
>  		break;
>  
> +	case L2CAP_CID_SMP:
> +		smp_sig_channel(conn, skb);
> +		break;
> +
>  	default:
>  		l2cap_data_channel(conn, cid, skb);
>  		break;
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index e427d11..e9dde5f 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -86,7 +86,7 @@ int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
>  		return 1;
>  	}
>  
> -	if (conn->hcon->link_mode & HCI_LM_MASTER) {
> +	if (conn->hcon->out) {

This change should not belong to this patch.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [RFC v2 2/9] Bluetooth: Implement the first SMP commands
From: Gustavo F. Padovan @ 2010-12-07 16:10 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth, Anderson Briglia
In-Reply-To: <1291671832-13435-3-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

* Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2010-12-06 18:43:45 -0300]:

> These simple commands will allow the SMP procedure to be started
> and terminated with a not supported error. This is the first step
> toward something useful.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
> ---
>  include/net/bluetooth/smp.h             |    4 +
>  net/bluetooth/Makefile                  |    1 +
>  net/bluetooth/{l2cap.c => l2cap_core.c} |    0
>  net/bluetooth/smp.c                     |  144 +++++++++++++++++++++++++++++++
>  4 files changed, 149 insertions(+), 0 deletions(-)
>  rename net/bluetooth/{l2cap.c => l2cap_core.c} (100%)
>  create mode 100644 net/bluetooth/smp.c
> 
> diff --git a/include/net/bluetooth/smp.h b/include/net/bluetooth/smp.h
> index 8f2edbf..b9603cc 100644
> --- a/include/net/bluetooth/smp.h
> +++ b/include/net/bluetooth/smp.h
> @@ -73,4 +73,8 @@ struct smp_cmd_security_req {
>  #define SMP_UNSPECIFIED		0x08
>  #define SMP_REPEATED_ATTEMPTS		0x09
>  
> +/* SMP Commands */
> +int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level);
> +int smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb);
> +
>  #endif /* __SMP_H */
> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
> index d1e433f..d138b23 100644
> --- a/net/bluetooth/Makefile
> +++ b/net/bluetooth/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_BT_CMTP)	+= cmtp/
>  obj-$(CONFIG_BT_HIDP)	+= hidp/
>  
>  bluetooth-objs := af_bluetooth.o hci_core.o hci_conn.o hci_event.o hci_sock.o hci_sysfs.o lib.o
> +l2cap-objs := l2cap_core.o smp.o
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap_core.c
> similarity index 100%
> rename from net/bluetooth/l2cap.c
> rename to net/bluetooth/l2cap_core.c
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> new file mode 100644
> index 0000000..e427d11
> --- /dev/null
> +++ b/net/bluetooth/smp.c
> @@ -0,0 +1,144 @@
> +/*
> +   BlueZ - Bluetooth protocol stack for Linux
> +   Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License version 2 as
> +   published by the Free Software Foundation;
> +
> +   THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> +   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +   FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
> +   IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
> +   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
> +   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> +   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> +   OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> +
> +   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
> +   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
> +   SOFTWARE IS DISCLAIMED.
> +*/
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/l2cap.h>
> +#include <net/bluetooth/smp.h>
> +
> +static struct sk_buff *smp_build_cmd(struct l2cap_conn *conn, u8 code,
> +		u16 dlen, void *data)
> +{
> +	struct sk_buff *skb;
> +	struct l2cap_hdr *lh;
> +	int len;
> +
> +	len = L2CAP_HDR_SIZE + 1 + dlen;
> +
> +	if (len > conn->mtu)
> +		return NULL;
> +
> +	skb = bt_skb_alloc(len, GFP_ATOMIC);
> +	if (!skb)
> +		return NULL;
> +
> +	lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
> +	lh->len = cpu_to_le16(1 + dlen);
> +	lh->cid = cpu_to_le16(L2CAP_CID_SMP);
> +
> +	memcpy(skb_put(skb, 1), &code, 1);
> +
> +	memcpy(skb_put(skb, dlen), data, dlen);
> +
> +	return skb;
> +}
> +
> +static void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
> +{
> +	struct sk_buff *skb = smp_build_cmd(conn, code, len, data);
> +
> +	BT_DBG("code 0x%2.2x", code);
> +
> +	if (!skb)
> +		return;
> +
> +	hci_send_acl(conn->hcon, skb, 0);
> +}
> +
> +int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
> +{
> +	__u8 authreq;
> +
> +	BT_DBG("conn %p hcon %p level 0x%2.2x", conn, conn->hcon, sec_level);
> +
> +	switch (sec_level) {
> +	case BT_SECURITY_MEDIUM:
> +		/* Encrypted, no MITM protection */
> +		authreq = 0x01;
> +		break;
> +
> +	case BT_SECURITY_HIGH:
> +		/* Bonding, MITM protection */
> +		authreq = 0x05;

It would be good have some defines for the authreq values.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [RFC v2 2/9] Bluetooth: Implement the first SMP commands
From: Gustavo F. Padovan @ 2010-12-07 16:03 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth, Anderson Briglia
In-Reply-To: <1291671832-13435-3-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

* Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2010-12-06 18:43:45 -0300]:

> These simple commands will allow the SMP procedure to be started
> and terminated with a not supported error. This is the first step
> toward something useful.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
> ---
>  include/net/bluetooth/smp.h             |    4 +
>  net/bluetooth/Makefile                  |    1 +
>  net/bluetooth/{l2cap.c => l2cap_core.c} |    0

I want a separated patch for the l2cap.c rename.

>  net/bluetooth/smp.c                     |  144 +++++++++++++++++++++++++++++++
>  4 files changed, 149 insertions(+), 0 deletions(-)
>  rename net/bluetooth/{l2cap.c => l2cap_core.c} (100%)
>  create mode 100644 net/bluetooth/smp.c
> 
> diff --git a/include/net/bluetooth/smp.h b/include/net/bluetooth/smp.h
> index 8f2edbf..b9603cc 100644
> --- a/include/net/bluetooth/smp.h
> +++ b/include/net/bluetooth/smp.h
> @@ -73,4 +73,8 @@ struct smp_cmd_security_req {
>  #define SMP_UNSPECIFIED		0x08
>  #define SMP_REPEATED_ATTEMPTS		0x09
>  
> +/* SMP Commands */
> +int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level);
> +int smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb);
> +
>  #endif /* __SMP_H */
> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
> index d1e433f..d138b23 100644
> --- a/net/bluetooth/Makefile
> +++ b/net/bluetooth/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_BT_CMTP)	+= cmtp/
>  obj-$(CONFIG_BT_HIDP)	+= hidp/
>  
>  bluetooth-objs := af_bluetooth.o hci_core.o hci_conn.o hci_event.o hci_sock.o hci_sysfs.o lib.o
> +l2cap-objs := l2cap_core.o smp.o
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap_core.c
> similarity index 100%
> rename from net/bluetooth/l2cap.c
> rename to net/bluetooth/l2cap_core.c
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> new file mode 100644
> index 0000000..e427d11
> --- /dev/null
> +++ b/net/bluetooth/smp.c
> @@ -0,0 +1,144 @@
> +/*
> +   BlueZ - Bluetooth protocol stack for Linux
> +   Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License version 2 as
> +   published by the Free Software Foundation;
> +
> +   THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> +   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +   FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
> +   IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
> +   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
> +   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> +   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> +   OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> +
> +   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
> +   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
> +   SOFTWARE IS DISCLAIMED.
> +*/
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/l2cap.h>
> +#include <net/bluetooth/smp.h>
> +
> +static struct sk_buff *smp_build_cmd(struct l2cap_conn *conn, u8 code,
> +		u16 dlen, void *data)
> +{
> +	struct sk_buff *skb;
> +	struct l2cap_hdr *lh;
> +	int len;
> +
> +	len = L2CAP_HDR_SIZE + 1 + dlen;
> +
> +	if (len > conn->mtu)
> +		return NULL;
> +
> +	skb = bt_skb_alloc(len, GFP_ATOMIC);
> +	if (!skb)
> +		return NULL;
> +
> +	lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
> +	lh->len = cpu_to_le16(1 + dlen);
> +	lh->cid = cpu_to_le16(L2CAP_CID_SMP);
> +
> +	memcpy(skb_put(skb, 1), &code, 1);
> +
> +	memcpy(skb_put(skb, dlen), data, dlen);
> +
> +	return skb;
> +}
> +
> +static void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
> +{
> +	struct sk_buff *skb = smp_build_cmd(conn, code, len, data);
> +
> +	BT_DBG("code 0x%2.2x", code);
> +
> +	if (!skb)
> +		return;
> +
> +	hci_send_acl(conn->hcon, skb, 0);
> +}
> +
> +int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
> +{
> +	__u8 authreq;
> +
> +	BT_DBG("conn %p hcon %p level 0x%2.2x", conn, conn->hcon, sec_level);
> +
> +	switch (sec_level) {
> +	case BT_SECURITY_MEDIUM:
> +		/* Encrypted, no MITM protection */
> +		authreq = 0x01;
> +		break;
> +
> +	case BT_SECURITY_HIGH:
> +		/* Bonding, MITM protection */
> +		authreq = 0x05;
> +		break;
> +
> +	case BT_SECURITY_LOW:
> +	default:
> +		return 1;
> +	}
> +
> +	if (conn->hcon->link_mode & HCI_LM_MASTER) {
> +		struct smp_cmd_pairing cp;
> +		cp.io_capability = 0x00;
> +		cp.oob_flag = 0x00;
> +		cp.max_key_size = 16;
> +		cp.init_key_dist = 0x00;
> +		cp.resp_key_dist = 0x00;
> +		cp.auth_req = authreq;
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
> +	} else {
> +		struct smp_cmd_security_req cp;
> +		cp.auth_req = authreq;
> +		smp_send_cmd(conn, SMP_CMD_SECURITY_REQ, sizeof(cp), &cp);
> +	}
> +
> +	return 0;
> +}
> +
> +int smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
> +{
> +	__u8 code = skb->data[0];
> +	__u8 reason;
> +	int err = 0;
> +
> +	skb_pull(skb, 1);
> +
> +	switch (code) {
> +	case SMP_CMD_PAIRING_REQ:
> +		reason = SMP_PAIRING_NOTSUPP;
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
> +		err = -1;

Don't use -1, use a proper error macro here.

> +		break;
> +
> +	case SMP_CMD_PAIRING_FAIL:
> +		break;
> +
> +	case SMP_CMD_PAIRING_RSP:
> +	case SMP_CMD_PAIRING_CONFIRM:
> +	case SMP_CMD_PAIRING_RANDOM:
> +	case SMP_CMD_ENCRYPT_INFO:
> +	case SMP_CMD_MASTER_IDENT:
> +	case SMP_CMD_IDENT_INFO:
> +	case SMP_CMD_IDENT_ADDR_INFO:
> +	case SMP_CMD_SIGN_INFO:
> +	case SMP_CMD_SECURITY_REQ:
> +	default:
> +		BT_DBG("Unknown command code 0x%2.2x", code);
> +
> +		reason = SMP_CMD_NOTSUPP;
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
> +		err = -1;

Same here.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 1/1] bluetooth: Fix NULL pointer dereference issue
From: Gustavo F. Padovan @ 2010-12-07 15:50 UTC (permalink / raw)
  To: Yuri Ershov
  Cc: marcel, davem, jprvita, ville.tervo, andrei.emeltchenko,
	linux-bluetooth@vger.kernel.org
In-Reply-To: <4CFE32A0.6090601@nokia.com>

Hi Yuri,

* Yuri Ershov <ext-yuri.ershov@nokia.com> [2010-12-07 16:12:00 +0300]:

> Hello Gustavo,
> 
> ext Gustavo F. Padovan wrote:
> > Hi Yuri,
> >
> > * Yuri Ershov <ext-yuri.ershov@nokia.com> [2010-11-25 12:55:33 +0300]:
> >
> >   
> >> This patch is an addition to my previous patch for this issue.
> >> The problem is in resynchronization between two loops:
> >> 1. Main controlling loop (l2cap_connect_req, l2cap_config_req,
> >> l2cap_config_rsp, l2cap_disconnect_req, etc.)
> >> 2. Loop waiting of BT_CONNECTED state of socket (l2cap_sock_accept,
> >> bt_accept_dequeue, etc.).
> >> In case of fast sequence of connect/disconnect operations the loop #1
> >> makes several cycles, while the loop #2 only has time to make one
> >> cycle and it results crash.
> >> The aim of the patch is to skeep handling of sockets queued for
> >> deletion.
> >>
> >> Signed-off-by: Yuri Ershov <ext-yuri.ershov@nokia.com>
> >> ---
> >>  net/bluetooth/af_bluetooth.c |    2 ++
> >>  net/bluetooth/l2cap.c        |    6 ++++--
> >>  2 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> >> index c4cf3f5..f9389da 100644
> >> --- a/net/bluetooth/af_bluetooth.c
> >> +++ b/net/bluetooth/af_bluetooth.c
> >> @@ -200,6 +200,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
> >>  	BT_DBG("parent %p", parent);
> >>  
> >>  	list_for_each_safe(p, n, &bt_sk(parent)->accept_q) {
> >> +		if (n == p)
> >> +			break;
> >>     
> >
> > So in which situations (n == p), or (p == p->next)? That should happen only
> > when p is the only element in the list, then p == head, right?
> >   
> The (n == p) is in situation, when sk is unlinked by task responsible 
> for handling connect/disconnect requests while the "bt_accept_dequeue". 
> This condition is indirect checking of sk validity.

Why not using a list lock here instead? Fits a way better.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 1/1] bluetooth: add NULL pointer check in hci
From: Gustavo F. Padovan @ 2010-12-07 15:06 UTC (permalink / raw)
  To: Jun Nie; +Cc: Vinicius Costa Gomes, Brian Gix, linux-bluetooth
In-Reply-To: <AANLkTinu7oKUD5FmGyJ2x+ZYqRmOyFijuZgWtgjg=tk=@mail.gmail.com>

Hi Jun,

* Jun Nie <niej0001@gmail.com> [2010-12-07 15:01:21 +0800]:

> Resend it to fix checkpatch.pl warning.

> From 75dc111b5d9f62619bbeec803b15e84412ae050e Mon Sep 17 00:00:00 2001
> From: Jun Nie <njun@marvell.com>
> Date: Tue, 7 Dec 2010 14:03:38 +0800
> Subject: [PATCH] bluetooth: add NULL pointer check in hci

Clearly a bug fix, but can you add a commit message to your patch. Thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* [PATCH 3/3] Add support for stat files bigger than 2GB on 32-bit systems
From: Luiz Augusto von Dentz @ 2010-12-07 15:01 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291734061-24408-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

>From stat documentation:

"(stat())  path  refers to a file whose size cannot be represented in the
type off_t.  This can occur when an application compiled on a 32-bit
platform without -D_FILE_OFFSET_BITS=64 calls stat() on a file whose size
exceeds (2<<31)-1 bits."

Note that this doesn't really help for files bigger than 4GB since obex
header field is 32-bits, but since folder-listing has no such limitation
it probably worth doing this anyway.
---
 acinclude.m4         |    2 +-
 client/transfer.c    |    5 +++--
 plugins/filesystem.c |    2 +-
 src/obex-priv.h      |    6 +++---
 src/obex.c           |    6 +++---
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 4594073..3d1f511 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -12,7 +12,7 @@ AC_DEFUN([AC_PROG_CC_PIE], [
 
 AC_DEFUN([COMPILER_FLAGS], [
 	if (test "${CFLAGS}" = ""); then
-		CFLAGS="-Wall -O2 -D_FORTIFY_SOURCE=2"
+		CFLAGS="-Wall -O2 -D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64"
 	fi
 	if (test "$USE_MAINTAINER_MODE" = "yes"); then
 		CFLAGS+=" -Werror -Wextra"
diff --git a/client/transfer.c b/client/transfer.c
index 6eec513..daab4dc 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -470,7 +470,7 @@ int transfer_put(struct transfer_data *transfer, transfer_callback_t func,
 	struct session_data *session = transfer->session;
 	gw_obex_xfer_cb_t cb;
 	struct stat st;
-	int fd;
+	int fd, size;
 
 	if (transfer->xfer != NULL)
 		return -EALREADY;
@@ -497,8 +497,9 @@ int transfer_put(struct transfer_data *transfer, transfer_callback_t func,
 	cb = put_xfer_progress;
 
 done:
+	size = transfer->size < G_MAXUINT32 ? transfer->size : 0;
 	transfer->xfer = gw_obex_put_async(session->obex, transfer->name,
-						transfer->type, transfer->size,
+						transfer->type, size,
 						-1, NULL);
 	if (transfer->xfer == NULL)
 		return -ENOTCONN;
diff --git a/plugins/filesystem.c b/plugins/filesystem.c
index bb758ab..0386c92 100644
--- a/plugins/filesystem.c
+++ b/plugins/filesystem.c
@@ -66,7 +66,7 @@
 
 #define FL_PARENT_FOLDER_ELEMENT "<parent-folder/>" EOL_CHARS
 
-#define FL_FILE_ELEMENT "<file name=\"%s\" size=\"%lu\"" \
+#define FL_FILE_ELEMENT "<file name=\"%s\" size=\"%llu\"" \
 			" %s accessed=\"%s\" " \
 			"modified=\"%s\" created=\"%s\"/>" EOL_CHARS
 
diff --git a/src/obex-priv.h b/src/obex-priv.h
index c9be1c5..0806a56 100644
--- a/src/obex-priv.h
+++ b/src/obex-priv.h
@@ -33,9 +33,9 @@ struct obex_session {
 	char *path;
 	time_t time;
 	uint8_t *buf;
-	int32_t pending;
-	int32_t offset;
-	int32_t size;
+	int64_t pending;
+	int64_t offset;
+	int64_t size;
 	void *object;
 	gboolean aborted;
 	struct obex_service_driver *service;
diff --git a/src/obex.c b/src/obex.c
index 6d4430d..99bfecc 100644
--- a/src/obex.c
+++ b/src/obex.c
@@ -590,7 +590,7 @@ static int obex_read_stream(struct obex_session *os, obex_t *obex,
 
 	/* only write if both object and driver are valid */
 	if (os->object == NULL || os->driver == NULL) {
-		DBG("Stored %u bytes into temporary buffer", os->pending);
+		DBG("Stored %llu bytes into temporary buffer", os->pending);
 		return 0;
 	}
 
@@ -796,7 +796,7 @@ static void cmd_get(struct obex_session *os, obex_t *obex, obex_object_t *obj)
 	if (err < 0)
 		goto done;
 
-	if (os->size != OBJECT_SIZE_UNKNOWN) {
+	if (os->size != OBJECT_SIZE_UNKNOWN && os->size < G_MAXUINT32) {
 		hd.bq4 = os->size;
 		OBEX_ObjectAddHeader(obex, obj,
 				OBEX_HDR_LENGTH, hd, 4, 0);
@@ -1005,7 +1005,7 @@ static gboolean check_put(obex_t *obex, obex_object_t *obj)
 
 		case OBEX_HDR_LENGTH:
 			os->size = hd.bq4;
-			DBG("OBEX_HDR_LENGTH: %d", os->size);
+			DBG("OBEX_HDR_LENGTH: %llu", os->size);
 			break;
 		case OBEX_HDR_TIME:
 			os->time = parse_iso8610((const char *) hd.bs, hlen);
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/3] Fix logging for obex-client
From: Luiz Augusto von Dentz @ 2010-12-07 15:01 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291734061-24408-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

Since obex-client and obexd share the same log code they both were using
obexd for openlog which makes it very confusing when reading the logs.

To fix this now __obex_log_init takes the binary name so that each daemon
can be properly labeled.
---
 client/main.c |    2 +-
 src/log.c     |    6 +++---
 src/log.h     |    2 +-
 src/main.c    |    2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/client/main.c b/client/main.c
index 74db15e..20d56d2 100644
--- a/client/main.c
+++ b/client/main.c
@@ -613,7 +613,7 @@ int main(int argc, char *argv[])
 
 	event_loop = g_main_loop_new(NULL, FALSE);
 
-	__obex_log_init(option_debug, !option_stderr);
+	__obex_log_init("obex-client", option_debug, !option_stderr);
 
 	DBG("Entering main loop");
 
diff --git a/src/log.c b/src/log.c
index 39489a2..baa57c5 100644
--- a/src/log.c
+++ b/src/log.c
@@ -100,7 +100,7 @@ void __obex_log_enable_debug()
 		desc->flags |= OBEX_DEBUG_FLAG_PRINT;
 }
 
-void __obex_log_init(const char *debug, int detach)
+void __obex_log_init(const char *label, const char *debug, int detach)
 {
 	int option = LOG_NDELAY | LOG_PID;
 	struct obex_debug_desc *desc;
@@ -125,9 +125,9 @@ void __obex_log_init(const char *debug, int detach)
 	if (!detach)
 		option |= LOG_PERROR;
 
-	openlog("obexd", option, LOG_DAEMON);
+	openlog(label, option, LOG_DAEMON);
 
-	syslog(LOG_INFO, "OBEX daemon %s", VERSION);
+	syslog(LOG_INFO, "%s daemon %s", label, VERSION);
 }
 
 void __obex_log_cleanup(void)
diff --git a/src/log.h b/src/log.h
index 1bf1b05..e322565 100644
--- a/src/log.h
+++ b/src/log.h
@@ -26,7 +26,7 @@ void error(const char *format, ...) __attribute__((format(printf, 1, 2)));
 
 void obex_debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
 
-void __obex_log_init(const char *debug, int detach);
+void __obex_log_init(const char *label, const char *debug, int detach);
 void __obex_log_cleanup(void);
 void __obex_log_enable_debug(void);
 
diff --git a/src/main.c b/src/main.c
index 14e7d16..1e78615 100644
--- a/src/main.c
+++ b/src/main.c
@@ -218,7 +218,7 @@ int main(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	__obex_log_init(option_debug, option_detach);
+	__obex_log_init("obexd", option_debug, option_detach);
 
 	DBG("Entering main loop");
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH 1/3] Fix possible crash when processing session callback
From: Luiz Augusto von Dentz @ 2010-12-07 15:00 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

If the callback removes the pending data it cause this:

==20639== Invalid read of size 4
==20639==    at 0x80553E9: free_pending (session.c:112)
==20639==    by 0x8056C83: session_request_reply (session.c:837)
==20639==    by 0x412F7E0: ??? (in /lib/libdbus-1.so.3.5.2)
==20639==    by 0x411D975: ??? (in /lib/libdbus-1.so.3.5.2)
==20639==    by 0x4120B81: dbus_connection_dispatch (in /lib/libdbus-1.so.3.5.2)
==20639==    by 0x804C27F: message_dispatch (mainloop.c:80)
==20639==    by 0x407EFCB: ??? (in /lib/libglib-2.0.so.0.2600.1)
==20639==    by 0x407E854: g_main_context_dispatch (in /lib/libglib-2.0.so.0.2600.1)
==20639==    by 0x4082667: ??? (in /lib/libglib-2.0.so.0.2600.1)
==20639==    by 0x4082BA6: g_main_loop_run (in /lib/libglib-2.0.so.0.2600.1)
==20639==    by 0x8055171: main (main.c:625)
==20639==  Address 0x4363c88 is 0 bytes inside a block of size 12 free'd
==20639==    at 0x40257ED: free (vg_replace_malloc.c:366)
==20639==    by 0x4087485: g_free (in /lib/libglib-2.0.so.0.2600.1)
==20639==    by 0x80553FE: free_pending (session.c:115)
==20639==    by 0x805543C: agent_free (session.c:127)
==20639==    by 0x80566A6: session_free (session.c:149)
==20639==    by 0x8056BCA: session_terminate_transfer (session.c:914)
==20639==    by 0x8056F61: session_prepare_put (session.c:1397)
==20639==    by 0x8056C74: session_request_reply (session.c:835)
==20639==    by 0x412F7E0: ??? (in /lib/libdbus-1.so.3.5.2)
==20639==    by 0x411D975: ??? (in /lib/libdbus-1.so.3.5.2)
==20639==    by 0x4120B81: dbus_connection_dispatch (in /lib/libdbus-1.so.3.5.2)
==20639==    by 0x804C27F: message_dispatch (mainloop.c:80)

To fix this agent->pending is now reset to NULL before calling the
callback, so even if the session is terminated it won't cause a free to
pending data, which is fine since it is latter freed on callback return.
---
 client/session.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/client/session.c b/client/session.c
index 223a2d3..88eae8d 100644
--- a/client/session.c
+++ b/client/session.c
@@ -832,10 +832,11 @@ static void session_request_reply(DBusPendingCall *call, gpointer user_data)
 		pending->transfer->name = g_strdup(name);
 	}
 
+	agent->pending = NULL;
+
 	pending->cb(session, NULL, pending->transfer);
 	dbus_message_unref(reply);
 	free_pending(pending);
-	agent->pending = NULL;
 
 	return;
 }
-- 
1.7.1


^ permalink raw reply related

* RE: [RFC] AVRCP 1.4 : Design proposal for future on Target Role
From: tim.howes @ 2010-12-07 10:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <AANLkTin+K_AYeF9+t0qWWFRJrW5uv9OM4DGY8a2Aaaek@mail.gmail.com>

Hi Shivendra,

> AVRCP 1.4
> ==========
> 
> - Add SDP record for AVRCP 1.4 to inform CT of the version and features
>   supported by target.
> 
>   Priority: Medium
>   Complexity: C1

One extra issue here is that the SDP record needs to vary according to the players running: if the AVRCP layer statically set the AVRCP version in the SDP record to be 1.4, AND the player(s) was a legacy pre-1.4 application that did not know about browsing then the device integrator would have compliance issues.

So I believe there is a need for an API that the player would set if it can support mandatory v.1.4 target features.  If the API is not called (eg by backwards compatibility) then the SDP record would continue to publish version 1.3.

This may increase your complexity value.

> 		As per spec section 6.6.1     GetElementAttributes
> 		As per spec section 6.10.4.3    GetItemAttributes

Note that by overall spec design these are very similar.  Indeed, according to 6.10.4.3, AVRCP 1.4 CTs shall only use GIA when communicating with 1.4 TGs - GEA is effectively deprecated (of course a 1.4 TG may see a GEA from a legacy CT).  I believe it would assist media players if these operations were multiplexed through one API to hide their underlying differences.  It would be annoying for players to have to implement two callbacks to process these two similar operations.

This may change (decrease?) your complexity for these.


Cheers

Tim


This message is for the designated recipient only and may contain privileged, proprietary, or otherwise private information.  If you have received it in error, please notify the sender immediately and delete the original.  Any other use of the email by you is prohibited.

^ permalink raw reply

* Re: [RFC] AVRCP 1.4 : Design proposal for future on Target Role
From: Shivendra Agrawal @ 2010-12-07  8:52 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <AANLkTi=_1qAiH9o_4eBy66OQ9=ba=4GuatVBVtcJG5=x@mail.gmail.com>

> I guess for all people that are interested in AVRCP 1.4 we can start
> adding items to TODO list, to make it easier to share development
> between us. Since this may be too much to handle in a single global
> TODO maybe we can have a separated file like audio/TODO.
>
Somehow I was unable to send the file with TODO list for AVRCP 1.4
So I have copied here for initial feedback from all.
If this needs any modifications please let me know and I would come
back with updates asap.

Background
==========

- Priority scale: High, Medium and Low

- Complexity scale: C1, C2, C4 and C8.  The complexity scale is exponential,
  with complexity 1 being the lowest complexity.  Complexity is a function
  of both task 'complexity' and task 'scope'.

  The general rule of thumb is that a complexity 1 task should take 1-2 weeks
  for a person very familiar with BlueZ codebase.  Higher complexity tasks
  require more time and have higher uncertainty.

  Higher complexity tasks should be refined into several lower complexity tasks
  once the task is better understood.

AVRCP 1.4
==========

- Add SDP record for AVRCP 1.4 to inform CT of the version and features
  supported by target.

  Priority: Medium
  Complexity: C1

- Add callback to accept browsing channel establishment.

  Priority: Medium
  Complexity: C1

- Enhancing bt_io_listen for browsing PSM

  Priority: Medium
  Complexity: C2

- Features to be added

	All below mentioned features will have
		Priority: Medium
		Complexity: C2

		As per spec section 6.4.1     GetCapabilities
		As per spec section 6.5.1     ListPlayerApplicationSettingAttributes
		As per spec section 6.5.2     ListPlayerApplicationSettingValues
		As per spec section 6.5.3     GetCurrentPlayerApplicationSettingValue
		As per spec section 6.5.4     SetPlayerApplicationSettingValue
		As per spec section 6.5.5     GetPlayerApplicationSettingAttributeText
		As per spec section 6.5.6     GetPlayerApplicationSettingValueText
		As per spec section 6.5.7     InformDisplayableCharacterSet
		As per spec section 6.5.8     InformBatteryStatusOfCT
		As per spec section 6.6.1     GetElementAttributes
		As per spec section 6.7.1     GetPlayStatus
		As per spec section 6.7.2     RegisterNotification
		As per spec section 6.8.1     RequestContinuingResponse
		As per spec section 6.8.2     AbortContinuingResponse
		As per spec section 6.9.1     SetAddressedPlayer
		As per spec section 6.9.2     Addressed Player Changed Notification
		As per spec section 6.9.3     SetBrowsedPlayer
		As per spec section 6.9.4     Available Players Changed Notification
		As per spec section 6.9.5     Notify Now Playing Content Changed
		As per spec section 6.10.3.3    UIDs Changed Notification
		As per spec section 6.10.4.1    ChangePath
		As per spec section 6.10.4.2    GetFolderItems
		As per spec section 6.10.4.3    GetItemAttributes
		As per spec section 6.12.1      PlayItem
		As per spec section 6.13.1      Absolute Volume
		As per spec section 6.13.3      Notify volume change

^ permalink raw reply

* Re: EVT_NUM_COMP_PKTS and LE, GATT and SM
From: Ville Tervo @ 2010-12-07  8:47 UTC (permalink / raw)
  To: ext Brian Gix; +Cc: linux-bluetooth
In-Reply-To: <002501cb95a6$b350fc50$19f2f4f0$@org>

Hi Brian,

On Mon, Dec 06, 2010 at 04:35:51PM -0800, ext Brian Gix wrote:
> Hi All,
> 
> I've have encountered a problem while using the gatttool, where my write
> commands get clobbered by the LE ACL being disconnected prior to the ATT
> (fixed channel 4) WRITE_CMD being sent over the LE based ACL link.
> 
> I believe this is fundamentally due to there being no dependency on the
> EVT_NUM_COMP_PKTS event when gatttool decides that the WRITE_CMD has been
> successfully sent.  There is multiple parts to this.
> 
> 1. In User space, the WRITE_CMD pkt is written to the socket.  Gatttool
> erroneously considers a successful socket write as completion, disconnects
> the socket and exits.
> 
> 2. In Kernel space, the ACL packet is added to an ACL queue, which is
> separate from the CMD queue, which can allow either the Disconnect request,
> or the ACL packet to be sent over the H4 link to the baseband.
> 

And in usb case CMD and ACL are even using different endpoints.

> 3. In the baseband, due to LE clocking (and possible other baseband
> activity) the ACL packet could be received first, and the Disconnect CMD
> second, and still result in the connection being detached prior to Tx of the
> ACL packet containing the ATT WRITE_CMD.
> 
> This is not an issue with  any of the ATT READ/FIND/MTU or WRITE_REQ
> transactions, because they require a response from the server.  I believe
> for ATT, this problem is restricted to the WRITE_CMD only, due to it's
> unacknowledged nature.

True.

> However, this will also be an issue with the LE Security Manager, because as
> stated in the Core Spec v4.0, Vol 3, in the last paragraph of 3.6.1 Key
> Distribution on page 630 (of 656):
> 
> 	> Key distribution is complete in the device sending the final key
> when it receives
> 	> the baseband acknowledgement for that key and is complete in the
> receiving
> 	> device when it receives the final key being distributed.
> 
> This is intended to prevent exactly the kind of problem I am experiencing
> with the ATT WRITE_CMD, and the acknowledgement from the baseband can only
> be the EVT_NUM_COMP_PKTS event.
> 
> While talking to my colleagues here, we were thinking that the cleanest
> method to get this accomplished would be by using the "select" method with
> the ATT socket, where the socket could be marked as non-writable by the
> kernel driver until the EVT_NUM_COMP_PKTS is received. The User space code
> could then wait for the socket to become writeable before issuing the socket
> disconnect.
> 

How about implement waiting on l2cap_sock_shutdown() like for ERTM.  User space
could then call shutdown() before closing the socket so make sure the data is
sent or timeout occurred.

Would this be enough to solve the problem?

> If the Security manager is totally within the kernel, it probably does not
> have to do as much work, however it does still need to wait on the
> EVT_NUM_COMP_PKTS before disconnecting the remote device.
> 
> Has anybody else observed this issue with ATT WRITE_CMD? It could be getting
> exacerbated by slow (115Kbps) H4 links that I am using, however the hcidump
> tool confirms that the disconnect happens prior to the EVT_NUM_COMP_PKTS.

I have seen this problem but didn't dig it deeper.

-- 
Ville

^ permalink raw reply

* [PATCH 1/1] bluetooth: add NULL pointer check in hci
From: Jun Nie @ 2010-12-07  7:01 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: Brian Gix, linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 40 bytes --]

Resend it to fix checkpatch.pl warning.

[-- Attachment #2: 0001-bluetooth-add-NULL-pointer-check-in-hci.patch --]
[-- Type: text/x-diff, Size: 841 bytes --]

From 75dc111b5d9f62619bbeec803b15e84412ae050e Mon Sep 17 00:00:00 2001
From: Jun Nie <njun@marvell.com>
Date: Tue, 7 Dec 2010 14:03:38 +0800
Subject: [PATCH] bluetooth: add NULL pointer check in hci

Signed-off-by: Jun Nie <njun@marvell.com>
---
 drivers/bluetooth/hci_ldisc.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 7201482..3c6cabc 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -311,8 +311,10 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 
 		if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
 			hu->proto->close(hu);
-			hci_unregister_dev(hdev);
-			hci_free_dev(hdev);
+			if (hdev) {
+				hci_unregister_dev(hdev);
+				hci_free_dev(hdev);
+			}
 		}
 	}
 }
-- 
1.7.0.4


^ permalink raw reply related

* add NULL pointer check in hci
From: Jun Nie @ 2010-12-07  6:16 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: Brian Gix, linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

>From 6e396f74efeb13e468804c19249424cb3a5f93d8 Mon Sep 17 00:00:00 2001
From: Jun Nie <njun@marvell.com>
Date: Tue, 7 Dec 2010 14:03:38 +0800
Subject: [PATCH] bluetooth: add NULL pointer check in hci

Signed-off-by: Jun Nie <njun@marvell.com>
---
 drivers/bluetooth/hci_ldisc.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 7201482..b58936c 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -311,8 +311,10 @@ static void hci_uart_tty_close(struct tty_struct *tty)

 		if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
 			hu->proto->close(hu);
-			hci_unregister_dev(hdev);
-			hci_free_dev(hdev);
+			if(hdev) {
+				hci_unregister_dev(hdev);
+				hci_free_dev(hdev);
+			}
 		}
 	}
 }
-- 
1.7.0.4

[-- Attachment #2: 0001-bluetooth-add-NULL-pointer-check-in-hci.patch --]
[-- Type: text/x-diff, Size: 840 bytes --]

From 6e396f74efeb13e468804c19249424cb3a5f93d8 Mon Sep 17 00:00:00 2001
From: Jun Nie <njun@marvell.com>
Date: Tue, 7 Dec 2010 14:03:38 +0800
Subject: [PATCH] bluetooth: add NULL pointer check in hci

Signed-off-by: Jun Nie <njun@marvell.com>
---
 drivers/bluetooth/hci_ldisc.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 7201482..b58936c 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -311,8 +311,10 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 
 		if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
 			hu->proto->close(hu);
-			hci_unregister_dev(hdev);
-			hci_free_dev(hdev);
+			if(hdev) {
+				hci_unregister_dev(hdev);
+				hci_free_dev(hdev);
+			}
 		}
 	}
 }
-- 
1.7.0.4


^ permalink raw reply related

* RE: EVT_NUM_COMP_PKTS and LE, GATT and SM
From: Brian Gix @ 2010-12-07  0:46 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <002501cb95a6$b350fc50$19f2f4f0$@org>

Additional point--

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Brian Gix
> Sent: 06 December, 2010 4:36 PM
> To: linux-bluetooth@vger.kernel.org
> Subject: EVT_NUM_COMP_PKTS and LE, GATT and SM
> 
> Hi All,
> 
> I've have encountered a problem while using the gatttool, where my
> write
> commands get clobbered by the LE ACL being disconnected prior to the
> ATT
> (fixed channel 4) WRITE_CMD being sent over the LE based ACL link.
> 
> I believe this is fundamentally due to there being no dependency on the
> EVT_NUM_COMP_PKTS event when gatttool decides that the WRITE_CMD has
> been
> successfully sent.  There is multiple parts to this.
> 
> 1. In User space, the WRITE_CMD pkt is written to the socket.  Gatttool
> erroneously considers a successful socket write as completion,
> disconnects
> the socket and exits.
> 
> 2. In Kernel space, the ACL packet is added to an ACL queue, which is
> separate from the CMD queue, which can allow either the Disconnect
> request,
> or the ACL packet to be sent over the H4 link to the baseband.
> 
> 3. In the baseband, due to LE clocking (and possible other baseband
> activity) the ACL packet could be received first, and the Disconnect
> CMD
> second, and still result in the connection being detached prior to Tx
> of the
> ACL packet containing the ATT WRITE_CMD.
> 
> This is not an issue with  any of the ATT READ/FIND/MTU or WRITE_REQ
> transactions, because they require a response from the server.  I
> believe
> for ATT, this problem is restricted to the WRITE_CMD only, due to it's
> unacknowledged nature.
> 
> However, this will also be an issue with the LE Security Manager,
> because as
> stated in the Core Spec v4.0, Vol 3, in the last paragraph of 3.6.1 Key
> Distribution on page 630 (of 656):
> 
> 	> Key distribution is complete in the device sending the final
> key
> when it receives
> 	> the baseband acknowledgement for that key and is complete in
> the
> receiving
> 	> device when it receives the final key being distributed.
> 
> This is intended to prevent exactly the kind of problem I am
> experiencing
> with the ATT WRITE_CMD, and the acknowledgement from the baseband can
> only
> be the EVT_NUM_COMP_PKTS event.
> 
> While talking to my colleagues here, we were thinking that the cleanest
> method to get this accomplished would be by using the "select" method
> with the ATT socket, where the socket could be marked as non-writable
> by the kernel driver until the EVT_NUM_COMP_PKTS is received. The User
> space code could then wait for the socket to become writeable before
> issuing the socket disconnect.

This solution could be isolated to LE links (and perhaps even fixed
channel 4) because of the strict cmd-resp nature of ATT transactions.
This would cause the inability to queue multiple WRITE_CMDS before
disconnecting however. 


> If the Security manager is totally within the kernel, it probably does
> not
> have to do as much work, however it does still need to wait on the
> EVT_NUM_COMP_PKTS before disconnecting the remote device.
> 
> Has anybody else observed this issue with ATT WRITE_CMD? It could be
> getting
> exacerbated by slow (115Kbps) H4 links that I am using, however the
> hcidump
> tool confirms that the disconnect happens prior to the
> EVT_NUM_COMP_PKTS.
> 
> 
> 
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* EVT_NUM_COMP_PKTS and LE, GATT and SM
From: Brian Gix @ 2010-12-07  0:35 UTC (permalink / raw)
  To: linux-bluetooth

Hi All,

I've have encountered a problem while using the gatttool, where my write
commands get clobbered by the LE ACL being disconnected prior to the ATT
(fixed channel 4) WRITE_CMD being sent over the LE based ACL link.

I believe this is fundamentally due to there being no dependency on the
EVT_NUM_COMP_PKTS event when gatttool decides that the WRITE_CMD has been
successfully sent.  There is multiple parts to this.

1. In User space, the WRITE_CMD pkt is written to the socket.  Gatttool
erroneously considers a successful socket write as completion, disconnects
the socket and exits.

2. In Kernel space, the ACL packet is added to an ACL queue, which is
separate from the CMD queue, which can allow either the Disconnect request,
or the ACL packet to be sent over the H4 link to the baseband.

3. In the baseband, due to LE clocking (and possible other baseband
activity) the ACL packet could be received first, and the Disconnect CMD
second, and still result in the connection being detached prior to Tx of the
ACL packet containing the ATT WRITE_CMD.

This is not an issue with  any of the ATT READ/FIND/MTU or WRITE_REQ
transactions, because they require a response from the server.  I believe
for ATT, this problem is restricted to the WRITE_CMD only, due to it's
unacknowledged nature.

However, this will also be an issue with the LE Security Manager, because as
stated in the Core Spec v4.0, Vol 3, in the last paragraph of 3.6.1 Key
Distribution on page 630 (of 656):

	> Key distribution is complete in the device sending the final key
when it receives
	> the baseband acknowledgement for that key and is complete in the
receiving
	> device when it receives the final key being distributed.

This is intended to prevent exactly the kind of problem I am experiencing
with the ATT WRITE_CMD, and the acknowledgement from the baseband can only
be the EVT_NUM_COMP_PKTS event.

While talking to my colleagues here, we were thinking that the cleanest
method to get this accomplished would be by using the "select" method with
the ATT socket, where the socket could be marked as non-writable by the
kernel driver until the EVT_NUM_COMP_PKTS is received. The User space code
could then wait for the socket to become writeable before issuing the socket
disconnect.

If the Security manager is totally within the kernel, it probably does not
have to do as much work, however it does still need to wait on the
EVT_NUM_COMP_PKTS before disconnecting the remote device.

Has anybody else observed this issue with ATT WRITE_CMD? It could be getting
exacerbated by slow (115Kbps) H4 links that I am using, however the hcidump
tool confirms that the disconnect happens prior to the EVT_NUM_COMP_PKTS.


	
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum




^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Arnd Bergmann @ 2010-12-06 23:07 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij, linux-bluetooth,
	linux-kernel, Marcel Holtmann
In-Reply-To: <AANLkTin_VOjz=bGCnANgp8G2SFFFOkT3TKRh50p5L1Gj@mail.gmail.com>

On Monday 06 December 2010 22:24:34 Vitaly Wool wrote:
> On Mon, Dec 6, 2010 at 5:54 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> >> But I was trying to make a different point here. On a basic level,
> >> there's this cg2000 chip from STE that does BT, FM and GPS. There's
> >> the chip from TI that does BT, FM and GPS, and there's the Broadcom
> >> chip that does BT+FM. They all use HCI to access the other functions
> >> of the combo chip and they do it in a really simiar way, with the
> >> differences mostly in power management techniques. So I think it's
> >> quite sensible to have some kind of framework that is suitable for
> >> such devices.
> >
> > Yes, I agree 100% in principle. I could not find the code that
> > Broadcom/TI FM and GPS stuff so far, can you point us to that?
> 
> Sure, the TI "shared transport" code is mostly at drivers/misc/ti-st.
> 
> Some Broadcom BCM43xx chips work in a similar way AFAIK but I'm not
> sure about the mainline support for those.

Ah, it had not occured to me to look in drivers/misc. Looking at the
code over there, it becomes much clearer what your point is. Evidently
the ti-st driver implements a line discipline similar to, but conflicting
with hci_ldisc, and it has its own copy of the hci_ll code.

I guess this is exactly what we're trying to avoid having with the
cg2900 code ;-).

> > The cg2900 solution for this was to use MFD (plus another layer
> > in the posted version, but that will go away I assume). Using
> > MFD is not the only possibility here, but I could not see anything
> > wrong with it either. Do you think we can move them all over to
> > use MFD infrastructure?
> 
> I guess so but it's probably more of a detail than what I'm up to now :)

Agreed.
 
> >> But generally speaking, isn't a line discipline/driver attached to a
> >> tty? We can use dumb tty for e. g. SPI and still be able to use
> >> hci_ll, right?
> >
> > I suggested that as well, but the point was made that this would
> > add an unnecessary indirection for the SPI case, which is not
> > really much like a serial port. It's certainly possible to do it
> > like you say, but if we add a way to register the high-level
> > protocols with an HCI-like multi-function device, we could
> > also do it in a way that does not rely on tty-ldisc but keeps it
> > as one of the options.
> 
> I actually don't think it's such a big indirection, I prefer to think
> of it more as of the abstraction layer. If not use this, are we going
> to have direct SPI device drivers? I'm afraid we might end up with a
> huge duplication of code in that case.

The question is how it should be modeled in a better way than today.

I believe we already agree it should be something like

   bt   ti-radio    st-e-radio    ti-gps   st-e-gps  broadcom-radio ...
   |         |          |            |          |          |         |
   +---------+----------+---------+--+----------+----------+---------+
                                  |
                          common-hci-module
                                  |
                      +-----------+-----------+
                      |        |       |      |
                    serial    spi     i2c    ...


Any objections to this?

If you agree, I think we should discuss the alternatives for the common
module. I think you are saying we should make the common module a single
ldisc doing the multiplexing and have all transports register as ttys into
it, right?

What we discussed before was to split it into multiple modules, one for
the tty ldisc and one for the common code, and then have the spi/i2c/...
drivers feed into the common multiplexer without going through tty.

I don't currently see a significant advantage or disadvantage with either
approach.

	Arnd

^ permalink raw reply

* [RFC v2 9/9] Bluetooth: Add support for resuming socket when SMP is finished
From: Vinicius Costa Gomes @ 2010-12-06 21:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1291671832-13435-1-git-send-email-vinicius.gomes@openbossa.org>

This adds support for resuming the user space traffic when SMP
negotiation is complete.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 net/bluetooth/l2cap_core.c |   72 +++++++++++++++++++++++++------------------
 net/bluetooth/smp.c        |    9 +++++
 2 files changed, 51 insertions(+), 30 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index da4f13d..061248f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -624,6 +624,22 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 	}
 }
 
+static void l2cap_chan_ready(struct sock *sk)
+{
+	struct sock *parent = bt_sk(sk)->parent;
+
+	BT_DBG("sk %p, parent %p", sk, parent);
+
+	l2cap_pi(sk)->conf_state = 0;
+	l2cap_sock_clear_timer(sk);
+
+	sk->sk_state = BT_CONNECTED;
+	sk->sk_state_change(sk);
+
+	if (parent)
+		parent->sk_data_ready(parent, 0);
+}
+
 static void l2cap_conn_ready(struct l2cap_conn *conn)
 {
 	struct l2cap_chan_list *l = &conn->chan_list;
@@ -640,14 +656,10 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 		bh_lock_sock(sk);
 
 		if (conn->hcon->type == LE_LINK) {
-			l2cap_sock_clear_timer(sk);
-			sk->sk_state = BT_CONNECTED;
-			sk->sk_state_change(sk);
 			if (smp_conn_security(conn, l2cap_pi(sk)->sec_level))
-				BT_DBG("Insufficient security");
-		}
+				l2cap_chan_ready(sk);
 
-		if (sk->sk_type != SOCK_SEQPACKET &&
+		} else if (sk->sk_type != SOCK_SEQPACKET &&
 				sk->sk_type != SOCK_STREAM) {
 			l2cap_sock_clear_timer(sk);
 			sk->sk_state = BT_CONNECTED;
@@ -1199,7 +1211,7 @@ static int l2cap_do_connect(struct sock *sk)
 				sk->sk_type != SOCK_STREAM) {
 			l2cap_sock_clear_timer(sk);
 			sk->sk_state = BT_CONNECTED;
-		} else
+		} else if (hcon->type == ACL_LINK)
 			l2cap_do_start(sk);
 	}
 
@@ -2162,6 +2174,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 {
 	struct sock *sk = sock->sk;
 	struct bt_security sec;
+	struct l2cap_conn *conn;
 	int len, err = 0;
 	u32 opt;
 
@@ -2198,6 +2211,18 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 		}
 
 		l2cap_pi(sk)->sec_level = sec.level;
+
+		conn = l2cap_pi(sk)->conn;
+		if (conn && conn->hcon->type == LE_LINK) {
+			if (!conn->hcon->out) {
+				err = -EINVAL;
+				break;
+			}
+
+			sk->sk_state = BT_CONFIG;
+			smp_conn_security(conn, sec.level);
+		}
+
 		break;
 
 	case BT_DEFER_SETUP:
@@ -2410,29 +2435,6 @@ static int l2cap_sock_release(struct socket *sock)
 	return err;
 }
 
-static void l2cap_chan_ready(struct sock *sk)
-{
-	struct sock *parent = bt_sk(sk)->parent;
-
-	BT_DBG("sk %p, parent %p", sk, parent);
-
-	l2cap_pi(sk)->conf_state = 0;
-	l2cap_sock_clear_timer(sk);
-
-	if (!parent) {
-		/* Outgoing channel.
-		 * Wake up socket sleeping on connect.
-		 */
-		sk->sk_state = BT_CONNECTED;
-		sk->sk_state_change(sk);
-	} else {
-		/* Incoming channel.
-		 * Wake up socket sleeping on accept.
-		 */
-		parent->sk_data_ready(parent, 0);
-	}
-}
-
 /* Copy frame to all raw sockets on that connection */
 static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
 {
@@ -4753,6 +4755,16 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 	for (sk = l->head; sk; sk = l2cap_pi(sk)->next_c) {
 		bh_lock_sock(sk);
 
+		if (l2cap_pi(sk)->scid == L2CAP_CID_LE_DATA) {
+			if (!status && encrypt) {
+				l2cap_pi(sk)->sec_level = BT_SECURITY_MEDIUM;
+				l2cap_chan_ready(sk);
+			}
+
+			bh_unlock_sock(sk);
+			continue;
+		}
+
 		if (l2cap_pi(sk)->conf_state & L2CAP_CONF_CONNECT_PEND) {
 			bh_unlock_sock(sk);
 			continue;
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index d19b8a2..c7a0e63 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -346,9 +346,13 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
 {
 	struct smp_cmd_security_req *rp = (void *) skb->data;
 	struct smp_cmd_pairing cp;
+	struct hci_conn *hcon = conn->hcon;
 
 	BT_DBG("");
 
+	if (test_bit(HCI_CONN_ENCRYPT_PEND, &hcon->pend))
+		return;
+
 	skb_pull(skb, sizeof(*rp));
 
 	memset(&cp, 0, sizeof(cp));
@@ -364,10 +368,13 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
 	memcpy(&conn->preq[1], &cp, sizeof(cp));
 
 	smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
+
+	set_bit(HCI_CONN_ENCRYPT_PEND, &hcon->pend);
 }
 
 int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
 {
+	struct hci_conn *hcon = conn->hcon;
 	__u8 authreq;
 
 	BT_DBG("conn %p hcon %p level 0x%2.2x", conn, conn->hcon, sec_level);
@@ -407,6 +414,8 @@ int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
 		smp_send_cmd(conn, SMP_CMD_SECURITY_REQ, sizeof(cp), &cp);
 	}
 
+	set_bit(HCI_CONN_ENCRYPT_PEND, &hcon->pend);
+
 	return 0;
 }
 
-- 
1.7.3.2


^ permalink raw reply related

* [RFC v2 8/9] Bluetooth: Add support for LE Start Encryption
From: Vinicius Costa Gomes @ 2010-12-06 21:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1291671832-13435-1-git-send-email-vinicius.gomes@openbossa.org>

This adds support for starting SMP Phase 2 Encryption, when the initial
SMP negotiation is successful. This adds the LE Start Encryption and LE
Long Term Key Request commands and related events.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 include/net/bluetooth/hci.h      |   34 +++++++++++++++++++
 include/net/bluetooth/hci_core.h |    5 +++
 net/bluetooth/hci_conn.c         |   47 ++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |   67 ++++++++++++++++++++++++++++++++++++++
 net/bluetooth/smp.c              |    8 ++++-
 5 files changed, 160 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index dff6ded..e6bed3f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -626,6 +626,33 @@ struct hci_cp_le_create_conn {
 
 #define HCI_OP_LE_CREATE_CONN_CANCEL	0x200e
 
+#define HCI_OP_LE_START_ENC		0x2019
+struct hci_cp_le_start_enc {
+	__le16	handle;
+	__u8	rand[8];
+	__le16	ediv;
+	__u8	ltk[16];
+} __packed;
+
+#define HCI_OP_LE_LTK_REPLY		0x201a
+struct hci_cp_le_ltk_reply {
+	__le16	handle;
+	__u8	ltk[16];
+} __packed;
+struct hci_rp_le_ltk_reply {
+	__u8	status;
+	__le16	handle;
+} __packed;
+
+#define HCI_OP_LE_LTK_NEG_REPLY		0x201b
+struct hci_cp_le_ltk_neg_reply {
+	__le16	handle;
+} __packed;
+struct hci_rp_le_ltk_neg_reply {
+	__u8	status;
+	__le16	handle;
+} __packed;
+
 /* ---- HCI Events ---- */
 #define HCI_EV_INQUIRY_COMPLETE		0x01
 
@@ -897,6 +924,13 @@ struct hci_ev_le_conn_complete {
 	__u8     clk_accurancy;
 } __packed;
 
+#define HCI_EV_LE_LTK_REQ		0x05
+struct hci_ev_le_ltk_req {
+	__le16	handle;
+	__u8	random[8];
+	__le16	ediv;
+} __packed;
+
 /* Internal events generated by Bluetooth stack */
 #define HCI_EV_STACK_INTERNAL	0xfd
 struct hci_ev_stack_internal {
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d0a9f5d..c6c44eb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -192,6 +192,7 @@ struct hci_conn {
 	__u8             sec_level;
 	__u8             power_save;
 	__u16            disc_timeout;
+	__u8		 ltk[16];
 	unsigned long	 pend;
 
 	unsigned int	 sent;
@@ -713,4 +714,8 @@ struct hci_sec_filter {
 
 void hci_req_complete(struct hci_dev *hdev, int result);
 
+void hci_le_start_enc(struct hci_conn *conn, u8 ltk[16]);
+void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
+void hci_le_ltk_neg_reply(struct hci_conn *conn);
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index edfb48b..f919ddb 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -183,6 +183,53 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 	hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
 }
 
+void hci_le_start_enc(struct hci_conn *conn, u8 ltk[16])
+{
+	struct hci_dev *hdev = conn->hdev;
+	struct hci_cp_le_start_enc cp;
+
+	BT_DBG("%p", conn);
+
+	memset(&cp, 0, sizeof(cp));
+
+	cp.handle = cpu_to_le16(conn->handle);
+	memcpy(cp.ltk, ltk, 16);
+
+	hci_send_cmd(hdev, HCI_OP_LE_START_ENC, sizeof(cp), &cp);
+}
+EXPORT_SYMBOL(hci_le_start_enc);
+
+void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16])
+{
+	struct hci_dev *hdev = conn->hdev;
+	struct hci_cp_le_ltk_reply cp;
+
+	BT_DBG("%p", conn);
+
+	memset(&cp, 0, sizeof(cp));
+
+	cp.handle = cpu_to_le16(conn->handle);
+	memcpy(&cp.ltk, ltk, sizeof(ltk));
+
+	hci_send_cmd(hdev, HCI_OP_LE_LTK_REPLY, sizeof(cp), &cp);
+}
+EXPORT_SYMBOL(hci_le_ltk_reply);
+
+void hci_le_ltk_neg_reply(struct hci_conn *conn)
+{
+	struct hci_dev *hdev = conn->hdev;
+	struct hci_cp_le_ltk_neg_reply cp;
+
+	BT_DBG("%p", conn);
+
+	memset(&cp, 0, sizeof(cp));
+
+	cp.handle = cpu_to_le16(conn->handle);
+
+	hci_send_cmd(hdev, HCI_OP_LE_LTK_NEG_REPLY, sizeof(cp), &cp);
+}
+EXPORT_SYMBOL(hci_le_ltk_neg_reply);
+
 /* Device _must_ be locked */
 void hci_sco_setup(struct hci_conn *conn, __u8 status)
 {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 55cdd6a..c90696f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -559,6 +559,30 @@ static void hci_cc_le_read_buffer_size(struct hci_dev *hdev,
 	hci_req_complete(hdev, rp->status);
 }
 
+static void hci_cc_le_ltk_reply(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_rp_le_ltk_reply *rp = (void *) skb->data;
+
+	BT_DBG("%s status 0x%x", hdev->name, rp->status);
+
+	if (rp->status)
+		return;
+
+	hci_req_complete(hdev, rp->status);
+}
+
+static void hci_cc_le_ltk_neg_reply(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_rp_le_ltk_neg_reply *rp = (void *) skb->data;
+
+	BT_DBG("%s status 0x%x", hdev->name, rp->status);
+
+	if (rp->status)
+		return;
+
+	hci_req_complete(hdev, rp->status);
+}
+
 static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 {
 	BT_DBG("%s status 0x%x", hdev->name, status);
@@ -920,6 +944,11 @@ static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
 	hci_dev_unlock(hdev);
 }
 
+static void hci_cs_le_start_enc(struct hci_dev *hdev, u8 status)
+{
+	BT_DBG("%s status 0x%x", hdev->name, status);
+}
+
 static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	__u8 status = *((__u8 *) skb->data);
@@ -1440,6 +1469,14 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
 		hci_cc_le_read_buffer_size(hdev, skb);
 		break;
 
+	case HCI_OP_LE_LTK_REPLY:
+		hci_cc_le_ltk_reply(hdev, skb);
+		break;
+
+	case HCI_OP_LE_LTK_NEG_REPLY:
+		hci_cc_le_ltk_neg_reply(hdev, skb);
+		break;
+
 	default:
 		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
 		break;
@@ -1510,6 +1547,10 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cs_le_create_conn(hdev, ev->status);
 		break;
 
+	case HCI_OP_LE_START_ENC:
+		hci_cs_le_start_enc(hdev, ev->status);
+		break;
+
 	default:
 		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
 		break;
@@ -2013,6 +2054,28 @@ unlock:
 	hci_dev_unlock(hdev);
 }
 
+static inline void hci_le_ltk_request_evt(struct hci_dev *hdev,
+						struct sk_buff *skb)
+{
+	struct hci_ev_le_ltk_req *ev = (void *) skb->data;
+	struct hci_cp_le_ltk_reply cp;
+	struct hci_conn *conn;
+
+	BT_DBG("%s handle %d", hdev->name, cpu_to_le16(ev->handle));
+
+	hci_dev_lock(hdev);
+
+	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
+
+	memset(&cp, 0, sizeof(cp));
+	cp.handle = cpu_to_le16(conn->handle);
+	memcpy(cp.ltk, conn->ltk, sizeof(conn->ltk));
+
+	hci_send_cmd(hdev, HCI_OP_LE_LTK_REPLY, sizeof(cp), &cp);
+
+	hci_dev_unlock(hdev);
+}
+
 static inline void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_le_meta *le_ev = (void *) skb->data;
@@ -2024,6 +2087,10 @@ static inline void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_le_conn_complete_evt(hdev, skb);
 		break;
 
+	case HCI_EV_LE_LTK_REQ:
+		hci_le_ltk_request_evt(hdev, skb);
+		break;
+
 	default:
 		break;
 	}
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 7d7e8ad..d19b8a2 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -289,7 +289,8 @@ static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb
 
 static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 {
-	struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
+	struct hci_conn *hcon = conn->hcon;
+	struct crypto_blkcipher *tfm = hcon->hdev->tfm;
 	int ret;
 	u8 k[16], key[16], res[16], random[16], confirm[16], buf[128];
 
@@ -297,6 +298,7 @@ static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 	skb_pull(skb, 16);
 
 	memset(k, 0, sizeof(k));
+	memset(hcon->ltk, 0, sizeof(hcon->ltk));
 
 	if (conn->hcon->out)
 		ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
@@ -320,6 +322,9 @@ static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 
 	if (conn->hcon->out) {
 		smp_s1(tfm, k, random, conn->prnd, key);
+		swap128(key, hcon->ltk);
+
+		hci_le_start_enc(conn->hcon, hcon->ltk);
 
 		hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
 		BT_DBG("key %s", buf);
@@ -330,6 +335,7 @@ static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, r);
 
 		smp_s1(tfm, k, conn->prnd, random, key);
+		swap128(key, hcon->ltk);
 
 		hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
 		BT_DBG("key %s", buf);
-- 
1.7.3.2


^ permalink raw reply related

* [RFC v2 7/9] Bluetooth: Add support for SMP confirmation checks
From: Vinicius Costa Gomes @ 2010-12-06 21:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1291671832-13435-1-git-send-email-vinicius.gomes@openbossa.org>

This adds supports for verifying the confirmation value that the
remote side has sent. This includes support for generating and sending
the random value used to produce the confirmation value.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 include/net/bluetooth/l2cap.h |    5 ++
 net/bluetooth/smp.c           |  121 ++++++++++++++++++++++++++++++++---------
 2 files changed, 101 insertions(+), 25 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index a3cb1ab..bcda2aa 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -290,6 +290,11 @@ struct l2cap_conn {
 
 	__u8		disc_reason;
 
+	__u8		preq[7];
+	__u8		pres[7];
+	__u8		prnd[16];
+	__u8		pcnf[16];
+
 	struct l2cap_chan_list chan_list;
 };
 
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index b62160e..7d7e8ad 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -203,7 +203,9 @@ static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
 
 	BT_DBG("");
 
-	skb_pull(skb, sizeof(struct smp_cmd_pairing));
+	conn->preq[0] = SMP_CMD_PAIRING_REQ;
+	memcpy(&conn->preq[1], rp, sizeof(*rp));
+	skb_pull(skb, sizeof(*rp));
 
 	rp->io_capability = 0x00;
 	rp->oob_flag = 0x00;
@@ -212,64 +214,125 @@ static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
 	rp->resp_key_dist = 0x00;
 	rp->auth_req &= 0x05;
 
+	conn->pres[0] = SMP_CMD_PAIRING_RSP;
+	memcpy(&conn->pres[1], rp, sizeof(rp));
+
 	smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
 }
 
 static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
 {
+	struct smp_cmd_pairing *rp = (void *) skb->data;
 	struct smp_cmd_pairing_confirm cp;
+	struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
+	int ret;
+	u8 k[16], res[16];
 
-	BT_DBG("");
+	/* Just Works */
+	memset(k, 0, sizeof(k));
+
+	conn->pres[0] = SMP_CMD_PAIRING_RSP;
+	memcpy(&conn->pres[1], rp, sizeof(*rp));
+	skb_pull(skb, sizeof(*rp));
+
+	ret = smp_rand(conn->prnd);
+	if (ret)
+		return;
 
-	memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
+	ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
+			conn->src, 0, conn->dst, res);
+	if (ret)
+		return;
+
+	swap128(res, cp.confirm_val);
 
 	smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
 }
 
 static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
 {
+	struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
+
 	BT_DBG("");
 
-	if (conn->hcon->out) {
-		struct smp_cmd_pairing_random random;
+	memcpy(conn->pcnf, skb->data, 16);
+	skb_pull(skb, 16);
 
-		BT_DBG("master");
+	if (conn->hcon->out) {
+		u8 random[16];
 
-		memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
+		swap128(conn->prnd, random);
 
-		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
-								&random);
+		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, random);
 	} else {
-		struct smp_cmd_pairing_confirm confirm;
+		struct smp_cmd_pairing_confirm cp;
+		int ret;
+		u8 k[16], res[16];
+
+		/* Just Works */
+		memset(k, 0, sizeof(k));
 
-		BT_DBG("slave");
+		ret = smp_rand(conn->prnd);
+		if (ret)
+			return;
 
-		memset(&confirm, 0, sizeof(struct smp_cmd_pairing_confirm));
+		ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
+				conn->dst, 0, conn->src, res);
+		if (ret)
+			return;
 
-		smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(confirm),
-								&confirm);
+		swap128(res, cp.confirm_val);
+
+		smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
 	}
 }
 
 static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 {
-	struct smp_cmd_pairing_random cp;
+	struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
+	int ret;
+	u8 k[16], key[16], res[16], random[16], confirm[16], buf[128];
+
+	swap128(skb->data, random);
+	skb_pull(skb, 16);
+
+	memset(k, 0, sizeof(k));
+
+	if (conn->hcon->out)
+		ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
+				conn->src, 0, conn->dst, res);
+	else
+		ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
+				conn->dst, 0, conn->src, res);
+	if (ret)
+		return;
 
-	BT_DBG("");
+	swap128(res, confirm);
 
-	skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
+	if (memcmp(conn->pcnf, confirm, 16) != 0) {
+		struct smp_cmd_pairing_fail cp;
 
-	/* FIXME: check if random matches */
+		BT_ERR("Pairing failed (confirmation values mismatch)");
+		cp.reason = SMP_CONFIRM_FAILED;
+		smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, sizeof(cp), &cp);
+		return;
+	}
 
 	if (conn->hcon->out) {
-		BT_DBG("master");
-		/* FIXME: start encryption */
+		smp_s1(tfm, k, random, conn->prnd, key);
+
+		hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
+		BT_DBG("key %s", buf);
 	} else {
-		BT_DBG("slave");
+		u8 r[16];
 
-		memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
+		swap128(conn->prnd, r);
+		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, r);
 
-		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp), &cp);
+		smp_s1(tfm, k, conn->prnd, random, key);
+
+		hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
+		BT_DBG("key %s", buf);
 	}
 }
 
@@ -280,8 +343,9 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
 
 	BT_DBG("");
 
-	skb_pull(skb, sizeof(struct smp_cmd_security_req));
-	memset(&cp, 0, sizeof(struct smp_cmd_pairing));
+	skb_pull(skb, sizeof(*rp));
+
+	memset(&cp, 0, sizeof(cp));
 
 	cp.io_capability = 0x00;
 	cp.oob_flag = 0x00;
@@ -290,6 +354,9 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
 	cp.resp_key_dist = 0x00;
 	cp.auth_req = rp->auth_req & 0x05;
 
+	conn->preq[0] = SMP_CMD_PAIRING_REQ;
+	memcpy(&conn->preq[1], &cp, sizeof(cp));
+
 	smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
 }
 
@@ -323,6 +390,10 @@ int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
 		cp.init_key_dist = 0x00;
 		cp.resp_key_dist = 0x00;
 		cp.auth_req = authreq;
+
+		conn->preq[0] = SMP_CMD_PAIRING_REQ;
+		memcpy(&conn->preq[1], &cp, sizeof(cp));
+
 		smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
 	} else {
 		struct smp_cmd_security_req cp;
-- 
1.7.3.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox