bluetooth: handle value within the ida range should not be handled in BIG

Commit Message

Edward Adam Davis June 16, 2024, 10:20 a.m. UTC

hci_le_big_sync_established_evt is necessary to filter out cases where the handle
value is belone to ida id range, otherwise ida will be erroneously released in 
hci_conn_cleanup. 

Fixes: 181a42edddf5 ("Bluetooth: Make handle of hci_conn be unique")
Reported-by: [e-mail]
Signed-off-by: Edward Adam Davis <[e-mail]>
---
 net/bluetooth/hci_event.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Pavel Skripkin June 16, 2024, 10:37 a.m. UTC | #1

On 6/16/24 1:20 PM, 'Edward Adam Davis' via syzkaller-bugs wrote:
> hci_le_big_sync_established_evt is necessary to filter out cases where the handle
> value is belone to ida id range, otherwise ida will be erroneously released in
> hci_conn_cleanup.
> 
> Fixes: 181a42edddf5 ("Bluetooth: Make handle of hci_conn be unique")
> Reported-by: [e-mail]
> Signed-off-by: Edward Adam Davis <[e-mail]>
> ---

There is one more user of  `hci_conn_add` which may pass too big handle 
which is `hci_le_cis_req_evt`.

I think, it should be resolved on API level as I tried to test here [0], 
but syzbot is feeling bad for some reason


[0] 
https://lore.kernel.org/all/[e-mail]/

>   net/bluetooth/hci_event.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index a487f9df8145..4130d64d9a80 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -6893,6 +6893,9 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
>   
>   		bis = hci_conn_hash_lookup_handle(hdev, handle);
>   		if (!bis) {
> +                        if (handle > HCI_CONN_HANDLE_MAX)
> +                               continue;
> +
>   			bis = hci_conn_add(hdev, ISO_LINK, BDADDR_ANY,
>   					   HCI_ROLE_SLAVE, handle);
>   			if (IS_ERR(bis))

[e-mail] June 16, 2024, 10:59 a.m. UTC | #2

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

---Test result---

Test Summary:
CheckPatch                    FAIL      0.86 seconds
GitLint                       FAIL      0.47 seconds
SubjectPrefix                 FAIL      0.28 seconds
BuildKernel                   PASS      29.86 seconds
CheckAllWarning               PASS      33.44 seconds
CheckSparse                   WARNING   38.13 seconds
CheckSmatch                   WARNING   102.54 seconds
BuildKernel32                 PASS      29.65 seconds
TestRunnerSetup               PASS      527.95 seconds
TestRunner_l2cap-tester       PASS      18.19 seconds
TestRunner_iso-tester         PASS      32.28 seconds
TestRunner_bnep-tester        PASS      4.81 seconds
TestRunner_mgmt-tester        FAIL      110.14 seconds
TestRunner_rfcomm-tester      PASS      7.36 seconds
TestRunner_sco-tester         PASS      15.04 seconds
TestRunner_ioctl-tester       PASS      10.65 seconds
TestRunner_mesh-tester        PASS      7.88 seconds
TestRunner_smp-tester         PASS      6.80 seconds
TestRunner_userchan-tester    PASS      4.96 seconds
IncrementalBuild              PASS      27.95 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
bluetooth: handle value within the ida range should not be handled in BIG
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#89: 
hci_le_big_sync_established_evt is necessary to filter out cases where the handle

WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#94: 
Reported-by: [e-mail]
Signed-off-by: Edward Adam Davis <[e-mail]>

ERROR: code indent should use tabs where possible
#108: FILE: net/bluetooth/hci_event.c:6896:
+                        if (handle > HCI_CONN_HANDLE_MAX)$

WARNING: please, no spaces at the start of a line
#108: FILE: net/bluetooth/hci_event.c:6896:
+                        if (handle > HCI_CONN_HANDLE_MAX)$

WARNING: suspect code indent for conditional statements (24, 31)
#108: FILE: net/bluetooth/hci_event.c:6896:
+                        if (handle > HCI_CONN_HANDLE_MAX)
+                               continue;

ERROR: code indent should use tabs where possible
#109: FILE: net/bluetooth/hci_event.c:6897:
+                               continue;$

WARNING: please, no spaces at the start of a line
#109: FILE: net/bluetooth/hci_event.c:6897:
+                               continue;$

total: 2 errors, 5 warnings, 0 checks, 9 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

/github/workspace/src/src/13699512.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
bluetooth: handle value within the ida range should not be handled in BIG

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
3: B1 Line exceeds max length (81>80): "hci_le_big_sync_established_evt is necessary to filter out cases where the handle"
4: B2 Line has trailing whitespace: "value is belone to ida id range, otherwise ida will be erroneously released in "
5: B2 Line has trailing whitespace: "hci_conn_cleanup. "
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 489 (99.4%), Failed: 1, Not Run: 2

Failed Test Cases
LL Privacy - Add Device 7 (AL is full)               Failed       0.190 seconds


---
Regards,
Linux Bluetooth

Edward Adam Davis June 16, 2024, 1:18 p.m. UTC | #3

On Sun, 16 Jun 2024 13:37:17 +0300, Pavel Skripkin wrote:
> > hci_le_big_sync_established_evt is necessary to filter out cases where the handle
> > value is belone to ida id range, otherwise ida will be erroneously released in
> > hci_conn_cleanup.
> >
> > Fixes: 181a42edddf5 ("Bluetooth: Make handle of hci_conn be unique")
> > Reported-by: [e-mail]
> > Signed-off-by: Edward Adam Davis <[e-mail]>
> > ---
> 
> There is one more user of  `hci_conn_add` which may pass too big handle
> which is `hci_le_cis_req_evt`.
This case only affect hci_le_big_sync_established_evt.

--
Edward

Patch

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a487f9df8145..4130d64d9a80 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -6893,6 +6893,9 @@  static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
 
 		bis = hci_conn_hash_lookup_handle(hdev, handle);
 		if (!bis) {
+                        if (handle > HCI_CONN_HANDLE_MAX)
+                               continue;
+
 			bis = hci_conn_add(hdev, ISO_LINK, BDADDR_ANY,
 					   HCI_ROLE_SLAVE, handle);
 			if (IS_ERR(bis))