diff mbox series

Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release

Message ID 20220913233157.526041-1-luiz.dentz@gmail.com
State New
Headers show
Series Bluetooth: RFCOMM: Fix possible deadlock on socket shutdown/release | expand

Commit Message

Luiz Augusto von Dentz Sept. 13, 2022, 11:31 p.m. UTC 
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

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 <luiz.von.dentz@intel.com>
---
 net/bluetooth/rfcomm/sock.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

bluez.test.bot@gmail.com Sept. 14, 2022, 12:13 a.m. UTC | #1 
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
Luiz Augusto von Dentz Sept. 14, 2022, 6:57 p.m. UTC | #2 
Hi,

On Tue, Sep 13, 2022 at 5:13 PM <bluez.test.bot@gmail.com> 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/20220913233349.526675-1-luiz.dentz@gmail.com/
patchwork-bot+bluetooth@kernel.org Sept. 15, 2022, 9:10 p.m. UTC | #3 
Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Tue, 13 Sep 2022 16:31:57 -0700 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> 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!
diff mbox series

Patch

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))