2012-04-16 07:58:27

by Mikel Astiz

Subject: [PATCH v2 0/1] Fixing EADDRINUSE in SCO sockets

This patch moves the EADDRINUSE check in SCO sockets from bind() to
listen(), as described in the commit message.

Note that the address matching to detect collisions is very
conservative: BDADDR_ANY will match any other address. This seems to
make sense but differs from the original approach in bind().

This second version includes the review from Johan: the helper function
has been renamed and bool is used instead of int.

Another approach, as proposed by Luiz, would be to use a helper function
that, instead of returning a bool, returns the socket that is using the
matching address. This might be useful in the future, but I personally
don't think it is very likely.

The code could be as follows:

static struct sock *__sco_get_sock_by_addr(int state, bdaddr_t *ba)
{
struct sock *sk = NULL, *sk1 = NULL;
struct hlist_node *node;

sk_for_each(sk, node, &sco_sk_list.head) {
if (sk->sk_state != state)
continue;

/* Exact match. */
if (!bacmp(&bt_sk(sk)->src, src))
break;

/* Closest match */
if (!bacmp(&bt_sk(sk)->src, BDADDR_ANY))
sk1 = sk;

/* Match any if given src is BDADDR_ANY */
if (!bacmp(src, BDADDR_ANY))
sk1 = sk;
}

return node ? sk : sk1;
}

This function would be quite similar from sco_get_sock_listen() with the
exception of the last BDADDR_ANY match, and the lock handling.

Cheers,
Mikel

Mikel Astiz (1):
Bluetooth: Fix EADDRINUSE check in SCO sockets

net/bluetooth/sco.c | 49 ++++++++++++++++++++++++++++++-------------------
1 files changed, 30 insertions(+), 19 deletions(-)

--
1.7.7.6

2012-04-16 07:58:28

by Mikel Astiz

Subject: [PATCH v2 1/1] Bluetooth: Fix EADDRINUSE check in SCO sockets

The EADDRINUSE error should be returned only when the SCO socket is
being used as a server. This means it should be checked in
sco_sock_listen() instead of in sco_sock_bind(), because in the later we
can't know if it is a server or not.

This patch is required in order to use multiple SCO connections in the
same Bluetooth adapter.

Signed-off-by: Mikel Astiz <[email protected]>
---
net/bluetooth/sco.c | 49 ++++++++++++++++++++++++++++++-------------------
1 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 8bf26d1..45bedd3 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -278,17 +278,30 @@ drop:
}

/* -------- Socket interface ---------- */
-static struct sock *__sco_get_sock_by_addr(bdaddr_t *ba)
+static bool __sco_addr_in_use(bdaddr_t *src)
{
struct sock *sk;
struct hlist_node *node;

- sk_for_each(sk, node, &sco_sk_list.head)
- if (!bacmp(&bt_sk(sk)->src, ba))
- goto found;
- sk = NULL;
-found:
- return sk;
+ sk_for_each(sk, node, &sco_sk_list.head) {
+ if (sk->sk_state != BT_LISTEN)
+ continue;
+
+ /* Exact match */
+ if (!bacmp(&bt_sk(sk)->src, src))
+ return true;
+
+ /* Closest match */
+ if (!bacmp(&bt_sk(sk)->src, BDADDR_ANY))
+ return true;
+
+ /* Match any if given src is BDADDR_ANY */
+ if (!bacmp(src, BDADDR_ANY))
+ return true;
+ }
+
+ /* No match found so address is free */
+ return false;
}

/* Find socket listening on source bdaddr.
@@ -467,7 +480,6 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
{
struct sockaddr_sco *sa = (struct sockaddr_sco *) addr;
struct sock *sk = sock->sk;
- bdaddr_t *src = &sa->sco_bdaddr;
int err = 0;

BT_DBG("sk %p %s", sk, batostr(&sa->sco_bdaddr));
@@ -482,17 +494,9 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
goto done;
}

- write_lock(&sco_sk_list.lock);
-
- if (bacmp(src, BDADDR_ANY) && __sco_get_sock_by_addr(src)) {
- err = -EADDRINUSE;
- } else {
- /* Save source address */
- bacpy(&bt_sk(sk)->src, &sa->sco_bdaddr);
- sk->sk_state = BT_BOUND;
- }
-
- write_unlock(&sco_sk_list.lock);
+ /* Save source address */
+ bacpy(&bt_sk(sk)->src, &sa->sco_bdaddr);
+ sk->sk_state = BT_BOUND;

done:
release_sock(sk);
@@ -543,17 +547,24 @@ static int sco_sock_listen(struct socket *sock, int backlog)
BT_DBG("sk %p backlog %d", sk, backlog);

lock_sock(sk);
+ write_lock(&sco_sk_list.lock);

if (sk->sk_state != BT_BOUND || sock->type != SOCK_SEQPACKET) {
err = -EBADFD;
goto done;
}

+ if (__sco_addr_in_use(&bt_sk(sk)->src)) {
+ err = -EADDRINUSE;
+ goto done;
+ }
+
sk->sk_max_ack_backlog = backlog;
sk->sk_ack_backlog = 0;
sk->sk_state = BT_LISTEN;

done:
+ write_unlock(&sco_sk_list.lock);
release_sock(sk);
return err;
}
--
1.7.7.6