From e7456437c15a2fd42cedd25c2b12b06876f285f0 Mon Sep 17 00:00:00 2001 From: Dean Jenkins Date: Wed, 14 Oct 2015 12:18:45 +0200 Subject: Bluetooth: Unwind l2cap_sock_shutdown() l2cap_sock_shutdown() is designed to only action shutdown of the channel when shutdown is not already in progress. Therefore, reorganise the code flow by adding a goto to jump to the end of function handling when shutdown is already being actioned. This removes one level of code indentation and make the code more readable. Signed-off-by: Dean Jenkins Signed-off-by: Harish Jenny K N Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_sock.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) (limited to 'net/bluetooth/l2cap_sock.c') diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 586b3d580cfc..ca5598d6a201 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1111,6 +1111,11 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) if (!sk) return 0; + if (sk->sk_shutdown) + goto shutdown_already; + + BT_DBG("Handling sock shutdown"); + /* prevent sk structure from being freed whilst unlocked */ sock_hold(sk); @@ -1127,23 +1132,21 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) l2cap_chan_lock(chan); lock_sock(sk); - if (!sk->sk_shutdown) { - if (chan->mode == L2CAP_MODE_ERTM && - chan->unacked_frames > 0 && - chan->state == BT_CONNECTED) - err = __l2cap_wait_ack(sk, chan); + if (chan->mode == L2CAP_MODE_ERTM && + chan->unacked_frames > 0 && + chan->state == BT_CONNECTED) + err = __l2cap_wait_ack(sk, chan); - sk->sk_shutdown = SHUTDOWN_MASK; + sk->sk_shutdown = SHUTDOWN_MASK; - release_sock(sk); - l2cap_chan_close(chan, 0); - lock_sock(sk); + release_sock(sk); + l2cap_chan_close(chan, 0); + lock_sock(sk); - if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime && - !(current->flags & PF_EXITING)) - err = bt_sock_wait_state(sk, BT_CLOSED, - sk->sk_lingertime); - } + if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime && + !(current->flags & PF_EXITING)) + err = bt_sock_wait_state(sk, BT_CLOSED, + sk->sk_lingertime); if (!err && sk->sk_err) err = -sk->sk_err; @@ -1157,6 +1160,7 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) l2cap_chan_put(chan); sock_put(sk); +shutdown_already: BT_DBG("err: %d", err); return err; -- cgit v1.2.3 From 04ba72e6b24f1e0e2221fcd73f08782870473fa1 Mon Sep 17 00:00:00 2001 From: Dean Jenkins Date: Wed, 14 Oct 2015 12:18:46 +0200 Subject: Bluetooth: Reorganize mutex lock in l2cap_sock_shutdown() This commit reorganizes the mutex lock and is now only protecting l2cap_chan_close(). This is now consistent with other places where l2cap_chan_close() is called. If a conn connection exists, call mutex_lock(&conn->chan_lock) before calling l2cap_chan_close() to ensure other L2CAP protocol operations do not interfere. Note that the conn structure has to be protected from being freed as it is possible for the connection to be disconnected whilst the locks are not held. This solution allows the mutex lock to be used even when the connection has just been disconnected. This commit also reduces the scope of chan locking. The only place where chan locking is needed is the call to l2cap_chan_close(chan, 0) which if necessary closes the channel. Therefore, move the l2cap_chan_lock(chan) and l2cap_chan_lock(chan) locking calls to around l2cap_chan_close(chan, 0). This allows __l2cap_wait_ack(sk, chan) to be called with no chan locks being held so L2CAP messaging over the ACL link can be done unimpaired. Signed-off-by: Dean Jenkins Signed-off-by: Harish Jenny K N Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_sock.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) (limited to 'net/bluetooth/l2cap_sock.c') diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index ca5598d6a201..d06fb54082aa 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1111,6 +1111,8 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) if (!sk) return 0; + lock_sock(sk); + if (sk->sk_shutdown) goto shutdown_already; @@ -1122,25 +1124,37 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) chan = l2cap_pi(sk)->chan; /* prevent chan structure from being freed whilst unlocked */ l2cap_chan_hold(chan); - conn = chan->conn; BT_DBG("chan %p state %s", chan, state_to_string(chan->state)); - if (conn) - mutex_lock(&conn->chan_lock); - - l2cap_chan_lock(chan); - lock_sock(sk); - if (chan->mode == L2CAP_MODE_ERTM && chan->unacked_frames > 0 && chan->state == BT_CONNECTED) err = __l2cap_wait_ack(sk, chan); sk->sk_shutdown = SHUTDOWN_MASK; - release_sock(sk); + + l2cap_chan_lock(chan); + conn = chan->conn; + if (conn) + /* prevent conn structure from being freed */ + l2cap_conn_get(conn); + l2cap_chan_unlock(chan); + + if (conn) + /* mutex lock must be taken before l2cap_chan_lock() */ + mutex_lock(&conn->chan_lock); + + l2cap_chan_lock(chan); l2cap_chan_close(chan, 0); + l2cap_chan_unlock(chan); + + if (conn) { + mutex_unlock(&conn->chan_lock); + l2cap_conn_put(conn); + } + lock_sock(sk); if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime && @@ -1148,20 +1162,16 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) err = bt_sock_wait_state(sk, BT_CLOSED, sk->sk_lingertime); + l2cap_chan_put(chan); + sock_put(sk); + +shutdown_already: if (!err && sk->sk_err) err = -sk->sk_err; release_sock(sk); - l2cap_chan_unlock(chan); - - if (conn) - mutex_unlock(&conn->chan_lock); - l2cap_chan_put(chan); - sock_put(sk); - -shutdown_already: - BT_DBG("err: %d", err); + BT_DBG("Sock shutdown complete err: %d", err); return err; } -- cgit v1.2.3 From 9f7378a9d6ced1784e08d3e21a9ddb769523baf2 Mon Sep 17 00:00:00 2001 From: Dean Jenkins Date: Wed, 14 Oct 2015 12:18:47 +0200 Subject: Bluetooth: l2cap_disconnection_req priority over shutdown There is a L2CAP protocol race between the local peer and the remote peer demanding disconnection of the L2CAP link. When L2CAP ERTM is used, l2cap_sock_shutdown() can be called from userland to disconnect L2CAP. However, there can be a delay introduced by waiting for ACKs. During this waiting period, the remote peer may have sent a Disconnection Request. Therefore, recheck the shutdown status of the socket after waiting for ACKs because there is no need to do further processing if the connection has gone. Signed-off-by: Dean Jenkins Signed-off-by: Harish Jenny K N Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_sock.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'net/bluetooth/l2cap_sock.c') diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index d06fb54082aa..1bb551527044 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1129,9 +1129,17 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) if (chan->mode == L2CAP_MODE_ERTM && chan->unacked_frames > 0 && - chan->state == BT_CONNECTED) + chan->state == BT_CONNECTED) { err = __l2cap_wait_ack(sk, chan); + /* After waiting for ACKs, check whether shutdown + * has already been actioned to close the L2CAP + * link such as by l2cap_disconnection_req(). + */ + if (sk->sk_shutdown) + goto has_shutdown; + } + sk->sk_shutdown = SHUTDOWN_MASK; release_sock(sk); @@ -1162,6 +1170,7 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) err = bt_sock_wait_state(sk, BT_CLOSED, sk->sk_lingertime); +has_shutdown: l2cap_chan_put(chan); sock_put(sk); -- cgit v1.2.3