[PATCH] Bluetooth: hci_core: Fix unbalanced unlock in set_device_flags()

[PATCH] Bluetooth: hci_core: Fix unbalanced unlock in set_device_flags()

[Hans de Goede]

There is only one "goto done;" in set_device_flags() and this happens
*before* hci_dev_lock() is called, move the done label to after the
hci_dev_unlock() to fix the following unlock balance:

[   31.493567] =====================================
[   31.493571] WARNING: bad unlock balance detected!
[   31.493576] 5.17.0-rc2+ #13 Tainted: G         C  E
[   31.493581] -------------------------------------
[   31.493584] bluetoothd/685 is trying to release lock (&hdev->lock) at:
[   31.493594] [<ffffffffc07603f5>] set_device_flags+0x65/0x1f0 [bluetooth]
[   31.493684] but there are no more locks to release!

Note this bug has been around for a couple of years, but before
commit fe92ee6425a2 ("Bluetooth: hci_core: Rework hci_conn_params flags")
supported_flags was hardcoded to "((1U << HCI_CONN_FLAG_MAX) - 1)" so
the check for unsupported flags which does the "goto done;" never
triggered.

Fixes: fe92ee6425a2 ("Bluetooth: hci_core: Rework hci_conn_params flags")
Cc: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 net/bluetooth/mgmt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 37087cf7dc5a..f05b9b73e84b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4513,9 +4513,9 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
 		}
 	}
 
-done:
 	hci_dev_unlock(hdev);
 
+done:
 	if (status == MGMT_STATUS_SUCCESS)
 		device_flags_changed(sk, hdev, &cp->addr.bdaddr, cp->addr.type,
 				     supported_flags, current_flags);
-- 
2.35.1

RE: Bluetooth: hci_core: Fix unbalanced unlock in set_device_flags()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1377 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=619579

---Test result---

Test Summary:
CheckPatch                    PASS      1.68 seconds
GitLint                       PASS      0.97 seconds
SubjectPrefix                 PASS      0.85 seconds
BuildKernel                   PASS      36.20 seconds
BuildKernel32                 PASS      31.72 seconds
Incremental Build with patchesPASS      43.70 seconds
TestRunner: Setup             PASS      564.00 seconds
TestRunner: l2cap-tester      PASS      15.37 seconds
TestRunner: bnep-tester       PASS      7.22 seconds
TestRunner: mgmt-tester       PASS      117.53 seconds
TestRunner: rfcomm-tester     FAIL      9.09 seconds
TestRunner: sco-tester        PASS      8.99 seconds
TestRunner: smp-tester        PASS      8.92 seconds
TestRunner: userchan-tester   PASS      7.47 seconds

Details
##############################
Test: TestRunner: rfcomm-tester - FAIL - 9.09 seconds
Run test-runner with rfcomm-tester
Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0

Failed Test Cases
Basic RFCOMM Socket Client - Write 32k Success       Failed       0.188 seconds



---
Regards,
Linux Bluetooth

Re: Bluetooth: hci_core: Fix unbalanced unlock in set_device_flags()

[Hans de Goede]

Hi,

On 3/2/22 15:09, 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=619579
> 
> ---Test result---
> 
> Test Summary:
> CheckPatch                    PASS      1.68 seconds
> GitLint                       PASS      0.97 seconds
> SubjectPrefix                 PASS      0.85 seconds
> BuildKernel                   PASS      36.20 seconds
> BuildKernel32                 PASS      31.72 seconds
> Incremental Build with patchesPASS      43.70 seconds
> TestRunner: Setup             PASS      564.00 seconds
> TestRunner: l2cap-tester      PASS      15.37 seconds
> TestRunner: bnep-tester       PASS      7.22 seconds
> TestRunner: mgmt-tester       PASS      117.53 seconds
> TestRunner: rfcomm-tester     FAIL      9.09 seconds
> TestRunner: sco-tester        PASS      8.99 seconds
> TestRunner: smp-tester        PASS      8.92 seconds
> TestRunner: userchan-tester   PASS      7.47 seconds
> 
> Details
> ##############################
> Test: TestRunner: rfcomm-tester - FAIL - 9.09 seconds
> Run test-runner with rfcomm-tester
> Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0
> 
> Failed Test Cases
> Basic RFCOMM Socket Client - Write 32k Success       Failed       0.188 seconds

This seems like a false-positive failure, since I don't
see anyway how this patch can cause this issue.

Regards,

Hans

Re: Bluetooth: hci_core: Fix unbalanced unlock in set_device_flags()

[Luiz Augusto von Dentz]

Hi Hans,

On Wed, Mar 2, 2022 at 8:52 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 3/2/22 15:09, 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=619579
> >
> > ---Test result---
> >
> > Test Summary:
> > CheckPatch                    PASS      1.68 seconds
> > GitLint                       PASS      0.97 seconds
> > SubjectPrefix                 PASS      0.85 seconds
> > BuildKernel                   PASS      36.20 seconds
> > BuildKernel32                 PASS      31.72 seconds
> > Incremental Build with patchesPASS      43.70 seconds
> > TestRunner: Setup             PASS      564.00 seconds
> > TestRunner: l2cap-tester      PASS      15.37 seconds
> > TestRunner: bnep-tester       PASS      7.22 seconds
> > TestRunner: mgmt-tester       PASS      117.53 seconds
> > TestRunner: rfcomm-tester     FAIL      9.09 seconds
> > TestRunner: sco-tester        PASS      8.99 seconds
> > TestRunner: smp-tester        PASS      8.92 seconds
> > TestRunner: userchan-tester   PASS      7.47 seconds
> >
> > Details
> > ##############################
> > Test: TestRunner: rfcomm-tester - FAIL - 9.09 seconds
> > Run test-runner with rfcomm-tester
> > Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0
> >
> > Failed Test Cases
> > Basic RFCOMM Socket Client - Write 32k Success       Failed       0.188 seconds
>
> This seems like a false-positive failure, since I don't
> see anyway how this patch can cause this issue.

Its a known problem, CI is running against bluetooth-next.git while
the fix for this is on our stable bluetooth.git, once
bluetooth-next.git gets rebased this should go away.

-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: hci_core: Fix unbalanced unlock in set_device_flags()

[Marcel Holtmann]

Hi Hans,

> There is only one "goto done;" in set_device_flags() and this happens
> *before* hci_dev_lock() is called, move the done label to after the
> hci_dev_unlock() to fix the following unlock balance:
> 
> [   31.493567] =====================================
> [   31.493571] WARNING: bad unlock balance detected!
> [   31.493576] 5.17.0-rc2+ #13 Tainted: G         C  E
> [   31.493581] -------------------------------------
> [   31.493584] bluetoothd/685 is trying to release lock (&hdev->lock) at:
> [   31.493594] [<ffffffffc07603f5>] set_device_flags+0x65/0x1f0 [bluetooth]
> [   31.493684] but there are no more locks to release!
> 
> Note this bug has been around for a couple of years, but before
> commit fe92ee6425a2 ("Bluetooth: hci_core: Rework hci_conn_params flags")
> supported_flags was hardcoded to "((1U << HCI_CONN_FLAG_MAX) - 1)" so
> the check for unsupported flags which does the "goto done;" never
> triggered.
> 
> Fixes: fe92ee6425a2 ("Bluetooth: hci_core: Rework hci_conn_params flags")
> Cc: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> net/bluetooth/mgmt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

patch has been applied to bluetooth-stable tree.

Regards

Marcel