Re: [PATCH v2 2/2] hmat acpi: Fix out of bounds access due to missing use of indirection

[Michael Tokarev]

07.03.2024 19:03, Jonathan Cameron via wrote:
> With a numa set up such as
>
> -numa nodeid=0,cpus=0 \
> -numa nodeid=1,memdev=mem \
> -numa nodeid=2,cpus=1
>
> and appropriate hmat_lb entries the initiator list is correctly
> computed and writen to HMAT as 0,2 but then the LB data is accessed
> using the node id (here 2), landing outside the entry_list array.
>
> Stash the reverse lookup when writing the initiator list and use
> it to get the correct array index index.
>
> Fixes: 4586a2cb83 ("hmat acpi: Build System Locality Latency and Bandwidth 
> Information Structure(s)")
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This seems like a -stable material, is it not?

Thanks,

/mjt

> ---
>   hw/acpi/hmat.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
> index 723ae28d32..b933ae3c06 100644
> --- a/hw/acpi/hmat.c
> +++ b/hw/acpi/hmat.c
> @@ -78,6 +78,7 @@ static void build_hmat_lb(GArray *table_data, HMAT_LB_Info 
> *hmat_lb,
>                             uint32_t *initiator_list)
>   {
>       int i, index;
> +    uint32_t initiator_to_index[MAX_NODES] = {};
>       HMAT_LB_Data *lb_data;
>       uint16_t *entry_list;
>       uint32_t base;
> @@ -121,6 +122,8 @@ static void build_hmat_lb(GArray *table_data, HMAT_LB_Info 
> *hmat_lb,
>       /* Initiator Proximity Domain List */
>       for (i = 0; i < num_initiator; i++) {
>           build_append_int_noprefix(table_data, initiator_list[i], 4);
> +        /* Reverse mapping for array possitions */
> +        initiator_to_index[initiator_list[i]] = i;
>       }
>   
>       /* Target Proximity Domain List */
> @@ -132,7 +135,8 @@ static void build_hmat_lb(GArray *table_data, HMAT_LB_Info 
> *hmat_lb,
>       entry_list = g_new0(uint16_t, num_initiator * num_target);
>       for (i = 0; i < hmat_lb->list->len; i++) {
>           lb_data = &g_array_index(hmat_lb->list, HMAT_LB_Data, i);
> -        index = lb_data->initiator * num_target + lb_data->target;
> +        index = initiator_to_index[lb_data->initiator] * num_target +
> +            lb_data->target;
>   
>           entry_list[index] = (uint16_t)(lb_data->data / hmat_lb->base);
>       }