Commit c4585edf authored by Gustavo A. R. Silva's avatar Gustavo A. R. Silva Committed by Luiz Augusto von Dentz

Bluetooth: hci_conn, hci_sync: Use __counted_by() to avoid -Wfamnae warnings

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 multiple on-stack definitions
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.

Notice that, due to the use of `__counted_by()` in `struct
hci_cp_le_create_cis`, the for loop in function `hci_cs_le_create_cis()`
had to be modified. Once the index `i`, through which `cp->cis[i]` is
accessed, falls in the interval [0, cp->num_cis), `cp->num_cis` cannot
be decremented all the way down to zero while accessing `cp->cis[]`:

net/bluetooth/hci_event.c:4310:
4310    for (i = 0; cp->num_cis; cp->num_cis--, i++) {
                ...
4314            handle = __le16_to_cpu(cp->cis[i].cis_handle);

otherwise, only half (one iteration before `cp->num_cis == i`) or half
plus one (one iteration before `cp->num_cis < i`) of the items in the
array will be accessed before running into an out-of-bounds issue. So,
in order to avoid this, set `cp->num_cis` to zero just after the for
loop.

Also, make use of `aux_num_cis` variable to update `cmd->num_cis` after
a `list_for_each_entry_rcu()` loop.

With these changes, fix the following warnings:
net/bluetooth/hci_sync.c:1239:56: warning: structure containing a flexible
array member is not at the end of another structure
[-Wflex-array-member-not-at-end]
net/bluetooth/hci_sync.c:1415:51: warning: structure containing a flexible
array member is not at the end of another structure
[-Wflex-array-member-not-at-end]
net/bluetooth/hci_sync.c:1731:51: warning: structure containing a flexible
array member is not at the end of another structure
[-Wflex-array-member-not-at-end]
net/bluetooth/hci_sync.c:6497:45: 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/202Signed-off-by: default avatarGustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: default avatarLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
parent 3487cda2
...@@ -2026,7 +2026,7 @@ struct hci_cp_le_set_ext_adv_data { ...@@ -2026,7 +2026,7 @@ struct hci_cp_le_set_ext_adv_data {
__u8 operation; __u8 operation;
__u8 frag_pref; __u8 frag_pref;
__u8 length; __u8 length;
__u8 data[]; __u8 data[] __counted_by(length);
} __packed; } __packed;
#define HCI_OP_LE_SET_EXT_SCAN_RSP_DATA 0x2038 #define HCI_OP_LE_SET_EXT_SCAN_RSP_DATA 0x2038
...@@ -2035,7 +2035,7 @@ struct hci_cp_le_set_ext_scan_rsp_data { ...@@ -2035,7 +2035,7 @@ struct hci_cp_le_set_ext_scan_rsp_data {
__u8 operation; __u8 operation;
__u8 frag_pref; __u8 frag_pref;
__u8 length; __u8 length;
__u8 data[]; __u8 data[] __counted_by(length);
} __packed; } __packed;
#define HCI_OP_LE_SET_EXT_ADV_ENABLE 0x2039 #define HCI_OP_LE_SET_EXT_ADV_ENABLE 0x2039
...@@ -2061,7 +2061,7 @@ struct hci_cp_le_set_per_adv_data { ...@@ -2061,7 +2061,7 @@ struct hci_cp_le_set_per_adv_data {
__u8 handle; __u8 handle;
__u8 operation; __u8 operation;
__u8 length; __u8 length;
__u8 data[]; __u8 data[] __counted_by(length);
} __packed; } __packed;
#define HCI_OP_LE_SET_PER_ADV_ENABLE 0x2040 #define HCI_OP_LE_SET_PER_ADV_ENABLE 0x2040
...@@ -2162,7 +2162,7 @@ struct hci_cis { ...@@ -2162,7 +2162,7 @@ struct hci_cis {
struct hci_cp_le_create_cis { struct hci_cp_le_create_cis {
__u8 num_cis; __u8 num_cis;
struct hci_cis cis[]; struct hci_cis cis[] __counted_by(num_cis);
} __packed; } __packed;
#define HCI_OP_LE_REMOVE_CIG 0x2065 #define HCI_OP_LE_REMOVE_CIG 0x2065
......
...@@ -4307,7 +4307,7 @@ static void hci_cs_le_create_cis(struct hci_dev *hdev, u8 status) ...@@ -4307,7 +4307,7 @@ static void hci_cs_le_create_cis(struct hci_dev *hdev, u8 status)
hci_dev_lock(hdev); hci_dev_lock(hdev);
/* Remove connection if command failed */ /* Remove connection if command failed */
for (i = 0; cp->num_cis; cp->num_cis--, i++) { for (i = 0; i < cp->num_cis; i++) {
struct hci_conn *conn; struct hci_conn *conn;
u16 handle; u16 handle;
...@@ -4323,6 +4323,7 @@ static void hci_cs_le_create_cis(struct hci_dev *hdev, u8 status) ...@@ -4323,6 +4323,7 @@ static void hci_cs_le_create_cis(struct hci_dev *hdev, u8 status)
hci_conn_del(conn); hci_conn_del(conn);
} }
} }
cp->num_cis = 0;
if (pending) if (pending)
hci_le_create_cis_pending(hdev); hci_le_create_cis_pending(hdev);
......
...@@ -1235,31 +1235,27 @@ int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance) ...@@ -1235,31 +1235,27 @@ int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
static int hci_set_ext_scan_rsp_data_sync(struct hci_dev *hdev, u8 instance) static int hci_set_ext_scan_rsp_data_sync(struct hci_dev *hdev, u8 instance)
{ {
struct { DEFINE_FLEX(struct hci_cp_le_set_ext_scan_rsp_data, pdu, data, length,
struct hci_cp_le_set_ext_scan_rsp_data cp; HCI_MAX_EXT_AD_LENGTH);
u8 data[HCI_MAX_EXT_AD_LENGTH];
} pdu;
u8 len; u8 len;
struct adv_info *adv = NULL; struct adv_info *adv = NULL;
int err; int err;
memset(&pdu, 0, sizeof(pdu));
if (instance) { if (instance) {
adv = hci_find_adv_instance(hdev, instance); adv = hci_find_adv_instance(hdev, instance);
if (!adv || !adv->scan_rsp_changed) if (!adv || !adv->scan_rsp_changed)
return 0; return 0;
} }
len = eir_create_scan_rsp(hdev, instance, pdu.data); len = eir_create_scan_rsp(hdev, instance, pdu->data);
pdu.cp.handle = instance; pdu->handle = instance;
pdu.cp.length = len; pdu->length = len;
pdu.cp.operation = LE_SET_ADV_DATA_OP_COMPLETE; pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
pdu.cp.frag_pref = LE_SET_ADV_DATA_NO_FRAG; pdu->frag_pref = LE_SET_ADV_DATA_NO_FRAG;
err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_SCAN_RSP_DATA, err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_SCAN_RSP_DATA,
sizeof(pdu.cp) + len, &pdu.cp, struct_size(pdu, data, len), pdu,
HCI_CMD_TIMEOUT); HCI_CMD_TIMEOUT);
if (err) if (err)
return err; return err;
...@@ -1267,7 +1263,7 @@ static int hci_set_ext_scan_rsp_data_sync(struct hci_dev *hdev, u8 instance) ...@@ -1267,7 +1263,7 @@ static int hci_set_ext_scan_rsp_data_sync(struct hci_dev *hdev, u8 instance)
if (adv) { if (adv) {
adv->scan_rsp_changed = false; adv->scan_rsp_changed = false;
} else { } else {
memcpy(hdev->scan_rsp_data, pdu.data, len); memcpy(hdev->scan_rsp_data, pdu->data, len);
hdev->scan_rsp_data_len = len; hdev->scan_rsp_data_len = len;
} }
...@@ -1411,14 +1407,10 @@ static int hci_set_per_adv_params_sync(struct hci_dev *hdev, u8 instance, ...@@ -1411,14 +1407,10 @@ static int hci_set_per_adv_params_sync(struct hci_dev *hdev, u8 instance,
static int hci_set_per_adv_data_sync(struct hci_dev *hdev, u8 instance) static int hci_set_per_adv_data_sync(struct hci_dev *hdev, u8 instance)
{ {
struct { DEFINE_FLEX(struct hci_cp_le_set_per_adv_data, pdu, data, length,
struct hci_cp_le_set_per_adv_data cp; HCI_MAX_PER_AD_LENGTH);
u8 data[HCI_MAX_PER_AD_LENGTH];
} pdu;
u8 len; u8 len;
memset(&pdu, 0, sizeof(pdu));
if (instance) { if (instance) {
struct adv_info *adv = hci_find_adv_instance(hdev, instance); struct adv_info *adv = hci_find_adv_instance(hdev, instance);
...@@ -1426,14 +1418,14 @@ static int hci_set_per_adv_data_sync(struct hci_dev *hdev, u8 instance) ...@@ -1426,14 +1418,14 @@ static int hci_set_per_adv_data_sync(struct hci_dev *hdev, u8 instance)
return 0; return 0;
} }
len = eir_create_per_adv_data(hdev, instance, pdu.data); len = eir_create_per_adv_data(hdev, instance, pdu->data);
pdu.cp.length = len; pdu->length = len;
pdu.cp.handle = instance; pdu->handle = instance;
pdu.cp.operation = LE_SET_ADV_DATA_OP_COMPLETE; pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_PER_ADV_DATA, return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_PER_ADV_DATA,
sizeof(pdu.cp) + len, &pdu, struct_size(pdu, data, len), pdu,
HCI_CMD_TIMEOUT); HCI_CMD_TIMEOUT);
} }
...@@ -1727,31 +1719,27 @@ int hci_le_terminate_big_sync(struct hci_dev *hdev, u8 handle, u8 reason) ...@@ -1727,31 +1719,27 @@ int hci_le_terminate_big_sync(struct hci_dev *hdev, u8 handle, u8 reason)
static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance) static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
{ {
struct { DEFINE_FLEX(struct hci_cp_le_set_ext_adv_data, pdu, data, length,
struct hci_cp_le_set_ext_adv_data cp; HCI_MAX_EXT_AD_LENGTH);
u8 data[HCI_MAX_EXT_AD_LENGTH];
} pdu;
u8 len; u8 len;
struct adv_info *adv = NULL; struct adv_info *adv = NULL;
int err; int err;
memset(&pdu, 0, sizeof(pdu));
if (instance) { if (instance) {
adv = hci_find_adv_instance(hdev, instance); adv = hci_find_adv_instance(hdev, instance);
if (!adv || !adv->adv_data_changed) if (!adv || !adv->adv_data_changed)
return 0; return 0;
} }
len = eir_create_adv_data(hdev, instance, pdu.data); len = eir_create_adv_data(hdev, instance, pdu->data);
pdu.cp.length = len; pdu->length = len;
pdu.cp.handle = instance; pdu->handle = instance;
pdu.cp.operation = LE_SET_ADV_DATA_OP_COMPLETE; pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
pdu.cp.frag_pref = LE_SET_ADV_DATA_NO_FRAG; pdu->frag_pref = LE_SET_ADV_DATA_NO_FRAG;
err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_DATA, err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_DATA,
sizeof(pdu.cp) + len, &pdu.cp, struct_size(pdu, data, len), pdu,
HCI_CMD_TIMEOUT); HCI_CMD_TIMEOUT);
if (err) if (err)
return err; return err;
...@@ -1760,7 +1748,7 @@ static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance) ...@@ -1760,7 +1748,7 @@ static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
if (adv) { if (adv) {
adv->adv_data_changed = false; adv->adv_data_changed = false;
} else { } else {
memcpy(hdev->adv_data, pdu.data, len); memcpy(hdev->adv_data, pdu->data, len);
hdev->adv_data_len = len; hdev->adv_data_len = len;
} }
...@@ -6493,10 +6481,8 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data) ...@@ -6493,10 +6481,8 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
int hci_le_create_cis_sync(struct hci_dev *hdev) int hci_le_create_cis_sync(struct hci_dev *hdev)
{ {
struct { DEFINE_FLEX(struct hci_cp_le_create_cis, cmd, cis, num_cis, 0x1f);
struct hci_cp_le_create_cis cp; size_t aux_num_cis = 0;
struct hci_cis cis[0x1f];
} cmd;
struct hci_conn *conn; struct hci_conn *conn;
u8 cig = BT_ISO_QOS_CIG_UNSET; u8 cig = BT_ISO_QOS_CIG_UNSET;
...@@ -6523,8 +6509,6 @@ int hci_le_create_cis_sync(struct hci_dev *hdev) ...@@ -6523,8 +6509,6 @@ int hci_le_create_cis_sync(struct hci_dev *hdev)
* remains pending. * remains pending.
*/ */
memset(&cmd, 0, sizeof(cmd));
hci_dev_lock(hdev); hci_dev_lock(hdev);
rcu_read_lock(); rcu_read_lock();
...@@ -6561,7 +6545,7 @@ int hci_le_create_cis_sync(struct hci_dev *hdev) ...@@ -6561,7 +6545,7 @@ int hci_le_create_cis_sync(struct hci_dev *hdev)
goto done; goto done;
list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) { list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
struct hci_cis *cis = &cmd.cis[cmd.cp.num_cis]; struct hci_cis *cis = &cmd->cis[aux_num_cis];
if (hci_conn_check_create_cis(conn) || if (hci_conn_check_create_cis(conn) ||
conn->iso_qos.ucast.cig != cig) conn->iso_qos.ucast.cig != cig)
...@@ -6570,25 +6554,25 @@ int hci_le_create_cis_sync(struct hci_dev *hdev) ...@@ -6570,25 +6554,25 @@ int hci_le_create_cis_sync(struct hci_dev *hdev)
set_bit(HCI_CONN_CREATE_CIS, &conn->flags); set_bit(HCI_CONN_CREATE_CIS, &conn->flags);
cis->acl_handle = cpu_to_le16(conn->parent->handle); cis->acl_handle = cpu_to_le16(conn->parent->handle);
cis->cis_handle = cpu_to_le16(conn->handle); cis->cis_handle = cpu_to_le16(conn->handle);
cmd.cp.num_cis++; aux_num_cis++;
if (cmd.cp.num_cis >= ARRAY_SIZE(cmd.cis)) if (aux_num_cis >= 0x1f)
break; break;
} }
cmd->num_cis = aux_num_cis;
done: done:
rcu_read_unlock(); rcu_read_unlock();
hci_dev_unlock(hdev); hci_dev_unlock(hdev);
if (!cmd.cp.num_cis) if (!aux_num_cis)
return 0; return 0;
/* Wait for HCI_LE_CIS_Established */ /* Wait for HCI_LE_CIS_Established */
return __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_CREATE_CIS, return __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_CREATE_CIS,
sizeof(cmd.cp) + sizeof(cmd.cis[0]) * struct_size(cmd, cis, cmd->num_cis),
cmd.cp.num_cis, &cmd, cmd, HCI_EVT_LE_CIS_ESTABLISHED,
HCI_EVT_LE_CIS_ESTABLISHED,
conn->conn_timeout, NULL); conn->conn_timeout, NULL);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment