RE: [PATCH v2 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse()

Dan Williams
2024-01-19T00:32:41Z

Robert Richter wrote:
> 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 when 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, the table buffer size can be different from the size
> specified in the length field. This may cause out-of-bound access then
> parsing the CDAT table.
>
> Fix that by providing an optonal 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.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Signed-off-by: Robert Richter <rrichter@amd.com>
[..]
> diff --git a/lib/fw_table.c b/lib/fw_table.c
> index 1e5e0b2f7012..ddb67853b7ac 100644
> --- a/lib/fw_table.c
> +++ b/lib/fw_table.c
[..]
> @@ -164,8 +166,10 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
> int i;
>
> type = acpi_get_subtable_type(id);
> - table_end = (unsigned long)table_header +
> - acpi_table_get_length(type, table_header);
> + table_len = acpi_table_get_length(type, table_header);
> + if (max_length && max_length < table_len)
> + table_len = max_length;

The other patches in this series look good, my only quibble here is
that this is an open-coded min_not_zero(). If nothing else pops up in
testing that triggers a v3 I will just fix that up locally.

Thanks, Robert!