| Message ID | 20240229182911.750135-1-dave.jiang@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Fix the incorrect assignment of SSLBIS entry pointer initial location | expand |
Dave Jiang wrote: > The 'entry' pointer in cdat_sslbis_handler() is set to header + > sizeof(common header). However, the math missed the addition of the SSLBIS > main header. It should be header + sizeof(common header) + sizeof(*sslbis). Insert form letter => "please document the user visible effects of the bug in the changelog, please give some breadcrumbs to -stable and backporters about this fix." A "how found" note usually helps to clarify the impact of the bug. > Fixes: 80aa780dda20 ("cxl: Add callback to parse the SSLBIS subtable from CDAT") > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/core/cdat.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c > index 08fd0baea7a0..b7c678d4b1aa 100644 > --- a/drivers/cxl/core/cdat.c > +++ b/drivers/cxl/core/cdat.c > @@ -414,7 +414,9 @@ static int cdat_sslbis_handler(union acpi_subtable_headers *header, void *arg, > return 0; > > entries = remain / sizeof(*entry); > - entry = (struct acpi_cdat_sslbe *)((unsigned long)header + sizeof(*sslbis)); > + entry = (struct acpi_cdat_sslbe *)((unsigned long)header + > + sizeof(header->cdat) + > + sizeof(*sslbis)); This looks way too fragile, what about something like: struct acpi_cdat_ssblis_table { struct acpi_cdat_header header; struct acpi_cdat_ssblis ssblis_header; struct acpi_cdat_sslbe entries[]; } *tbl = (struct acpi_cdat_ssblis_table *) header; for (i = 0; i < entries; i++) { u16 x = le16_to_cpu((__force __le16)tlb->entries[i].portx_id); ... ...then you can also use struct_size() to validate the size of the table relative to the number of entries.
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c index 08fd0baea7a0..b7c678d4b1aa 100644 --- a/drivers/cxl/core/cdat.c +++ b/drivers/cxl/core/cdat.c @@ -414,7 +414,9 @@ static int cdat_sslbis_handler(union acpi_subtable_headers *header, void *arg, return 0; entries = remain / sizeof(*entry); - entry = (struct acpi_cdat_sslbe *)((unsigned long)header + sizeof(*sslbis)); + entry = (struct acpi_cdat_sslbe *)((unsigned long)header + + sizeof(header->cdat) + + sizeof(*sslbis)); for (i = 0; i < entries; i++) { u16 x = le16_to_cpu((__force __le16)entry->portx_id);
The 'entry' pointer in cdat_sslbis_handler() is set to header + sizeof(common header). However, the math missed the addition of the SSLBIS main header. It should be header + sizeof(common header) + sizeof(*sslbis). Fixes: 80aa780dda20 ("cxl: Add callback to parse the SSLBIS subtable from CDAT") Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/cxl/core/cdat.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)