Hi Jonas,
On Mon, Jan 29, 2024 at 6:49 AM Jonas Dreßler <verdre@xxxxxxx> wrote:
>
> In add_expect_hci_list() we iterate through the entries of the
> expect_hci_list as long as there is an opcode, which means currently
> this relies on overflowing the buffer to detect the end of the list.
>
> This is not great and when running with address sanitizer, the
> out-of-bounds read gets detected and mgmt-tester aborts. Fix it by
> adding a trailing 0-opcode to all those lists.
> ---
> tools/mgmt-tester.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
> index 7dfd1b0c7..ee12ed7d5 100644
> --- a/tools/mgmt-tester.c
> +++ b/tools/mgmt-tester.c
> @@ -8798,6 +8798,9 @@ static const struct hci_cmd_data multi_ext_adv_add_second_hci_cmds[] = {
> .len = sizeof(le_set_ext_adv_enable_inst_2),
> .param = le_set_ext_adv_enable_inst_2,
> },
> + {
> + .opcode = 0,
> + },
Normally the compiler would put a NULL term when last member has ',',
but we should either use {} to properly terminate the list or perhaps
it would have been better to have a something like
.expect_hci_list_len = ARRAY_SIZE(list) to ensure we never access past
the end of the list.
> };
>
> static const struct generic_data multi_ext_advertising_add_second_2 = {
> @@ -8845,6 +8848,9 @@ static const struct hci_cmd_data multi_ext_adv_remove_adv_hci_cmds[] = {
> .len = sizeof(advertising_instance1_param),
> .param = advertising_instance1_param,
> },
> + {
> + .opcode = 0,
> + },
> };
>
> static const struct generic_data multi_ext_advertising_remove = {
> @@ -8877,6 +8883,9 @@ static const struct hci_cmd_data multi_ext_adv_remove_all_adv_hci_cmds[] = {
> {
> .opcode = BT_HCI_CMD_LE_CLEAR_ADV_SETS,
> },
> + {
> + .opcode = 0,
> + },
> };
>
> static const struct generic_data multi_ext_advertising_remove_all = {
> @@ -8913,6 +8922,9 @@ static const struct hci_cmd_data multi_ext_adv_add_2_advs_hci_cmds[] = {
> .len = sizeof(set_ext_adv_data_test1),
> .param = set_ext_adv_data_test1,
> },
> + {
> + .opcode = 0,
> + },
> };
>
> static const struct generic_data multi_ext_advertising_add_no_power = {
> @@ -10378,6 +10390,9 @@ static const struct hci_cmd_data ll_privacy_add_device_3_hci_list[] = {
> .param = set_resolv_on_param,
> .len = sizeof(set_resolv_on_param),
> },
> + {
> + .opcode = 0,
> + },
> };
>
> static const struct generic_data ll_privacy_add_device_3 = {
> @@ -10495,6 +10510,9 @@ static const struct hci_cmd_data ll_privacy_add_device_9_hci_list[] = {
> .len = sizeof(le_add_to_resolv_list_param),
> .param = le_add_to_resolv_list_param
> },
> + {
> + .opcode = 0,
> + },
> };
>
> static const struct generic_data ll_privacy_add_device_9 = {
> @@ -10823,6 +10841,9 @@ static const struct hci_cmd_data ll_privacy_set_device_flags_1_hci_list[] = {
> .param = set_resolv_on_param,
> .len = sizeof(set_resolv_on_param),
> },
> + {
> + .opcode = 0,
> + },
> };
>
> static const uint8_t device_flags_changed_params_1[] = {
> --
> 2.43.0
>
--
Luiz Augusto von Dentz
