mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-31 14:04:27 -04:00
Bluetooth: L2CAP: reject accept queue add unless BT_LISTEN
New sk should not be added to parent socket accept queue after last
l2cap_sock_cleanup_listen() has run in l2cap_sock_teardown_cb() and
state set to BT_CLOSED, as that can result to UAF on dereferencing the
dangling parent reference.
l2cap_sock_new_connection_cb() may race with parent l2cap_chan teardown,
due to chan->state accessed without consistent locking:
[Task 1] [Task 2]
l2cap_sock_release(parent) l2cap_connect
l2cap_sock_shutdown pchan = l2cap_global_chan_by_psm
l2cap_chan_lock(pchan)
l2cap_chan_close
l2cap_sock_teardown_cb
pchan->state = BT_CLOSED
l2cap_chan_unlock(pchan) ------> l2cap_chan_lock(pchan)
l2cap_new_connection
l2cap_sock_new_connection_cb
l2cap_chan_lock(pchan) <-------- l2cap_chan_unlock(pchan)
l2cap_sock_kill(parent) /* bt_sk(sk)->parent dangling */
Fix by adding check for sk_state == BT_LISTEN after acquiring sk lock in
l2cap_sock_new_connection_cb(). Add lock_sock() around sk_state writes
where missing, to avoid data races.
Although the data races on pchan->state should be fixed too, this
defensive sk_state check probably makes sense in any case.
Fixes: 2ff1a41a91 ("Bluetooth: L2CAP: Fix null-ptr-deref in l2cap_sock_state_change_cb()")
Reported-by: syzbot+9265e754091c2d27ea29@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9265e754091c2d27ea29
Signed-off-by: Pauli Virtanen <pav@iki.fi>
Reported-by: syzbot+9265e754091c2d27ea29@syzkaller.appspotmail.com
Tested-by: syzbot+9265e754091c2d27ea29@syzkaller.appspotmail.com
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This commit is contained in:
committed by
Luiz Augusto von Dentz
parent
ca2c4c2649
commit
d4bfa78fd6
@@ -1600,6 +1600,11 @@ static int l2cap_sock_new_connection_cb(struct l2cap_chan *chan,
|
||||
|
||||
lock_sock(parent);
|
||||
|
||||
if (parent->sk_state != BT_LISTEN) {
|
||||
release_sock(parent);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
/* Check for backlog size */
|
||||
if (sk_acceptq_is_full(parent)) {
|
||||
BT_DBG("backlog full %d", parent->sk_ack_backlog);
|
||||
@@ -1763,10 +1768,14 @@ static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
|
||||
if (!sk)
|
||||
return;
|
||||
|
||||
lock_sock(sk);
|
||||
|
||||
sk->sk_state = state;
|
||||
|
||||
if (err)
|
||||
sk->sk_err = err;
|
||||
|
||||
release_sock(sk);
|
||||
}
|
||||
|
||||
static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
|
||||
@@ -1842,6 +1851,8 @@ static void l2cap_sock_resume_cb(struct l2cap_chan *chan)
|
||||
if (!sk)
|
||||
return;
|
||||
|
||||
lock_sock(sk);
|
||||
|
||||
if (test_and_clear_bit(FLAG_PENDING_SECURITY, &chan->flags)) {
|
||||
sk->sk_state = BT_CONNECTED;
|
||||
chan->state = BT_CONNECTED;
|
||||
@@ -1849,6 +1860,8 @@ static void l2cap_sock_resume_cb(struct l2cap_chan *chan)
|
||||
|
||||
clear_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags);
|
||||
sk->sk_state_change(sk);
|
||||
|
||||
release_sock(sk);
|
||||
}
|
||||
|
||||
static void l2cap_sock_set_shutdown_cb(struct l2cap_chan *chan)
|
||||
|
||||
Reference in New Issue
Block a user