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

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Wed, 13 Mar 2024 21:24:06 +0300
Michael Tokarev <mjt@tls.msk.ru> wrote:

> 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?

Yes. Use case is obscure, but indeed seems suitable for stable.
Thanks.

Jonathan

> 
> 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);
> >       }  
>