The maximum CIS_Count is 0x1f (Core v5.3 Vol 4 Part E Sec 7.8.97).
Fixes: 26afbd826ee3 ("Bluetooth: Add initial implementation of CIS connections")
Signed-off-by: Pauli Virtanen <[email protected]>
---
net/bluetooth/hci_conn.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 1f906f8508bc..7b1a83ec50ae 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -788,7 +788,7 @@ struct iso_list_data {
int count;
struct {
struct hci_cp_le_set_cig_params cp;
- struct hci_cis_params cis[0x11];
+ struct hci_cis_params cis[0x1f];
} pdu;
};
@@ -1815,7 +1815,8 @@ static bool hci_le_set_cig_params(struct hci_conn *conn, struct bt_iso_qos *qos)
}
/* Reprogram all CIS(s) with the same CIG */
- for (data.cig = qos->ucast.cig, data.cis = 0x00; data.cis < 0x11;
+ for (data.cig = qos->ucast.cig, data.cis = 0x00;
+ data.cis < ARRAY_SIZE(data.pdu.cis);
data.cis++) {
data.count = 0;
--
2.40.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=751684
---Test result---
Test Summary:
CheckPatch PASS 5.24 seconds
GitLint PASS 1.85 seconds
SubjectPrefix PASS 0.61 seconds
BuildKernel PASS 37.62 seconds
CheckAllWarning PASS 40.98 seconds
CheckSparse WARNING 46.29 seconds
CheckSmatch WARNING 126.89 seconds
BuildKernel32 PASS 36.43 seconds
TestRunnerSetup PASS 517.97 seconds
TestRunner_l2cap-tester PASS 19.01 seconds
TestRunner_iso-tester PASS 25.59 seconds
TestRunner_bnep-tester PASS 6.55 seconds
TestRunner_mgmt-tester PASS 129.05 seconds
TestRunner_rfcomm-tester PASS 10.34 seconds
TestRunner_sco-tester PASS 9.55 seconds
TestRunner_ioctl-tester PASS 11.24 seconds
TestRunner_mesh-tester PASS 8.29 seconds
TestRunner_smp-tester PASS 9.34 seconds
TestRunner_userchan-tester PASS 6.83 seconds
IncrementalBuild PASS 90.25 seconds
Details
##############################
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):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):net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
---
Regards,
Linux Bluetooth
su, 2023-05-28 kello 17:44 +0000, Pauli Virtanen kirjoitti:
> The maximum CIS_Count is 0x1f (Core v5.3 Vol 4 Part E Sec 7.8.97).
>
> Fixes: 26afbd826ee3 ("Bluetooth: Add initial implementation of CIS connections")
> Signed-off-by: Pauli Virtanen <[email protected]>
> ---
> net/bluetooth/hci_conn.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 1f906f8508bc..7b1a83ec50ae 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -788,7 +788,7 @@ struct iso_list_data {
> int count;
> struct {
> struct hci_cp_le_set_cig_params cp;
> - struct hci_cis_params cis[0x11];
> + struct hci_cis_params cis[0x1f];
> } pdu;
> };
>
> @@ -1815,7 +1815,8 @@ static bool hci_le_set_cig_params(struct hci_conn *conn, struct bt_iso_qos *qos)
> }
>
> /* Reprogram all CIS(s) with the same CIG */
> - for (data.cig = qos->ucast.cig, data.cis = 0x00; data.cis < 0x11;
> + for (data.cig = qos->ucast.cig, data.cis = 0x00;
> + data.cis < ARRAY_SIZE(data.pdu.cis);
> data.cis++) {
> data.count = 0;
>
Probably should also have cleaned up this loop while at it, the command
takes 0x1f configurations at most, but CIS IDs are <= 0xef. For v2...
--
Pauli Virtanen
Hi Pauli,
On Sun, May 28, 2023 at 4:58 PM Pauli Virtanen <[email protected]> wrote:
>
> su, 2023-05-28 kello 17:44 +0000, Pauli Virtanen kirjoitti:
> > The maximum CIS_Count is 0x1f (Core v5.3 Vol 4 Part E Sec 7.8.97).
> >
> > Fixes: 26afbd826ee3 ("Bluetooth: Add initial implementation of CIS connections")
> > Signed-off-by: Pauli Virtanen <[email protected]>
> > ---
> > net/bluetooth/hci_conn.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 1f906f8508bc..7b1a83ec50ae 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -788,7 +788,7 @@ struct iso_list_data {
> > int count;
> > struct {
> > struct hci_cp_le_set_cig_params cp;
> > - struct hci_cis_params cis[0x11];
> > + struct hci_cis_params cis[0x1f];
> > } pdu;
> > };
> >
> > @@ -1815,7 +1815,8 @@ static bool hci_le_set_cig_params(struct hci_conn *conn, struct bt_iso_qos *qos)
> > }
> >
> > /* Reprogram all CIS(s) with the same CIG */
> > - for (data.cig = qos->ucast.cig, data.cis = 0x00; data.cis < 0x11;
> > + for (data.cig = qos->ucast.cig, data.cis = 0x00;
> > + data.cis < ARRAY_SIZE(data.pdu.cis);
> > data.cis++) {
> > data.count = 0;
> >
>
> Probably should also have cleaned up this loop while at it, the command
> takes 0x1f configurations at most, but CIS IDs are <= 0xef. For v2...
Can you prioritize this set? I'd like to have it merged asap since
without it CI is failing some tests.
--
Luiz Augusto von Dentz
Hi Luiz,
ke, 2023-05-31 kello 12:16 -0700, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
>
> On Sun, May 28, 2023 at 4:58 PM Pauli Virtanen <[email protected]> wrote:
> >
> > su, 2023-05-28 kello 17:44 +0000, Pauli Virtanen kirjoitti:
> > > The maximum CIS_Count is 0x1f (Core v5.3 Vol 4 Part E Sec 7.8.97).
> > >
> > > Fixes: 26afbd826ee3 ("Bluetooth: Add initial implementation of CIS connections")
> > > Signed-off-by: Pauli Virtanen <[email protected]>
> > > ---
> > > net/bluetooth/hci_conn.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > > index 1f906f8508bc..7b1a83ec50ae 100644
> > > --- a/net/bluetooth/hci_conn.c
> > > +++ b/net/bluetooth/hci_conn.c
> > > @@ -788,7 +788,7 @@ struct iso_list_data {
> > > int count;
> > > struct {
> > > struct hci_cp_le_set_cig_params cp;
> > > - struct hci_cis_params cis[0x11];
> > > + struct hci_cis_params cis[0x1f];
> > > } pdu;
> > > };
> > >
> > > @@ -1815,7 +1815,8 @@ static bool hci_le_set_cig_params(struct hci_conn *conn, struct bt_iso_qos *qos)
> > > }
> > >
> > > /* Reprogram all CIS(s) with the same CIG */
> > > - for (data.cig = qos->ucast.cig, data.cis = 0x00; data.cis < 0x11;
> > > + for (data.cig = qos->ucast.cig, data.cis = 0x00;
> > > + data.cis < ARRAY_SIZE(data.pdu.cis);
> > > data.cis++) {
> > > data.count = 0;
> > >
> >
> > Probably should also have cleaned up this loop while at it, the command
> > takes 0x1f configurations at most, but CIS IDs are <= 0xef. For v2...
>
> Can you prioritize this set? I'd like to have it merged asap since
> without it CI is failing some tests.
>
Sure, I'll do this first.
This is a bit delayed by:
Returning any error directly from hci_le_set_cig_params can trigger
kernel use-after-free in the emulator. If that's a problem, you may
want to hold off the patches that add new checks there.
Like this: in bluetooth-next/master change it to always error
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 795b2daa5bac..2d95b9460af3 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1788,6 +1788,8 @@ static bool hci_le_set_cig_params(struct hci_conn *conn, struct bt_iso_qos *qos)
struct hci_dev *hdev = conn->hdev;
struct iso_list_data data;
+ return false;
+
memset(&data, 0, sizeof(data));
/* Allocate first still reconfigurable CIG if not set */
Then
./tools/test-runner -k../linux/arch/x86_64/boot/bzImage -- ./tools/iso-tester -s "ISO QoS 8_1_1 - Success"
...
==================================================================
BUG: KASAN: slab-use-after-free in
hci_update_passive_scan_sync+0x780/0xf80
Read of size 8 at addr ffff888001b11c90 by task kworker/u3:2/43
CPU: 0 PID: 43 Comm: kworker/u3:2 Not tainted 6.3.0-rc7-02344-
g533e9a458c74-dirty #190
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38
04/01/2014
Workqueue: hci0 hci_cmd_sync_work
Call Trace:
<TASK>
dump_stack_lvl+0x1d/0x30
print_report+0xce/0x620
? __hci_cmd_sync_status_sk+0x63/0xb0
? __virt_addr_valid+0xd8/0x160
? hci_update_passive_scan_sync+0x780/0xf80
kasan_report+0xb9/0xe0
? hci_update_passive_scan_sync+0x780/0xf80
hci_update_passive_scan_sync+0x780/0xf80
? __pfx_hci_update_passive_scan_sync+0x10/0x10
? mutex_lock+0x11/0xe0
? __pfx_mutex_lock+0x10/0x10
? __pfx_mutex_unlock+0x10/0x10
? __pfx_update_passive_scan_sync+0x10/0x10
hci_cmd_sync_work+0xf3/0x1d0
process_one_work+0x3fe/0x6a0
worker_thread+0xbb/0x720
? __pfx_worker_thread+0x10/0x10
kthread+0x142/0x170
? __pfx_kthread+0x10/0x10
ret_from_fork+0x2b/0x50
</TASK>
Allocated by task 29:
Freed by task 15:
The buggy address belongs to the object at ffff888001b11c80
which belongs to the cache kmalloc-96 of size 96
The buggy address is located 16 bytes inside of
freed 96-byte region [ffff888001b11c80, ffff888001b11ce0)
The buggy address belongs to the physical page:
Memory state around the buggy address:
ffff888001b11b80: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
ffff888001b11c00: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
>ffff888001b11c80: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
^
ffff888001b11d00: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
ffff888001b11d80: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
==================================================================
stack segment: 0000 [#1] PREEMPT KASAN PTI
CPU: 0 PID: 43 Comm: kworker/u3:2 Tainted: G B 6.3.0-
rc7-02344-g533e9a458c74-dirty #190
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38
04/01/2014
Workqueue: hci0 hci_cmd_sync_work
RIP: 0010:hci_le_add_accept_list_sync+0x11c/0x3d0
Code: 89 f7 e8 a7 1e 9f ff 4c 89 f7 e8 8f 0d 9f ff 48 8b 83 c8 10 00 00
48 0f ba e0 2b 72 25 48 8d 7d 26 4c 8d 75 20 e8 b4 09 9f ff <0f> b6 55
26 4c 89 f6 48 89 df e8 2
RSP: 0018:ffff888001f3fbe8 EFLAGS: 00010292
RAX: 0000000000000000 RBX: ffff888001dd0000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff888001f3fbb0
RBP: dead0000000000f0 R08: 0000000000000000 R09: ffff888001dd10cf
R10: ffffed10003ba219 R11: 0000000000000001 R12: 1ffff110003e7f80
R13: ffff888001f3fd28 R14: dead000000000110 R15: dead0000000000f0
FS: 0000000000000000(0000) GS:ffffffff82e9d000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f9617e48000 CR3: 0000000001ace000 CR4: 00000000000006f0
Call Trace:
<TASK>
? add_taint+0x25/0x90
? __pfx_hci_le_add_accept_list_sync+0x10/0x10
? hci_update_passive_scan_sync+0x780/0xf80
hci_update_passive_scan_sync+0x79f/0xf80
? __pfx_hci_update_passive_scan_sync+0x10/0x10
? mutex_lock+0x11/0xe0
? __pfx_mutex_lock+0x10/0x10
? __pfx_mutex_unlock+0x10/0x10
? __pfx_update_passive_scan_sync+0x10/0x10
hci_cmd_sync_work+0xf3/0x1d0
process_one_work+0x3fe/0x6a0
worker_thread+0xbb/0x720
? __pfx_worker_thread+0x10/0x10
kthread+0x142/0x170
? __pfx_kthread+0x10/0x10
ret_from_fork+0x2b/0x50
</TASK>
---[ end trace 0000000000000000 ]---
RIP: 0010:hci_le_add_accept_list_sync+0x11c/0x3d0
Code: 89 f7 e8 a7 1e 9f ff 4c 89 f7 e8 8f 0d 9f ff 48 8b 83 c8 10 00 00
48 0f ba e0 2b 72 25 48 8d 7d 26 4c 8d 75 20 e8 b4 09 9f ff <0f> b6 55
26 4c 89 f6 48 89 df e8 2
RSP: 0018:ffff888001f3fbe8 EFLAGS: 00010292
RAX: 0000000000000000 RBX: ffff888001dd0000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff888001f3fbb0
RBP: dead0000000000f0 R08: 0000000000000000 R09: ffff888001dd10cf
R10: ffffed10003ba219 R11: 0000000000000001 R12: 1ffff110003e7f80
R13: ffff888001f3fd28 R14: dead000000000110 R15: dead0000000000f0
FS: 0000000000000000(0000) GS:ffffffff82e9d000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f9617e48000 CR3: 0000000001ace000 CR4: 00000000000006f0
Didn't manage to find out how to fix that, but some tracebacks pointed
toward hci_conn_params of the CIS.
--
Pauli Virtanen