[PATCH v2] Bluetooth: RFCOMM: hold the listener across child teardown

[PATCH v2] Bluetooth: RFCOMM: hold the listener across child teardown

[Hang Nan]

rfcomm_sk_state_change() takes the listener from bt_sk(sk)->parent,
unlinks a closing child from the accept queue and then notifies the
listener through that pointer.  bt_accept_enqueue() retains only the
child, so once bt_accept_unlink() returns a concurrent close() can free
the listener:

    krfcommd                          close(listener)
    rfcomm_sk_state_change()
      parent = bt_sk(sk)->parent
      bt_accept_unlink(sk)
                                      cleanup_listen(): queue empty
                                      rfcomm_sock_kill() -> kfree
      parent->sk_data_ready(parent)   <- use-after-free

KASAN: slab-use-after-free in rfcomm_sk_state_change (read of 8 bytes at
offset 264), freed by rfcomm_sock_release() -> __sk_destruct(); the window
between the unlink and the notification was widened for the test.

Same class as commit 4020751fd0df ("Bluetooth: L2CAP: Hold the listener
while notifying child teardown") and commit b4a00dedc5cc ("Bluetooth: ISO:
Hold the listener during disconnect notification"); take a reference
before the unlink and drop it after the notification.  Lock order is
unchanged, the child lock keeps cleanup_listen() out until then.

Cc: stable@vger.kernel.org
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Hang Nan <2122295973@qq.com>
---
v1 -> v2: cite the two reference commits as
          'commit <sha> ("<title>")', as checkpatch.pl requires.
          No code change.

BUG: KASAN: slab-use-after-free in rfcomm_sk_state_change+0x2ed/0x320
Read of size 8 at addr ffff888002716908 by task krfcommd/60
CPU: 1 PID: 60 Comm: krfcommd Tainted: G    B      PREEMPT(full)
  rfcomm_sk_state_change+0x2ed/0x320
  __rfcomm_dlc_close+0x16a/0x3d0
  rfcomm_run+0x1824/0x4000
  kthread

Allocated by task 72:
  rfcomm_sock_alloc.constprop.0+0x47/0x240
  rfcomm_sock_create+0xa6/0x2a0
  __sys_socket+0x116/0x1d0

Freed by task 72:
  __sk_destruct+0x499/0x5d0
  rfcomm_sock_release+0x101/0x140
  __sock_release+0xa1/0x260
  __x64_sys_close+0x78/0xd0

The buggy address is located 264 bytes inside of a freed 1024-byte
region (kmalloc-1k).


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

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 3fb8b5474948..8f59fa1fa07d 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -83,9 +83,14 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
 	if (parent) {
 		if (d->state == BT_CLOSED) {
 			sock_set_flag(sk, SOCK_ZAPPED);
+			/* keep the listener alive across the unlink */
+			sock_hold(parent);
 			bt_accept_unlink(sk);
+			parent->sk_data_ready(parent);
+			sock_put(parent);
+		} else {
+			parent->sk_data_ready(parent);
 		}
-		parent->sk_data_ready(parent);
 	} else {
 		if (d->state == BT_CONNECTED)
 			rfcomm_session_getaddr(d->session,

RE: [v2] Bluetooth: RFCOMM: hold the listener across child teardown

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/series/1192660/

---Test result---

Test Summary:
CheckPatch                    PASS      0.73 seconds
VerifyFixes                   PASS      0.11 seconds
VerifySignedoff               PASS      0.11 seconds
GitLint                       PASS      0.30 seconds
SubjectPrefix                 PASS      0.11 seconds
BuildKernel                   PASS      29.54 seconds
CheckAllWarning               PASS      33.79 seconds
CheckSparse                   PASS      36.67 seconds
BuildKernel32                 PASS      29.62 seconds
CheckKernelLLVM               PASS      35.45 seconds
TestRunnerSetup               PASS      698.11 seconds
TestRunner_rfcomm-tester      PASS      6.17 seconds
IncrementalBuild              PASS      27.42 seconds



https://github.com/bluez/bluetooth-next/pull/996

---
Regards,
Linux Bluetooth