syzbot reports a possible deadlock in rfcomm_sk_state_change [1].
While rfcomm_sock_connect acquires the sk lock and waits for
the rfcomm lock, rfcomm_sock_release could have the rfcomm
lock and hit a deadlock for acquiring the sk lock.
Here's a simplified flow:
rfcomm_sock_connect:
lock_sock(sk)
rfcomm_dlc_open:
rfcomm_lock()
rfcomm_sock_release:
rfcomm_sock_shutdown:
rfcomm_lock()
__rfcomm_dlc_close:
rfcomm_k_state_change:
lock_sock(sk)
This patch drops the sk lock before calling rfcomm_dlc_open to
avoid the possible deadlock and holds sk's reference count to
prevent use-after-free after rfcomm_dlc_open completes.
Reported-by: [email protected]
Fixes: 1804fdf6e494 ("Bluetooth: btintel: Combine setting up MSFT extension")
Link: https://syzkaller.appspot.com/bug?extid=d7ce59b06b3eb14fd218 [1]
Signed-off-by: Ying Hsu <[email protected]>
---
This commit has been tested with a C reproducer on qemu-x86_64
and a ChromeOS device.
Changes in v3:
- Revise the commit message.
Changes in v2:
- Fix potential use-after-free in rfc_comm_sock_connect.
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 21e24da4847f..4397e14ff560 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -391,6 +391,7 @@ static int rfcomm_sock_connect(struct socket *sock, struct sockaddr *addr, int a
addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
+ sock_hold(sk);
lock_sock(sk);
if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND) {
@@ -410,14 +411,18 @@ static int rfcomm_sock_connect(struct socket *sock, struct sockaddr *addr, int a
d->sec_level = rfcomm_pi(sk)->sec_level;
d->role_switch = rfcomm_pi(sk)->role_switch;
+ /* Drop sock lock to avoid potential deadlock with the RFCOMM lock */
+ release_sock(sk);
err = rfcomm_dlc_open(d, &rfcomm_pi(sk)->src, &sa->rc_bdaddr,
sa->rc_channel);
- if (!err)
+ lock_sock(sk);
+ if (!err && !sock_flag(sk, SOCK_ZAPPED))
err = bt_sock_wait_state(sk, BT_CONNECTED,
sock_sndtimeo(sk, flags & O_NONBLOCK));
done:
release_sock(sk);
+ sock_put(sk);
return err;
}
--
2.39.0.314.g84b9a713c41-goog
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=710770
---Test result---
Test Summary:
CheckPatch PASS 0.77 seconds
GitLint FAIL 0.53 seconds
SubjectPrefix PASS 0.05 seconds
BuildKernel PASS 30.88 seconds
CheckAllWarning PASS 33.71 seconds
CheckSparse PASS 38.46 seconds
CheckSmatch PASS 106.19 seconds
BuildKernel32 PASS 29.67 seconds
TestRunnerSetup PASS 427.02 seconds
TestRunner_l2cap-tester PASS 15.97 seconds
TestRunner_iso-tester PASS 16.16 seconds
TestRunner_bnep-tester PASS 5.52 seconds
TestRunner_mgmt-tester PASS 105.33 seconds
TestRunner_rfcomm-tester PASS 8.60 seconds
TestRunner_sco-tester PASS 7.89 seconds
TestRunner_ioctl-tester PASS 9.18 seconds
TestRunner_mesh-tester PASS 6.85 seconds
TestRunner_smp-tester PASS 7.81 seconds
TestRunner_userchan-tester PASS 5.69 seconds
IncrementalBuild PASS 27.91 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v3] Bluetooth: Fix possible deadlock in rfcomm_sk_state_change
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
19: B3 Line contains hard tab characters (\t): " lock_sock(sk)"
---
Regards,
Linux Bluetooth
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Wed, 11 Jan 2023 03:16:14 +0000 you wrote:
> syzbot reports a possible deadlock in rfcomm_sk_state_change [1].
> While rfcomm_sock_connect acquires the sk lock and waits for
> the rfcomm lock, rfcomm_sock_release could have the rfcomm
> lock and hit a deadlock for acquiring the sk lock.
> Here's a simplified flow:
>
> rfcomm_sock_connect:
> lock_sock(sk)
> rfcomm_dlc_open:
> rfcomm_lock()
>
> [...]
Here is the summary with links:
- [v3] Bluetooth: Fix possible deadlock in rfcomm_sk_state_change
https://git.kernel.org/bluetooth/bluetooth-next/c/7ed38304a633
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html