2012-02-22 15:11:54

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv3 0/3] Bluetooth: Change socket lock to l2cap_chan lock

From: Andrei Emeltchenko <[email protected]>

Changing socket lock to L2CAP chan lock in L2CAP code. Needed for implementing
protocols above L2CAP without creating sockets.

Changes:
* PATCHv3: Do not remove couple of empty lines and remove old comments.
* PATCHv2: Rebase remaining parts against latest tree and merge all
patches dealing with converting sk lock to chan lock together following
recommendation from Ulisses and Gustavo.
* PATCHv1: Added extra line (per Gustavo comment)
* RFCv6: Same code but patches 2,3 and 4 from RFCv5 are merged together
following recommendations from review.
* RFCv5: Fixed locking bug in l2cap_data_channel, added locks in
l2cap_sock_shutdown function, fixed several styles issues.
* RFCv4: Better split patches so they looks more clear and obvious,
taking coments about naming change and delete unused vars. See diff change
from the previous version below:
* RFCv3: Split the big patch to several small (I believe logical) chunks,
remove unneded locks from cleanup_listen, use the same arguments for
locked/unlocked socket error functions.
* RFCv2: Convert l2cap channel list back to mutex from RCU list.

Andrei Emeltchenko (3):
Bluetooth: Add unlocked __l2cap_chan_add function
Bluetooth: Change sk lock to chan lock in L2CAP core
Bluetooth: Remove socket lock check

net/bluetooth/l2cap_core.c | 175 ++++++++++++++++++++++++++------------------
net/bluetooth/l2cap_sock.c | 26 +++++--
2 files changed, 125 insertions(+), 76 deletions(-)

--
1.7.9



2012-02-22 22:42:56

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCHv3 0/3] Bluetooth: Change socket lock to l2cap_chan lock

Hi Andrei,

On Wed, Feb 22, 2012, Andrei Emeltchenko wrote:
> Changing socket lock to L2CAP chan lock in L2CAP code. Needed for implementing
> protocols above L2CAP without creating sockets.
>
> Changes:
> * PATCHv3: Do not remove couple of empty lines and remove old comments.
> * PATCHv2: Rebase remaining parts against latest tree and merge all
> patches dealing with converting sk lock to chan lock together following
> recommendation from Ulisses and Gustavo.
> * PATCHv1: Added extra line (per Gustavo comment)
> * RFCv6: Same code but patches 2,3 and 4 from RFCv5 are merged together
> following recommendations from review.
> * RFCv5: Fixed locking bug in l2cap_data_channel, added locks in
> l2cap_sock_shutdown function, fixed several styles issues.
> * RFCv4: Better split patches so they looks more clear and obvious,
> taking coments about naming change and delete unused vars. See diff change
> from the previous version below:
> * RFCv3: Split the big patch to several small (I believe logical) chunks,
> remove unneded locks from cleanup_listen, use the same arguments for
> locked/unlocked socket error functions.
> * RFCv2: Convert l2cap channel list back to mutex from RCU list.
>
> Andrei Emeltchenko (3):
> Bluetooth: Add unlocked __l2cap_chan_add function
> Bluetooth: Change sk lock to chan lock in L2CAP core
> Bluetooth: Remove socket lock check
>
> net/bluetooth/l2cap_core.c | 175 ++++++++++++++++++++++++++------------------
> net/bluetooth/l2cap_sock.c | 26 +++++--
> 2 files changed, 125 insertions(+), 76 deletions(-)

All three patches have been applied. Thanks.

Johan

2012-02-22 18:41:46

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCHv3 2/3] Bluetooth: Change sk lock to chan lock in L2CAP core

Hi Andrei,

* Andrei Emeltchenko <[email protected]> [2012-02-22 17:11:56 +0200]:

> From: Andrei Emeltchenko <[email protected]>
>
> Change sk lock to chan lock in l2cap core and move sk locks
> to l2cap sock code. bh_locks were used because of being RCU
> critical section. When needed use explicit socket locks.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Reviewed-by: Ulisses Furquim <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 166 ++++++++++++++++++++++++++------------------
> net/bluetooth/l2cap_sock.c | 18 ++++-
> 2 files changed, 114 insertions(+), 70 deletions(-)

Acked-by: Gustavo F. Padovan <[email protected]>

Now lets go to the next steps in the removal of socket usage inside L2CAP
core.

Gustavo

2012-02-22 15:11:57

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv3 3/3] Bluetooth: Remove socket lock check

From: Andrei Emeltchenko <[email protected]>

Simplify code so that we do not need to check whether socket is locked.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap_sock.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 73a06c1..52c94c7 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -125,15 +125,15 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al

err = l2cap_chan_connect(chan, la.l2_psm, la.l2_cid, &la.l2_bdaddr);
if (err)
- goto done;
+ return err;

lock_sock(sk);

err = bt_sock_wait_state(sk, BT_CONNECTED,
sock_sndtimeo(sk, flags & O_NONBLOCK));
-done:
- if (sock_owned_by_user(sk))
- release_sock(sk);
+
+ release_sock(sk);
+
return err;
}

--
1.7.9


2012-02-22 15:11:56

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv3 2/3] Bluetooth: Change sk lock to chan lock in L2CAP core

From: Andrei Emeltchenko <[email protected]>

Change sk lock to chan lock in l2cap core and move sk locks
to l2cap sock code. bh_locks were used because of being RCU
critical section. When needed use explicit socket locks.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Reviewed-by: Ulisses Furquim <[email protected]>
---
net/bluetooth/l2cap_core.c | 166 ++++++++++++++++++++++++++------------------
net/bluetooth/l2cap_sock.c | 18 ++++-
2 files changed, 114 insertions(+), 70 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 0e4f4cb..5f4cfea 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -238,13 +238,12 @@ static void l2cap_chan_timeout(struct work_struct *work)
struct l2cap_chan *chan = container_of(work, struct l2cap_chan,
chan_timer.work);
struct l2cap_conn *conn = chan->conn;
- struct sock *sk = chan->sk;
int reason;

BT_DBG("chan %p state %s", chan, state_to_string(chan->state));

mutex_lock(&conn->chan_lock);
- lock_sock(sk);
+ l2cap_chan_lock(chan);

if (chan->state == BT_CONNECTED || chan->state == BT_CONFIG)
reason = ECONNREFUSED;
@@ -256,7 +255,7 @@ static void l2cap_chan_timeout(struct work_struct *work)

l2cap_chan_close(chan, reason);

- release_sock(sk);
+ l2cap_chan_unlock(chan);

chan->ops->close(chan->data);
mutex_unlock(&conn->chan_lock);
@@ -356,8 +355,6 @@ void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
mutex_unlock(&conn->chan_lock);
}

-/* Delete channel.
- * Must be called on the locked socket. */
static void l2cap_chan_del(struct l2cap_chan *chan, int err)
{
struct sock *sk = chan->sk;
@@ -378,6 +375,8 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
hci_conn_put(conn->hcon);
}

+ lock_sock(sk);
+
__l2cap_state_change(chan, BT_CLOSED);
sock_set_flag(sk, SOCK_ZAPPED);

@@ -390,6 +389,8 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
} else
sk->sk_state_change(sk);

+ release_sock(sk);
+
if (!(test_bit(CONF_OUTPUT_DONE, &chan->conf_state) &&
test_bit(CONF_INPUT_DONE, &chan->conf_state)))
return;
@@ -422,10 +423,10 @@ static void l2cap_chan_cleanup_listen(struct sock *parent)
while ((sk = bt_accept_dequeue(parent, NULL))) {
struct l2cap_chan *chan = l2cap_pi(sk)->chan;

+ l2cap_chan_lock(chan);
__clear_chan_timer(chan);
- lock_sock(sk);
l2cap_chan_close(chan, ECONNRESET);
- release_sock(sk);
+ l2cap_chan_unlock(chan);

chan->ops->close(chan->data);
}
@@ -441,10 +442,12 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)

switch (chan->state) {
case BT_LISTEN:
+ lock_sock(sk);
l2cap_chan_cleanup_listen(sk);

__l2cap_state_change(chan, BT_CLOSED);
sock_set_flag(sk, SOCK_ZAPPED);
+ release_sock(sk);
break;

case BT_CONNECTED:
@@ -487,7 +490,9 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
break;

default:
+ lock_sock(sk);
sock_set_flag(sk, SOCK_ZAPPED);
+ release_sock(sk);
break;
}
}
@@ -715,6 +720,7 @@ static inline int l2cap_mode_supported(__u8 mode, __u32 feat_mask)

static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *chan, int err)
{
+ struct sock *sk = chan->sk;
struct l2cap_disconn_req req;

if (!conn)
@@ -731,8 +737,10 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *c
l2cap_send_cmd(conn, l2cap_get_ident(conn),
L2CAP_DISCONN_REQ, sizeof(req), &req);

+ lock_sock(sk);
__l2cap_state_change(chan, BT_DISCONN);
__l2cap_chan_set_err(chan, err);
+ release_sock(sk);
}

/* ---- L2CAP connections ---- */
@@ -747,10 +755,10 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
struct sock *sk = chan->sk;

- bh_lock_sock(sk);
+ l2cap_chan_lock(chan);

if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
continue;
}

@@ -759,17 +767,15 @@ static void l2cap_conn_start(struct l2cap_conn *conn)

if (!l2cap_chan_check_security(chan) ||
!__l2cap_no_conn_pending(chan)) {
- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
continue;
}

if (!l2cap_mode_supported(chan->mode, conn->feat_mask)
&& test_bit(CONF_STATE2_DEVICE,
&chan->conf_state)) {
- /* l2cap_chan_close() calls list_del(chan)
- * so release the lock */
l2cap_chan_close(chan, ECONNRESET);
- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
continue;
}

@@ -789,6 +795,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
rsp.dcid = cpu_to_le16(chan->scid);

if (l2cap_chan_check_security(chan)) {
+ lock_sock(sk);
if (bt_sk(sk)->defer_setup) {
struct sock *parent = bt_sk(sk)->parent;
rsp.result = cpu_to_le16(L2CAP_CR_PEND);
@@ -801,6 +808,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
rsp.result = cpu_to_le16(L2CAP_CR_SUCCESS);
rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
}
+ release_sock(sk);
} else {
rsp.result = cpu_to_le16(L2CAP_CR_PEND);
rsp.status = cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
@@ -811,7 +819,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn)

if (test_bit(CONF_REQ_SENT, &chan->conf_state) ||
rsp.result != L2CAP_CR_SUCCESS) {
- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
continue;
}

@@ -821,7 +829,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
chan->num_conf_req++;
}

- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
}

mutex_unlock(&conn->chan_lock);
@@ -910,7 +918,11 @@ clean:
static void l2cap_chan_ready(struct l2cap_chan *chan)
{
struct sock *sk = chan->sk;
- struct sock *parent = bt_sk(sk)->parent;
+ struct sock *parent;
+
+ lock_sock(sk);
+
+ parent = bt_sk(sk)->parent;

BT_DBG("sk %p, parent %p", sk, parent);

@@ -922,6 +934,8 @@ static void l2cap_chan_ready(struct l2cap_chan *chan)

if (parent)
parent->sk_data_ready(parent, 0);
+
+ release_sock(sk);
}

static void l2cap_conn_ready(struct l2cap_conn *conn)
@@ -939,23 +953,25 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
mutex_lock(&conn->chan_lock);

list_for_each_entry(chan, &conn->chan_l, list) {
- struct sock *sk = chan->sk;

- bh_lock_sock(sk);
+ l2cap_chan_lock(chan);

if (conn->hcon->type == LE_LINK) {
if (smp_conn_security(conn, chan->sec_level))
l2cap_chan_ready(chan);

} else if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
+ struct sock *sk = chan->sk;
__clear_chan_timer(chan);
+ lock_sock(sk);
__l2cap_state_change(chan, BT_CONNECTED);
sk->sk_state_change(sk);
+ release_sock(sk);

} else if (chan->state == BT_CONNECT)
l2cap_do_start(chan);

- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
}

mutex_unlock(&conn->chan_lock);
@@ -993,7 +1009,6 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
{
struct l2cap_conn *conn = hcon->l2cap_data;
struct l2cap_chan *chan, *l;
- struct sock *sk;

if (!conn)
return;
@@ -1006,10 +1021,12 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)

/* Kill channels */
list_for_each_entry_safe(chan, l, &conn->chan_l, list) {
- sk = chan->sk;
- lock_sock(sk);
+ l2cap_chan_lock(chan);
+
l2cap_chan_del(chan, err);
- release_sock(sk);
+
+ l2cap_chan_unlock(chan);
+
chan->ops->close(chan->data);
}

@@ -1140,7 +1157,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d

hci_dev_lock(hdev);

- lock_sock(sk);
+ l2cap_chan_lock(chan);

/* PSM must be odd and lsb of upper byte must be 0 */
if ((__le16_to_cpu(psm) & 0x0101) != 0x0001 && !cid &&
@@ -1167,17 +1184,21 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
goto done;
}

+ lock_sock(sk);
+
switch (sk->sk_state) {
case BT_CONNECT:
case BT_CONNECT2:
case BT_CONFIG:
/* Already connecting */
err = 0;
+ release_sock(sk);
goto done;

case BT_CONNECTED:
/* Already connected */
err = -EISCONN;
+ release_sock(sk);
goto done;

case BT_OPEN:
@@ -1187,11 +1208,15 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d

default:
err = -EBADFD;
+ release_sock(sk);
goto done;
}

/* Set destination address and psm */
bacpy(&bt_sk(sk)->dst, dst);
+
+ release_sock(sk);
+
chan->psm = psm;
chan->dcid = cid;

@@ -1219,16 +1244,18 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
/* Update source addr of the socket */
bacpy(src, conn->src);

+ l2cap_chan_unlock(chan);
l2cap_chan_add(conn, chan);
+ l2cap_chan_lock(chan);

- __l2cap_state_change(chan, BT_CONNECT);
+ l2cap_state_change(chan, BT_CONNECT);
__set_chan_timer(chan, sk->sk_sndtimeo);

if (hcon->state == BT_CONNECTED) {
if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
__clear_chan_timer(chan);
if (l2cap_chan_check_security(chan))
- __l2cap_state_change(chan, BT_CONNECTED);
+ l2cap_state_change(chan, BT_CONNECTED);
} else
l2cap_do_start(chan);
}
@@ -1236,6 +1263,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
err = 0;

done:
+ l2cap_chan_unlock(chan);
hci_dev_unlock(hdev);
hci_dev_put(hdev);
return err;
@@ -1277,14 +1305,14 @@ static void l2cap_monitor_timeout(struct work_struct *work)
{
struct l2cap_chan *chan = container_of(work, struct l2cap_chan,
monitor_timer.work);
- struct sock *sk = chan->sk;

BT_DBG("chan %p", chan);

- lock_sock(sk);
+ l2cap_chan_lock(chan);
+
if (chan->retry_count >= chan->remote_max_tx) {
l2cap_send_disconn_req(chan->conn, chan, ECONNABORTED);
- release_sock(sk);
+ l2cap_chan_unlock(chan);
return;
}

@@ -1292,25 +1320,26 @@ static void l2cap_monitor_timeout(struct work_struct *work)
__set_monitor_timer(chan);

l2cap_send_rr_or_rnr(chan, L2CAP_CTRL_POLL);
- release_sock(sk);
+ l2cap_chan_unlock(chan);
}

static void l2cap_retrans_timeout(struct work_struct *work)
{
struct l2cap_chan *chan = container_of(work, struct l2cap_chan,
retrans_timer.work);
- struct sock *sk = chan->sk;

BT_DBG("chan %p", chan);

- lock_sock(sk);
+ l2cap_chan_lock(chan);
+
chan->retry_count = 1;
__set_monitor_timer(chan);

set_bit(CONN_WAIT_F, &chan->conn_state);

l2cap_send_rr_or_rnr(chan, L2CAP_CTRL_POLL);
- release_sock(sk);
+
+ l2cap_chan_unlock(chan);
}

static void l2cap_drop_acked_frames(struct l2cap_chan *chan)
@@ -2001,9 +2030,11 @@ static void l2cap_ack_timeout(struct work_struct *work)

BT_DBG("chan %p", chan);

- lock_sock(chan->sk);
+ l2cap_chan_lock(chan);
+
__l2cap_send_ack(chan);
- release_sock(chan->sk);
+
+ l2cap_chan_unlock(chan);

l2cap_chan_put(chan);
}
@@ -2664,7 +2695,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd

bt_accept_enqueue(parent, sk);

- l2cap_chan_add(conn, chan);
+ __l2cap_chan_add(conn, chan);

dcid = chan->scid;

@@ -2737,7 +2768,6 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
struct l2cap_conn_rsp *rsp = (struct l2cap_conn_rsp *) data;
u16 scid, dcid, result, status;
struct l2cap_chan *chan;
- struct sock *sk;
u8 req[128];
int err;

@@ -2767,8 +2797,7 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd

err = 0;

- sk = chan->sk;
- lock_sock(sk);
+ l2cap_chan_lock(chan);

switch (result) {
case L2CAP_CR_SUCCESS:
@@ -2794,7 +2823,7 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
break;
}

- release_sock(sk);
+ l2cap_chan_unlock(chan);

unlock:
mutex_unlock(&conn->chan_lock);
@@ -2819,7 +2848,6 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
u16 dcid, flags;
u8 rsp[64];
struct l2cap_chan *chan;
- struct sock *sk;
int len;

dcid = __le16_to_cpu(req->dcid);
@@ -2831,8 +2859,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
if (!chan)
return -ENOENT;

- sk = chan->sk;
- lock_sock(sk);
+ l2cap_chan_lock(chan);

if (chan->state != BT_CONFIG && chan->state != BT_CONNECT2) {
struct l2cap_cmd_rej_cid rej;
@@ -2921,7 +2948,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
}

unlock:
- release_sock(sk);
+ l2cap_chan_unlock(chan);
return 0;
}

@@ -2930,7 +2957,6 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
struct l2cap_conf_rsp *rsp = (struct l2cap_conf_rsp *)data;
u16 scid, flags, result;
struct l2cap_chan *chan;
- struct sock *sk;
int len = cmd->len - sizeof(*rsp);

scid = __le16_to_cpu(rsp->scid);
@@ -2944,8 +2970,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
if (!chan)
return 0;

- sk = chan->sk;
- lock_sock(sk);
+ l2cap_chan_lock(chan);

switch (result) {
case L2CAP_CONF_SUCCESS:
@@ -3004,7 +3029,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
}

default:
- __l2cap_chan_set_err(chan, ECONNRESET);
+ l2cap_chan_set_err(chan, ECONNRESET);

__set_chan_timer(chan,
msecs_to_jiffies(L2CAP_DISC_REJ_TIMEOUT));
@@ -3031,7 +3056,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
}

done:
- release_sock(sk);
+ l2cap_chan_unlock(chan);
return 0;
}

@@ -3056,17 +3081,21 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
return 0;
}

+ l2cap_chan_lock(chan);
+
sk = chan->sk;
- lock_sock(sk);

rsp.dcid = cpu_to_le16(chan->scid);
rsp.scid = cpu_to_le16(chan->dcid);
l2cap_send_cmd(conn, cmd->ident, L2CAP_DISCONN_RSP, sizeof(rsp), &rsp);

+ lock_sock(sk);
sk->sk_shutdown = SHUTDOWN_MASK;
+ release_sock(sk);

l2cap_chan_del(chan, ECONNRESET);
- release_sock(sk);
+
+ l2cap_chan_unlock(chan);

chan->ops->close(chan->data);

@@ -3080,7 +3109,6 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
struct l2cap_disconn_rsp *rsp = (struct l2cap_disconn_rsp *) data;
u16 dcid, scid;
struct l2cap_chan *chan;
- struct sock *sk;

scid = __le16_to_cpu(rsp->scid);
dcid = __le16_to_cpu(rsp->dcid);
@@ -3095,11 +3123,11 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
return 0;
}

- sk = chan->sk;
- lock_sock(sk);
+ l2cap_chan_lock(chan);

l2cap_chan_del(chan, 0);
- release_sock(sk);
+
+ l2cap_chan_unlock(chan);

chan->ops->close(chan->data);

@@ -4251,7 +4279,6 @@ drop:
static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk_buff *skb)
{
struct l2cap_chan *chan;
- struct sock *sk = NULL;
u32 control;
u16 tx_seq;
int len;
@@ -4259,11 +4286,12 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
chan = l2cap_get_chan_by_scid(conn, cid);
if (!chan) {
BT_DBG("unknown cid 0x%4.4x", cid);
- goto drop;
+ /* Drop packet and return */
+ kfree(skb);
+ return 0;
}

- sk = chan->sk;
- lock_sock(sk);
+ l2cap_chan_lock(chan);

BT_DBG("chan %p, len %d", chan, skb->len);

@@ -4334,8 +4362,7 @@ drop:
kfree_skb(skb);

done:
- if (sk)
- release_sock(sk);
+ l2cap_chan_unlock(chan);

return 0;
}
@@ -4554,9 +4581,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
mutex_lock(&conn->chan_lock);

list_for_each_entry(chan, &conn->chan_l, list) {
- struct sock *sk = chan->sk;
-
- bh_lock_sock(sk);
+ l2cap_chan_lock(chan);

BT_DBG("chan->scid %d", chan->scid);

@@ -4566,19 +4591,19 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
l2cap_chan_ready(chan);
}

- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
continue;
}

if (test_bit(CONF_CONNECT_PEND, &chan->conf_state)) {
- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
continue;
}

if (!status && (chan->state == BT_CONNECTED ||
chan->state == BT_CONFIG)) {
l2cap_check_encryption(chan, encrypt);
- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
continue;
}

@@ -4599,9 +4624,12 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
msecs_to_jiffies(L2CAP_DISC_TIMEOUT));
}
} else if (chan->state == BT_CONNECT2) {
+ struct sock *sk = chan->sk;
struct l2cap_conn_rsp rsp;
__u16 res, stat;

+ lock_sock(sk);
+
if (!status) {
if (bt_sk(sk)->defer_setup) {
struct sock *parent = bt_sk(sk)->parent;
@@ -4622,6 +4650,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
stat = L2CAP_CS_NO_INFO;
}

+ release_sock(sk);
+
rsp.scid = cpu_to_le16(chan->dcid);
rsp.dcid = cpu_to_le16(chan->scid);
rsp.result = cpu_to_le16(res);
@@ -4630,7 +4660,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
sizeof(rsp), &rsp);
}

- bh_unlock_sock(sk);
+ l2cap_chan_unlock(chan);
}

mutex_unlock(&conn->chan_lock);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 1273fcb..73a06c1 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -127,6 +127,8 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
if (err)
goto done;

+ lock_sock(sk);
+
err = bt_sock_wait_state(sk, BT_CONNECTED,
sock_sndtimeo(sk, flags & O_NONBLOCK));
done:
@@ -810,14 +812,18 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
if (conn)
mutex_lock(&conn->chan_lock);

+ l2cap_chan_lock(chan);
lock_sock(sk);
+
if (!sk->sk_shutdown) {
if (chan->mode == L2CAP_MODE_ERTM)
err = __l2cap_wait_ack(sk);

sk->sk_shutdown = SHUTDOWN_MASK;

+ release_sock(sk);
l2cap_chan_close(chan, 0);
+ lock_sock(sk);

if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime)
err = bt_sock_wait_state(sk, BT_CLOSED,
@@ -828,6 +834,7 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
err = -sk->sk_err;

release_sock(sk);
+ l2cap_chan_unlock(chan);

if (conn)
mutex_unlock(&conn->chan_lock);
@@ -874,8 +881,12 @@ static int l2cap_sock_recv_cb(void *data, struct sk_buff *skb)
struct sock *sk = data;
struct l2cap_pinfo *pi = l2cap_pi(sk);

- if (pi->rx_busy_skb)
- return -ENOMEM;
+ lock_sock(sk);
+
+ if (pi->rx_busy_skb) {
+ err = -ENOMEM;
+ goto done;
+ }

err = sock_queue_rcv_skb(sk, skb);

@@ -894,6 +905,9 @@ static int l2cap_sock_recv_cb(void *data, struct sk_buff *skb)
err = 0;
}

+done:
+ release_sock(sk);
+
return err;
}

--
1.7.9


2012-02-22 15:11:55

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv3 1/3] Bluetooth: Add unlocked __l2cap_chan_add function

From: Andrei Emeltchenko <[email protected]>

Add unlocked L2CAP channel add function. Unlocked version will
be used in later patches.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Reviewed-by: Ulisses Furquim <[email protected]>
Acked-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap_core.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index c0640b7..0e4f4cb 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -300,7 +300,7 @@ void l2cap_chan_destroy(struct l2cap_chan *chan)
l2cap_chan_put(chan);
}

-static void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
+void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
{
BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn,
chan->psm, chan->dcid);
@@ -346,8 +346,13 @@ static void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)

l2cap_chan_hold(chan);

- mutex_lock(&conn->chan_lock);
list_add(&chan->list, &conn->chan_l);
+}
+
+void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
+{
+ mutex_lock(&conn->chan_lock);
+ __l2cap_chan_add(conn, chan);
mutex_unlock(&conn->chan_lock);
}

--
1.7.9