2022-09-13 23:33:24

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH] Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release

From: Luiz Augusto von Dentz <[email protected]>

Due to change to switch to use lock_sock inside rfcomm_sk_state_change
the socket shutdown/release procedure can cause a deadlock:

rfcomm_sock_shutdown():
lock_sock();
__rfcomm_sock_close():
rfcomm_dlc_close():
__rfcomm_dlc_close():
rfcomm_dlc_lock();
rfcomm_sk_state_change():
lock_sock();

To fix this when the call __rfcomm_sock_close is now done without
holding the lock_sock since rfcomm_dlc_lock exists to protect
the dlc data there is no need to use lock_sock in that code path.

Link: https://lore.kernel.org/all/CAD+dNTsbuU4w+Y_P7o+VEN7BYCAbZuwZx2+tH+OTzCdcZF82YA@mail.gmail.com/
Fixes: b7ce436a5d79 ("Bluetooth: switch to lock_sock in RFCOMM")
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 4bf4ea6cbb5e..21e24da4847f 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -902,7 +902,10 @@ static int rfcomm_sock_shutdown(struct socket *sock, int how)
lock_sock(sk);
if (!sk->sk_shutdown) {
sk->sk_shutdown = SHUTDOWN_MASK;
+
+ release_sock(sk);
__rfcomm_sock_close(sk);
+ lock_sock(sk);

if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime &&
!(current->flags & PF_EXITING))
--
2.37.3


2022-09-14 00:33:55

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release

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=676714

---Test result---

Test Summary:
CheckPatch PASS 0.92 seconds
GitLint FAIL 0.47 seconds
SubjectPrefix PASS 0.31 seconds
BuildKernel PASS 45.45 seconds
BuildKernel32 PASS 40.12 seconds
Incremental Build with patchesPASS 59.47 seconds
TestRunner: Setup PASS 667.48 seconds
TestRunner: l2cap-tester PASS 20.48 seconds
TestRunner: iso-tester PASS 20.45 seconds
TestRunner: bnep-tester PASS 7.74 seconds
TestRunner: mgmt-tester PASS 127.19 seconds
TestRunner: rfcomm-tester PASS 12.53 seconds
TestRunner: sco-tester PASS 11.90 seconds
TestRunner: smp-tester PASS 11.79 seconds
TestRunner: userchan-tester PASS 8.35 seconds

Details
##############################
Test: GitLint - FAIL - 0.47 seconds
Run gitlint with rule in .gitlint
Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release
21: B1 Line exceeds max length (101>80): "Link: https://lore.kernel.org/all/CAD+dNTsbuU4w+Y_P7o+VEN7BYCAbZuwZx2+tH+OTzCdcZF82YA@mail.gmail.com/"




---
Regards,
Linux Bluetooth

2022-09-14 19:15:45

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release

Hi,

On Tue, Sep 13, 2022 at 5:13 PM <[email protected]> wrote:
>
> 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=676714
>
> ---Test result---
>
> Test Summary:
> CheckPatch PASS 0.92 seconds
> GitLint FAIL 0.47 seconds
> SubjectPrefix PASS 0.31 seconds
> BuildKernel PASS 45.45 seconds
> BuildKernel32 PASS 40.12 seconds
> Incremental Build with patchesPASS 59.47 seconds
> TestRunner: Setup PASS 667.48 seconds
> TestRunner: l2cap-tester PASS 20.48 seconds
> TestRunner: iso-tester PASS 20.45 seconds
> TestRunner: bnep-tester PASS 7.74 seconds
> TestRunner: mgmt-tester PASS 127.19 seconds
> TestRunner: rfcomm-tester PASS 12.53 seconds
> TestRunner: sco-tester PASS 11.90 seconds
> TestRunner: smp-tester PASS 11.79 seconds
> TestRunner: userchan-tester PASS 8.35 seconds
>
> Details
> ##############################
> Test: GitLint - FAIL - 0.47 seconds
> Run gitlint with rule in .gitlint
> Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release
> 21: B1 Line exceeds max length (101>80): "Link: https://lore.kernel.org/all/CAD+dNTsbuU4w+Y_P7o+VEN7BYCAbZuwZx2+tH+OTzCdcZF82YA@mail.gmail.com/"

Let me know if this fixes the problems you are are having, note that I
did add a test for rfcomm-tester to verify the deadlock is fixed:

https://patchwork.kernel.org/project/bluetooth/patch/[email protected]/

--
Luiz Augusto von Dentz

2022-09-15 21:16:11

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Tue, 13 Sep 2022 16:31:57 -0700 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> Due to change to switch to use lock_sock inside rfcomm_sk_state_change
> the socket shutdown/release procedure can cause a deadlock:
>
> rfcomm_sock_shutdown():
> lock_sock();
> __rfcomm_sock_close():
> rfcomm_dlc_close():
> __rfcomm_dlc_close():
> rfcomm_dlc_lock();
> rfcomm_sk_state_change():
> lock_sock();
>
> [...]

Here is the summary with links:
- Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release
https://git.kernel.org/bluetooth/bluetooth-next/c/812e92b824c1

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html