@@ -4634,26 +4634,6 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
return err;
}
-static struct l2cap_chan *l2cap_del_chan_by_scid(struct l2cap_conn *conn,
- u16 cid, int err)
-{
- struct l2cap_chan *c;
-
- mutex_lock(&conn->chan_lock);
- c = __l2cap_get_chan_by_scid(conn, cid);
- if (c) {
- /* Only lock if chan reference is not 0 */
- c = l2cap_chan_hold_unless_zero(c);
- if (c) {
- l2cap_chan_lock(c);
- l2cap_chan_del(c, err);
- }
- }
- mutex_unlock(&conn->chan_lock);
-
- return c;
-}
-
static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
struct l2cap_cmd_hdr *cmd, u16 cmd_len,
u8 *data)
@@ -4671,7 +4651,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
BT_DBG("scid 0x%4.4x dcid 0x%4.4x", scid, dcid);
- chan = l2cap_del_chan_by_scid(conn, dcid, ECONNRESET);
+ chan = l2cap_get_chan_by_scid(conn, dcid);
if (!chan) {
cmd_reject_invalid_cid(conn, cmd->ident, dcid, scid);
return 0;
@@ -4682,6 +4662,13 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
l2cap_send_cmd(conn, cmd->ident, L2CAP_DISCONN_RSP, sizeof(rsp), &rsp);
chan->ops->set_shutdown(chan);
+
+ l2cap_chan_unlock(chan);
+ mutex_lock(&conn->chan_lock);
+ l2cap_chan_lock(chan);
+ l2cap_chan_del(chan, ECONNRESET);
+ mutex_unlock(&conn->chan_lock);
+
chan->ops->close(chan);
l2cap_chan_unlock(chan);
@@ -4706,7 +4693,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn,
BT_DBG("dcid 0x%4.4x scid 0x%4.4x", dcid, scid);
- chan = l2cap_del_chan_by_scid(conn, scid, 0);
+ chan = l2cap_get_chan_by_scid(conn, scid);
if (!chan)
return 0;
@@ -4716,6 +4703,12 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn,
return 0;
}
+ l2cap_chan_unlock(chan);
+ mutex_lock(&conn->chan_lock);
+ l2cap_chan_lock(chan);
+ l2cap_chan_del(chan, 0);
+ mutex_unlock(&conn->chan_lock);
+
chan->ops->close(chan);
l2cap_chan_unlock(chan);
L2CAP assumes that the locks conn->chan_lock and chan->lock are acquired in the order conn->chan_lock, chan->lock to avoid potential deadlock. For example, l2sock_shutdown acquires these locks in the order: mutex_lock(&conn->chan_lock) l2cap_chan_lock(chan) However, l2cap_disconnect_req acquires chan->lock in l2cap_get_chan_by_scid first and then acquires conn->chan_lock before calling l2cap_chan_del. This means that these locks are acquired in unexpected order, which leads to potential deadlock: l2cap_chan_lock(c) mutex_lock(&conn->chan_lock) This patch releases chan->lock before acquiring the conn_chan_lock to avoid the potential deadlock. Fixes: a2a9339e1c9d ("Bluetooth: L2CAP: Fix use-after-free in l2cap_disconnect_{req,rsp}") Signed-off-by: Ying Hsu <yinghsu@chromium.org> --- This commit has been tested on a Chromebook device. Changes in v6: - Fixing format of the fixes tag. Changes in v5: - Fixing the merge conflict by removing l2cap_del_chan_by_scid. Changes in v4: - Using l2cap_get_chan_by_scid to avoid repeated code. - Releasing chan->lock before acquiring conn->chan_lock. Changes in v3: - Adding the fixes tag. Changes in v2: - Adding the prefix "Bluetooth:" to subject line. net/bluetooth/l2cap_core.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-)