[PATCH] Bluetooth: ISO: fix use-after-free of listener socket in iso_conn_ready

[PATCH] Bluetooth: ISO: fix use-after-free of listener socket in iso_conn_ready

[Hang Nan]

iso_conn_ready() looks up the BIS listener socket with iso_get_sock(),
which takes a reference, and then, without re-checking its state,
creates a child socket from it:

	parent = iso_get_sock(hdev, ...);
	if (!parent)
		return;

	lock_sock(parent);
	sk = iso_sock_alloc(sock_net(parent), NULL, BTPROTO_ISO, ...);
	...
	iso_chan_add(conn, sk, parent);
	...
	release_sock(parent);
	sock_put(parent);

If the listener socket is closed concurrently, between iso_get_sock()
and lock_sock(), the reference taken by iso_get_sock() may be the last
one: the close path drops the link-list reference, and once
iso_conn_ready() drops its own reference at the end of the function the
socket is freed.  The child socket, however, is already linked to the
freed parent, and a later disconnect of the child runs iso_chan_del()
-> bt_accept_unlink(), which dereferences the dangling parent pointer
into the freed accept queue (a use-after-free).  The same dangling
pointer is also dereferenced through parent->sk_data_ready(parent) in
iso_chan_del().

Fix it the same way the connected (non-BIS) path was fixed in commit
0d255e63fcf3 ("Bluetooth: ISO: hold sk properly in iso_conn_ready"):
after taking the socket lock, re-check that the parent is still a
listening, alive socket, and bail out otherwise.

Fixes: ccf74f2390d60 ("Bluetooth: Add BTPROTO_ISO socket type")
Cc: stable@vger.kernel.org
Signed-off-by: Hang Nan <2122295973@qq.com>
---
 net/bluetooth/iso.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index aa2ce78f56a2..069fc87a4e18 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -2277,6 +2277,21 @@ static void iso_conn_ready(struct iso_conn *conn)
 
 		lock_sock(parent);
 
+		/* The listener socket may have been closed concurrently
+		 * between iso_get_sock() and lock_sock(): the reference
+		 * taken by iso_get_sock() may be the last one, in which
+		 * case the socket is freed as soon as we drop it at the
+		 * end of this function.  Recheck that the parent is still
+		 * a valid, listening socket before creating a child
+		 * socket from it.
+		 */
+		if (parent->sk_state != BT_LISTEN ||
+		    sock_flag(parent, SOCK_ZAPPED)) {
+			release_sock(parent);
+			sock_put(parent);
+			return;
+		}
+
 		sk = iso_sock_alloc(sock_net(parent), NULL,
 				    BTPROTO_ISO, GFP_ATOMIC, 0);
 		if (!sk) {

RE: Bluetooth: ISO: fix use-after-free of listener socket in iso_conn_ready

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2101 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/project/bluetooth/list/?series=1144631

---Test result---

Test Summary:
CheckPatch                    PASS      0.75 seconds
VerifyFixes                   PASS      0.15 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       FAIL      0.33 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      27.42 seconds
CheckAllWarning               PASS      29.93 seconds
CheckSparse                   PASS      28.65 seconds
BuildKernel32                 PASS      26.13 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      496.76 seconds
TestRunner_iso-tester         PASS      75.42 seconds
IncrementalBuild              PASS      25.49 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: ISO: fix use-after-free of listener socket in iso_conn_ready

7: B3 Line contains hard tab characters (\t): "	parent = iso_get_sock(hdev, ...);"
8: B3 Line contains hard tab characters (\t): "	if (!parent)"
9: B3 Line contains hard tab characters (\t): "		return;"
11: B3 Line contains hard tab characters (\t): "	lock_sock(parent);"
12: B3 Line contains hard tab characters (\t): "	sk = iso_sock_alloc(sock_net(parent), NULL, BTPROTO_ISO, ...);"
13: B3 Line contains hard tab characters (\t): "	..."
14: B3 Line contains hard tab characters (\t): "	iso_chan_add(conn, sk, parent);"
15: B3 Line contains hard tab characters (\t): "	..."
16: B3 Line contains hard tab characters (\t): "	release_sock(parent);"
17: B3 Line contains hard tab characters (\t): "	sock_put(parent);"
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


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

---
Regards,
Linux Bluetooth