| Message ID | ZjO9qCx10KUJbK6w@neat |
|---|---|
| State | Accepted |
| Commit | f7bbc1e0ec8ff1dbdf68dadb99619707761da3e9 |
| Headers | show |
| Series | [next] Bluetooth: hci_conn: Use __counted_by() and avoid -Wfamnae warning | expand |
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=849960 ---Test result--- Test Summary: CheckPatch PASS 1.64 seconds GitLint FAIL 0.68 seconds SubjectPrefix PASS 0.07 seconds BuildKernel PASS 30.02 seconds CheckAllWarning PASS 32.49 seconds CheckSparse PASS 38.56 seconds CheckSmatch FAIL 35.75 seconds BuildKernel32 PASS 28.86 seconds TestRunnerSetup PASS 529.44 seconds TestRunner_l2cap-tester PASS 20.60 seconds TestRunner_iso-tester FAIL 32.45 seconds TestRunner_bnep-tester PASS 4.69 seconds TestRunner_mgmt-tester FAIL 112.58 seconds TestRunner_rfcomm-tester PASS 7.45 seconds TestRunner_sco-tester PASS 15.15 seconds TestRunner_ioctl-tester PASS 7.86 seconds TestRunner_mesh-tester PASS 5.89 seconds TestRunner_smp-tester PASS 6.77 seconds TestRunner_userchan-tester PASS 5.07 seconds IncrementalBuild PASS 28.43 seconds Details ############################## Test: GitLint - FAIL Desc: Run gitlint Output: [next] Bluetooth: hci_conn: Use __counted_by() and avoid -Wfamnae warning 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 18: B1 Line exceeds max length (158>80): "net/bluetooth/hci_conn.c:669:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]" ############################## Test: CheckSmatch - FAIL Desc: Run smatch tool with source Output: Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139 make[4]: *** Deleting file 'net/bluetooth/hci_core.o' make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2 make[2]: *** [scripts/Makefile.build:485: net] Error 2 make[2]: *** Waiting for unfinished jobs.... Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139 make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o' make[4]: *** Waiting for unfinished jobs.... make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2 make[2]: *** [scripts/Makefile.build:485: drivers] Error 2 make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2 make: *** [Makefile:240: __sub-make] Error 2 ############################## Test: TestRunner_iso-tester - FAIL Desc: Run iso-tester with test-runner Output: Total: 122, Passed: 121 (99.2%), Failed: 1, Not Run: 0 Failed Test Cases ISO Connect Suspend - Success Failed 4.189 seconds ############################## 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 6 (RL is full) Failed 0.191 seconds --- Regards, Linux Bluetooth
On Thu, May 02, 2024 at 10:22:00AM -0600, Gustavo A. R. Silva wrote: > Prepare for the coming implementation by GCC and Clang of the > __counted_by attribute. Flexible array members annotated with > __counted_by can have their accesses bounds-checked at run-time > via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE > (for strcpy/memcpy-family functions). > > Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are > getting ready to enable it globally. > > So, use the `DEFINE_FLEX()` helper for an on-stack definition of > a flexible structure where the size of the flexible-array member > is known at compile-time, and refactor the rest of the code, > accordingly. > > With these changes, fix the following warning: > net/bluetooth/hci_conn.c:669:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > > Link: https://github.com/KSPP/linux/issues/202 > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Nice! This looks really clean; I'll point people at this patch when they want to see these kinds of conversions. It has it all! :) Reviewed-by: Kees Cook <keescook@chromium.org>
>> So, use the `DEFINE_FLEX()` helper for an on-stack definition of >> a flexible structure where the size of the flexible-array member >> is known at compile-time, and refactor the rest of the code, >> accordingly. >> >> With these changes, fix the following warning: >> net/bluetooth/hci_conn.c:669:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] >> >> Link: https://github.com/KSPP/linux/issues/202 >> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > Nice! This looks really clean; I'll point people at this patch when they > want to see these kinds of conversions. It has it all! :) I really enjoyed writing it! It was great to find out I could remove that global struct. :) > > Reviewed-by: Kees Cook <keescook@chromium.org> > Thanks! -- Gustavo
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Thu, 2 May 2024 10:22:00 -0600 you wrote: > Prepare for the coming implementation by GCC and Clang of the > __counted_by attribute. Flexible array members annotated with > __counted_by can have their accesses bounds-checked at run-time > via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE > (for strcpy/memcpy-family functions). > > Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are > getting ready to enable it globally. > > [...] Here is the summary with links: - [next] Bluetooth: hci_conn: Use __counted_by() and avoid -Wfamnae warning https://git.kernel.org/bluetooth/bluetooth-next/c/6d18d5b03ee1 You are awesome, thank you!
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index c4c6b8810701..e6838321f4d9 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -2144,7 +2144,7 @@ struct hci_cp_le_set_cig_params { __le16 c_latency; __le16 p_latency; __u8 num_cis; - struct hci_cis_params cis[]; + struct hci_cis_params cis[] __counted_by(num_cis); } __packed; struct hci_rp_le_set_cig_params { diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index c508609be105..6e60e8287956 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -665,11 +665,6 @@ static void le_conn_timeout(struct work_struct *work) hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM); } -struct iso_cig_params { - struct hci_cp_le_set_cig_params cp; - struct hci_cis_params cis[0x1f]; -}; - struct iso_list_data { union { u8 cig; @@ -1725,34 +1720,33 @@ static int hci_le_create_big(struct hci_conn *conn, struct bt_iso_qos *qos) static int set_cig_params_sync(struct hci_dev *hdev, void *data) { + DEFINE_FLEX(struct hci_cp_le_set_cig_params, pdu, cis, num_cis, 0x1f); u8 cig_id = PTR_UINT(data); struct hci_conn *conn; struct bt_iso_qos *qos; - struct iso_cig_params pdu; + u8 aux_num_cis = 0; u8 cis_id; conn = hci_conn_hash_lookup_cig(hdev, cig_id); if (!conn) return 0; - memset(&pdu, 0, sizeof(pdu)); - qos = &conn->iso_qos; - pdu.cp.cig_id = cig_id; - hci_cpu_to_le24(qos->ucast.out.interval, pdu.cp.c_interval); - hci_cpu_to_le24(qos->ucast.in.interval, pdu.cp.p_interval); - pdu.cp.sca = qos->ucast.sca; - pdu.cp.packing = qos->ucast.packing; - pdu.cp.framing = qos->ucast.framing; - pdu.cp.c_latency = cpu_to_le16(qos->ucast.out.latency); - pdu.cp.p_latency = cpu_to_le16(qos->ucast.in.latency); + pdu->cig_id = cig_id; + hci_cpu_to_le24(qos->ucast.out.interval, pdu->c_interval); + hci_cpu_to_le24(qos->ucast.in.interval, pdu->p_interval); + pdu->sca = qos->ucast.sca; + pdu->packing = qos->ucast.packing; + pdu->framing = qos->ucast.framing; + pdu->c_latency = cpu_to_le16(qos->ucast.out.latency); + pdu->p_latency = cpu_to_le16(qos->ucast.in.latency); /* Reprogram all CIS(s) with the same CIG, valid range are: * num_cis: 0x00 to 0x1F * cis_id: 0x00 to 0xEF */ for (cis_id = 0x00; cis_id < 0xf0 && - pdu.cp.num_cis < ARRAY_SIZE(pdu.cis); cis_id++) { + aux_num_cis < pdu->num_cis; cis_id++) { struct hci_cis_params *cis; conn = hci_conn_hash_lookup_cis(hdev, NULL, 0, cig_id, cis_id); @@ -1761,7 +1755,7 @@ static int set_cig_params_sync(struct hci_dev *hdev, void *data) qos = &conn->iso_qos; - cis = &pdu.cis[pdu.cp.num_cis++]; + cis = &pdu->cis[aux_num_cis++]; cis->cis_id = cis_id; cis->c_sdu = cpu_to_le16(conn->iso_qos.ucast.out.sdu); cis->p_sdu = cpu_to_le16(conn->iso_qos.ucast.in.sdu); @@ -1772,14 +1766,14 @@ static int set_cig_params_sync(struct hci_dev *hdev, void *data) cis->c_rtn = qos->ucast.out.rtn; cis->p_rtn = qos->ucast.in.rtn; } + pdu->num_cis = aux_num_cis; - if (!pdu.cp.num_cis) + if (!pdu->num_cis) return 0; return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_CIG_PARAMS, - sizeof(pdu.cp) + - pdu.cp.num_cis * sizeof(pdu.cis[0]), &pdu, - HCI_CMD_TIMEOUT); + struct_size(pdu, cis, pdu->num_cis), + pdu, HCI_CMD_TIMEOUT); } static bool hci_le_set_cig_params(struct hci_conn *conn, struct bt_iso_qos *qos)
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally. So, use the `DEFINE_FLEX()` helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly. With these changes, fix the following warning: net/bluetooth/hci_conn.c:669:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] Link: https://github.com/KSPP/linux/issues/202 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- include/net/bluetooth/hci.h | 2 +- net/bluetooth/hci_conn.c | 38 ++++++++++++++++--------------------- 2 files changed, 17 insertions(+), 23 deletions(-)