Bluetooth: RFCOMM: hold listener socket in rfcomm_connect_ind()

rfcomm_get_sock_by_channel() scans rfcomm_sk_list under the list lock,
but returns the selected listener after dropping that lock without
taking a reference. rfcomm_connect_ind() then locks the listener,
queues a child socket on it, and may notify it after unlocking it.

The buggy scenario involves two paths, with each column showing the
order within that path:

rfcomm_connect_ind():            listener close:
  1. Find parent in              1. close() enters
     rfcomm_get_sock_by_channel()   rfcomm_sock_release().
  2. Drop rfcomm_sk_list.lock    2. rfcomm_sock_shutdown()
     without pinning parent.        closes the listener.
  3. Call lock_sock(parent) and  3. rfcomm_sock_kill()
     bt_accept_enqueue(parent,      unlinks and puts parent.
     sk, true).
  4. Read parent flags and may   4. parent can be freed.
     call sk_state_change().

If close wins the race, parent can be freed before
rfcomm_connect_ind() reaches lock_sock(), bt_accept_enqueue(), or the
deferred-setup callback.

Take a reference on the listener before leaving rfcomm_sk_list.lock.
After lock_sock() succeeds, recheck that it is still in BT_LISTEN
before queueing a child, cache the deferred-setup bit while the parent
is locked, and drop the reference after the last parent use.

KASAN reported a slab-use-after-free in lock_sock_nested() from
rfcomm_connect_ind(), with the freeing stack going through
rfcomm_sock_kill() and rfcomm_sock_release().

Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This commit is contained in:
Zhang Cen 2026-05-28 15:56:41 +08:00 committed by Luiz Augusto von Dentz
parent cdf88b35e0
commit 43c441edac

View File

@ -122,7 +122,7 @@ static struct sock *__rfcomm_get_listen_sock_by_addr(u8 channel, bdaddr_t *src)
}
/* Find socket with channel and source bdaddr.
* Returns closest match.
* Returns closest match with an extra reference held.
*/
static struct sock *rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
{
@ -136,15 +136,25 @@ static struct sock *rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *
if (rfcomm_pi(sk)->channel == channel) {
/* Exact match. */
if (!bacmp(&rfcomm_pi(sk)->src, src))
if (!bacmp(&rfcomm_pi(sk)->src, src)) {
sock_hold(sk);
break;
}
/* Closest match */
if (!bacmp(&rfcomm_pi(sk)->src, BDADDR_ANY))
if (!bacmp(&rfcomm_pi(sk)->src, BDADDR_ANY)) {
if (sk1)
sock_put(sk1);
sk1 = sk;
sock_hold(sk1);
}
}
}
if (sk && sk1)
sock_put(sk1);
read_unlock(&rfcomm_sk_list.lock);
return sk ? sk : sk1;
@ -941,6 +951,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
{
struct sock *sk, *parent;
bdaddr_t src, dst;
bool defer_setup = false;
int result = 0;
BT_DBG("session %p channel %d", s, channel);
@ -954,6 +965,11 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
lock_sock(parent);
if (parent->sk_state != BT_LISTEN)
goto done;
defer_setup = test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags);
/* Check for backlog size */
if (sk_acceptq_is_full(parent)) {
BT_DBG("backlog full %d", parent->sk_ack_backlog);
@ -981,9 +997,11 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
done:
release_sock(parent);
if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags))
if (defer_setup)
parent->sk_state_change(parent);
sock_put(parent);
return result;
}