[PATCH] Bluetooth: RFCOMM: free the skb when the DLC has no owner

Maxim Skokov posted 1 patch 1 week, 3 days ago
There is a newer version of this series
net/bluetooth/rfcomm/sock.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] Bluetooth: RFCOMM: free the skb when the DLC has no owner
Posted by Maxim Skokov 1 week, 3 days ago
rfcomm_recv_data() hands the skb to d->data_ready() and returns 0
without freeing it, so the callback owns the skb from that point on.
The tty implementation frees it when the DLC has been detached from its
device:

  static void rfcomm_dev_data_ready(struct rfcomm_dlc *dlc,
                                    struct sk_buff *skb)
  {
          struct rfcomm_dev *dev = dlc->owner;

          if (!dev) {
                  kfree_skb(skb);
                  return;
          }

The socket implementation returns without freeing it, and the skb is
leaked.

A connected DLC can have no owner: rfcomm_sock_destruct() clears
d->owner, while the DLC itself stays on the session until it is closed.
Data frames that arrive in that window still find d->state ==
BT_CONNECTED in rfcomm_recv_data() and are handed to the callback, one
leaked skb each.

The leak does not stop on its own either.  The remote runs out of
credits only once RFCOMM_RX_THROTTLED is set, and for a socket the only
place that sets it is rfcomm_sk_data_ready() itself, below the return.
rfcomm_process_dlcs() keeps granting credits, so the remote keeps
sending and the leak keeps growing.

Free the skb, as the tty side does.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Closes: https://lore.kernel.org/linux-bluetooth/CABXGCsPe5wykd8Aj-hm+SJ7SGQyb8yoGNnx3XDpPmBathj+EGg@mail.gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Maxim Skokov <skokovmaksimevg@gmail.com>
Assisted-by: Claude:claude-opus-5
---

Notes:
    Noticed in the v3 thread of "Bluetooth: RFCOMM: connect the session
    socket without rfcomm_mutex", where the same leak is described.
    
    Found by inspection.  I did not manage to produce an orphaned connected
    DLC on hardware: it needs the socket to be destroyed while the remote is
    still sending on the DLC.  A kprobe on rfcomm_sk_data_ready() over an
    HFP session (RTL8851BE, btusb) counted 31 frames delivered to a live
    socket and one rfcomm_sock_destruct(), so the window does open in normal
    use; no frame happened to arrive inside it.
    
    Built and loaded on 7.3.0-rc1 with KASAN, UBSAN, PROVE_LOCKING and
    DEBUG_OBJECTS; HFP and A2DP sessions behave as before, and none of the
    sanitizers reported anything on the RFCOMM paths.

 net/bluetooth/rfcomm/sock.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 958081adb9..82aa38715c 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -48,8 +48,11 @@ static void rfcomm_sock_kill(struct sock *sk);
 static void rfcomm_sk_data_ready(struct rfcomm_dlc *d, struct sk_buff *skb)
 {
 	struct sock *sk = d->owner;
-	if (!sk)
+
+	if (!sk) {
+		kfree_skb(skb);
 		return;
+	}
 
 	atomic_add(skb->len, &sk->sk_rmem_alloc);
 	skb_queue_tail(&sk->sk_receive_queue, skb);
-- 
2.47.3