@@ -4651,8 +4651,15 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
BT_DBG("scid 0x%4.4x dcid 0x%4.4x", scid, dcid);
- chan = l2cap_get_chan_by_scid(conn, dcid);
+ mutex_lock(&conn->chan_lock);
+ chan = __l2cap_get_chan_by_scid(conn, dcid);
+ if (chan) {
+ chan = l2cap_chan_hold_unless_zero(chan);
+ if (chan)
+ l2cap_chan_lock(chan);
+ }
if (!chan) {
+ mutex_unlock(&conn->chan_lock);
cmd_reject_invalid_cid(conn, cmd->ident, dcid, scid);
return 0;
}
@@ -4663,14 +4670,13 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
chan->ops->set_shutdown(chan);
- mutex_lock(&conn->chan_lock);
l2cap_chan_del(chan, ECONNRESET);
- mutex_unlock(&conn->chan_lock);
chan->ops->close(chan);
l2cap_chan_unlock(chan);
l2cap_chan_put(chan);
+ mutex_unlock(&conn->chan_lock);
return 0;
}
@@ -4691,25 +4697,32 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn,
BT_DBG("dcid 0x%4.4x scid 0x%4.4x", dcid, scid);
- chan = l2cap_get_chan_by_scid(conn, scid);
+ mutex_lock(&conn->chan_lock);
+ chan = __l2cap_get_chan_by_scid(conn, scid);
+ if (chan) {
+ chan = l2cap_chan_hold_unless_zero(chan);
+ if (chan)
+ l2cap_chan_lock(chan);
+ }
if (!chan) {
+ mutex_unlock(&conn->chan_lock);
return 0;
}
if (chan->state != BT_DISCONN) {
l2cap_chan_unlock(chan);
l2cap_chan_put(chan);
+ mutex_unlock(&conn->chan_lock);
return 0;
}
- mutex_lock(&conn->chan_lock);
l2cap_chan_del(chan, 0);
- mutex_unlock(&conn->chan_lock);
chan->ops->close(chan);
l2cap_chan_unlock(chan);
l2cap_chan_put(chan);
+ mutex_unlock(&conn->chan_lock);
return 0;
}
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 uses __l2cap_get_chan_by_scid to replace l2cap_get_chan_by_scid and adjusts the locking order 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 v3: - Adding the fixes tag. Changes in v2: - Adding the prefix "Bluetooth:" to subject line. net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)