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

[PATCH] 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 4020751fd0df (L2CAP) and b4a00dedc5cc (ISO): hold the
listener 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>
---

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: Bluetooth: RFCOMM: hold the listener across child teardown

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1985 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/1192576/

---Test result---

Test Summary:
CheckPatch                    FAIL      0.55 seconds
VerifyFixes                   PASS      0.09 seconds
VerifySignedoff               PASS      0.09 seconds
GitLint                       PASS      0.24 seconds
SubjectPrefix                 PASS      0.08 seconds
BuildKernel                   PASS      22.77 seconds
CheckAllWarning               PASS      26.51 seconds
CheckSparse                   PASS      28.85 seconds
BuildKernel32                 PASS      23.11 seconds
CheckKernelLLVM               PASS      25.68 seconds
TestRunnerSetup               PASS      530.02 seconds
TestRunner_rfcomm-tester      PASS      7.19 seconds
IncrementalBuild              PASS      22.18 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: RFCOMM: hold the listener across child teardown
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 4020751fd0df ("Bluetooth: L2CAP: Hold the listener while notifying child teardown")'
#114: 
Same class as 4020751fd0df (L2CAP) and b4a00dedc5cc (ISO): hold the

total: 1 errors, 0 warnings, 0 checks, 15 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/home/runner/work/bluetooth-next/bluetooth-next/src/patch/14911944.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.




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

---
Regards,
Linux Bluetooth