2012-10-10 16:39:26

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH 1/4] Bluetooth: Fix L2CAP PSM bind issue

Problem: If we bind a particular PSM with source address as BDADDR_ANY,
then we are able to bind the same PSM with adapter address(or any other
address), which is incorrect.

Solution: Added check for comparing the stored source address and
given source address against BDADDR_ANY, so that irrespective of
BADADDR_ANY or any adapter address, a particular PSM will be allowed
to bind only once.

Details: After successful binding, both the PSM and the source device
address are stored in a global list. Before binding to a new PSM, the
old PSM and the source address from the gobal list are compared against
the new PSM and source device address for make the PSM binding unique.

Before this fix, If we pass the different device addresses, for the same
PSM, both the binding are successful. Further an incoming connection to
that particular PSM goes to incorrect profile (first binded). This PSM
binding issues can be easily reproducible using the l2test utility as
shown below.

Bind to a PSM without the device address
-sh-4.1# l2test -w -P 4097
l2test[2792]: Waiting for connection on psm 4097 ...

In another terminal, bind to same PSM with device address
-sh-4.1# l2test -w -P 4097 -i hci0
l2test[2787]: Waiting for connection on psm 4097 ...

Here we can see the binding is successful for both cases for the same
PSM. After this fix the binding for a particular PSM is allowed only
once.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
This patch resolves the HDP incoming connection issue when Obexd is
running in background. Both uses dynamic PSM range. Obex over l2cap
bind with source address as BDADDR_ANY and HDP binds with source address
as the adapter address. Both bindings for the same PSM are getting
success.

net/bluetooth/l2cap_core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d42cdb1..ace7cd8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -113,7 +113,16 @@ static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
struct l2cap_chan *c;

list_for_each_entry(c, &chan_list, global_l) {
- if (c->sport == psm && !bacmp(&bt_sk(c->sk)->src, src))
+ if (c->sport != psm)
+ continue;
+
+ /* Exact match */
+ if (!bacmp(&bt_sk(c->sk)->src, src))
+ return c;
+
+ /* BDADDR_ANY match */
+ if (!bacmp(&bt_sk(c->sk)->src, BDADDR_ANY) ||
+ !bacmp(src, BDADDR_ANY))
return c;
}
return NULL;
--
1.7.9.5



2012-10-11 06:27:07

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 4/4] Bluetooth: Use __constant modifier for RFCOMM PSM

Hi Syam,

* Syam Sidhardhan <[email protected]> [2012-10-10 22:09:29 +0530]:

> Since the RFCOMM_PSM is constant, __constant_cpu_to_le16() is
> the right go here.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/rfcomm/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Patch 3 and 4 have been applied to bluetooth-next. Thanks.

Gustavo

2012-10-10 17:48:49

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 4/4] Bluetooth: Use __constant modifier for RFCOMM PSM

Hi Syam,

> Since the RFCOMM_PSM is constant, __constant_cpu_to_le16() is
> the right go here.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/rfcomm/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-10 17:48:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/4] Bluetooth: Use __constant modifier for L2CAP SMP CID

Hi Syam,

> Since the L2CAP_CID_SMP is constant, __constant_cpu_to_le16() is
> the right go here.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-10 17:47:25

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/4] Bluetooth: Fix L2CAP PSM bind issue

Hi Syam,

> Problem: If we bind a particular PSM with source address as BDADDR_ANY,
> then we are able to bind the same PSM with adapter address(or any other
> address), which is incorrect.

why is this incorrect? Explain that to me.

Regards

Marcel



2012-10-10 16:39:29

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH 4/4] Bluetooth: Use __constant modifier for RFCOMM PSM

Since the RFCOMM_PSM is constant, __constant_cpu_to_le16() is
the right go here.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
net/bluetooth/rfcomm/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index fb1d83d..201fdf7 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -709,7 +709,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,

bacpy(&addr.l2_bdaddr, dst);
addr.l2_family = AF_BLUETOOTH;
- addr.l2_psm = cpu_to_le16(RFCOMM_PSM);
+ addr.l2_psm = __constant_cpu_to_le16(RFCOMM_PSM);
addr.l2_cid = 0;
*err = kernel_connect(sock, (struct sockaddr *) &addr, sizeof(addr), O_NONBLOCK);
if (*err == 0 || *err == -EINPROGRESS)
@@ -1987,7 +1987,7 @@ static int rfcomm_add_listener(bdaddr_t *ba)
/* Bind socket */
bacpy(&addr.l2_bdaddr, ba);
addr.l2_family = AF_BLUETOOTH;
- addr.l2_psm = cpu_to_le16(RFCOMM_PSM);
+ addr.l2_psm = __constant_cpu_to_le16(RFCOMM_PSM);
addr.l2_cid = 0;
err = kernel_bind(sock, (struct sockaddr *) &addr, sizeof(addr));
if (err < 0) {
--
1.7.9.5


2012-10-10 16:39:28

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH 3/4] Bluetooth: Use __constant modifier for L2CAP SMP CID

Since the L2CAP_CID_SMP is constant, __constant_cpu_to_le16() is
the right go here.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
net/bluetooth/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 8c225ef..9b54fea 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -165,7 +165,7 @@ static struct sk_buff *smp_build_cmd(struct l2cap_conn *conn, u8 code,

lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
lh->len = cpu_to_le16(sizeof(code) + dlen);
- lh->cid = cpu_to_le16(L2CAP_CID_SMP);
+ lh->cid = __constant_cpu_to_le16(L2CAP_CID_SMP);

memcpy(skb_put(skb, sizeof(code)), &code, sizeof(code));

--
1.7.9.5


2012-10-10 16:39:27

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH 2/4] Bluetooth: Fix RFCOMM bind issue

Problem: If we bind a particular RFCOMM channel with source address as
BDADDR_ANY, then we are able to bind the same RFCOMM channel with the
adapter address, which is incorrect.

Solution: Add check for comparing the stored source address and
given source address against BDADDR_ANY, so that irrespective of
BADADDR_ANY or any adapter address, a particular RFCOMM channel will
be allowed to bind only once.

Details:
Here given the steps to reproduce this issue:
Bind to a RFCOMM channel without the device address.
-sh-4.1# rctest -w -P 5
rctest[2920]: Waiting for connection on channel 5 ...

In another terminal, bind to same RFCOMM channel with a device address
-sh-4.1# rctest -w -P 5 -i hci0
rctest[2922]: Waiting for connection on channel 5 ...

Here we can see the binding is successful for both cases for the same
RFCOMM channel. After this fix, the binding for a particular RFCOMM
channel is allowed only once.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 867a065..4add776 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -110,8 +110,16 @@ static struct sock *__rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
struct hlist_node *node;

sk_for_each(sk, node, &rfcomm_sk_list.head) {
- if (rfcomm_pi(sk)->channel == channel &&
- !bacmp(&bt_sk(sk)->src, src))
+ if (rfcomm_pi(sk)->channel != channel)
+ continue;
+
+ /* Exact match */
+ if (!bacmp(&bt_sk(sk)->src, src))
+ break;
+
+ /* BDADDR_ANY match */
+ if (!bacmp(&bt_sk(sk)->src, BDADDR_ANY) ||
+ !bacmp(src, BDADDR_ANY))
break;
}

--
1.7.9.5