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

[Dan Williams]

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@xxxxxxxxxx>
> Cc: Len Brown <lenb@xxxxxxxxxx>
> Signed-off-by: Robert Richter <rrichter@xxxxxxx>
[..]
> 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!