Skip to content

Commit c6c3187

Browse files
Robert Richterdjbw
authored andcommitted
lib/firmware_table: Provide buffer length argument to cdat_table_parse()
There exist card implementations with a CDAT table using a fixed size buffer, but with entries filled in that do not fill the whole table length size. Then, the last entry in the CDAT table may not mark the end of the CDAT table buffer specified by the length field in the CDAT header. It can be shorter with trailing unused (zero'ed) data. The actual table length is determined while reading all CDAT entries of the table with DOE. If the table is greater than expected (containing zero'ed trailing data), the CDAT parser fails with: [ 48.691717] Malformed DSMAS table length: (24:0) [ 48.702084] [CDAT:0x00] Invalid zero length [ 48.711460] cxl_port endpoint1: Failed to parse CDAT: -22 In addition, a check of the table buffer length is missing to prevent an out-of-bound access then parsing the CDAT table. Hardening code against device returning borked table. Fix that by providing an optional buffer length argument to acpi_parse_entries_array() that can be used by cdat_table_parse() to propagate the buffer size down to its users to check the buffer length. This also prevents a possible out-of-bound access mentioned. Add a check to warn about a malformed CDAT table length. Cc: Rafael J. Wysocki <rafael@kernel.org> Cc: Len Brown <lenb@kernel.org> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Signed-off-by: Robert Richter <rrichter@amd.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Link: https://lore.kernel.org/r/ZdEnopFO0Tl3t2O1@rric.localdomain Signed-off-by: Dan Williams <dan.j.williams@intel.com>
1 parent e0c818e commit c6c3187

File tree

5 files changed

+24
-11
lines changed

5 files changed

+24
-11
lines changed

drivers/acpi/tables.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ int __init_or_acpilib acpi_table_parse_entries_array(
253253

254254
count = acpi_parse_entries_array(id, table_size,
255255
(union fw_table_header *)table_header,
256-
proc, proc_num, max_entries);
256+
0, proc, proc_num, max_entries);
257257

258258
acpi_put_table(table_header);
259259
return count;

drivers/cxl/core/cdat.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,13 @@ static int cxl_cdat_endpoint_process(struct cxl_port *port,
149149
int rc;
150150

151151
rc = cdat_table_parse(ACPI_CDAT_TYPE_DSMAS, cdat_dsmas_handler,
152-
dsmas_xa, port->cdat.table);
152+
dsmas_xa, port->cdat.table, port->cdat.length);
153153
rc = cdat_table_parse_output(rc);
154154
if (rc)
155155
return rc;
156156

157157
rc = cdat_table_parse(ACPI_CDAT_TYPE_DSLBIS, cdat_dslbis_handler,
158-
dsmas_xa, port->cdat.table);
158+
dsmas_xa, port->cdat.table, port->cdat.length);
159159
return cdat_table_parse_output(rc);
160160
}
161161

@@ -477,7 +477,7 @@ void cxl_switch_parse_cdat(struct cxl_port *port)
477477
return;
478478

479479
rc = cdat_table_parse(ACPI_CDAT_TYPE_SSLBIS, cdat_sslbis_handler,
480-
port, port->cdat.table);
480+
port, port->cdat.table, port->cdat.length);
481481
rc = cdat_table_parse_output(rc);
482482
if (rc)
483483
dev_dbg(&port->dev, "Failed to parse SSLBIS: %d\n", rc);

drivers/cxl/core/pci.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ void read_cdat_data(struct cxl_port *port)
623623
struct pci_dev *pdev = NULL;
624624
struct cxl_memdev *cxlmd;
625625
struct cdat_doe_rsp *buf;
626-
size_t length;
626+
size_t table_length, length;
627627
int rc;
628628

629629
if (is_cxl_memdev(uport)) {
@@ -662,10 +662,16 @@ void read_cdat_data(struct cxl_port *port)
662662
if (!buf)
663663
goto err;
664664

665+
table_length = length;
666+
665667
rc = cxl_cdat_read_table(dev, doe_mb, buf, &length);
666668
if (rc)
667669
goto err;
668670

671+
if (table_length != length)
672+
dev_warn(dev, "Malformed CDAT table length (%zu:%zu), discarding trailing data\n",
673+
table_length, length);
674+
669675
if (cdat_checksum(buf->data, length))
670676
goto err;
671677

include/linux/fw_table.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,14 @@ union acpi_subtable_headers {
4040

4141
int acpi_parse_entries_array(char *id, unsigned long table_size,
4242
union fw_table_header *table_header,
43+
unsigned long max_length,
4344
struct acpi_subtable_proc *proc,
4445
int proc_num, unsigned int max_entries);
4546

4647
int cdat_table_parse(enum acpi_cdat_type type,
4748
acpi_tbl_entry_handler_arg handler_arg, void *arg,
48-
struct acpi_table_cdat *table_header);
49+
struct acpi_table_cdat *table_header,
50+
unsigned long length);
4951

5052
/* CXL is the only non-ACPI consumer of the FIRMWARE_TABLE library */
5153
#if IS_ENABLED(CONFIG_ACPI) && !IS_ENABLED(CONFIG_CXL_BUS)

lib/fw_table.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ static __init_or_fwtbl_lib int call_handler(struct acpi_subtable_proc *proc,
127127
*
128128
* @id: table id (for debugging purposes)
129129
* @table_size: size of the root table
130+
* @max_length: maximum size of the table (ignore if 0)
130131
* @table_header: where does the table start?
131132
* @proc: array of acpi_subtable_proc struct containing entry id
132133
* and associated handler with it
@@ -148,18 +149,21 @@ static __init_or_fwtbl_lib int call_handler(struct acpi_subtable_proc *proc,
148149
int __init_or_fwtbl_lib
149150
acpi_parse_entries_array(char *id, unsigned long table_size,
150151
union fw_table_header *table_header,
152+
unsigned long max_length,
151153
struct acpi_subtable_proc *proc,
152154
int proc_num, unsigned int max_entries)
153155
{
154-
unsigned long table_end, subtable_len, entry_len;
156+
unsigned long table_len, table_end, subtable_len, entry_len;
155157
struct acpi_subtable_entry entry;
156158
enum acpi_subtable_type type;
157159
int count = 0;
158160
int i;
159161

160162
type = acpi_get_subtable_type(id);
161-
table_end = (unsigned long)table_header +
162-
acpi_table_get_length(type, table_header);
163+
table_len = acpi_table_get_length(type, table_header);
164+
if (max_length && max_length < table_len)
165+
table_len = max_length;
166+
table_end = (unsigned long)table_header + table_len;
163167

164168
/* Parse all entries looking for a match. */
165169

@@ -208,7 +212,8 @@ int __init_or_fwtbl_lib
208212
cdat_table_parse(enum acpi_cdat_type type,
209213
acpi_tbl_entry_handler_arg handler_arg,
210214
void *arg,
211-
struct acpi_table_cdat *table_header)
215+
struct acpi_table_cdat *table_header,
216+
unsigned long length)
212217
{
213218
struct acpi_subtable_proc proc = {
214219
.id = type,
@@ -222,6 +227,6 @@ cdat_table_parse(enum acpi_cdat_type type,
222227
return acpi_parse_entries_array(ACPI_SIG_CDAT,
223228
sizeof(struct acpi_table_cdat),
224229
(union fw_table_header *)table_header,
225-
&proc, 1, 0);
230+
length, &proc, 1, 0);
226231
}
227232
EXPORT_SYMBOL_FWTBL_LIB(cdat_table_parse);

0 commit comments

Comments
 (0)