Re: [Xen-devel] [PATCH] ACPI: fix APEI related table size checking

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.10.12 at 08:49, Huang Ying <ying.huang@xxxxxxxxx> wrote:
> On Tue, 2012-10-16 at 15:46 +0100, Jan Beulich wrote:
>> On Huang Ying's machine:
>> 
>> erst_tab->header_length == sizeof(struct acpi_table_einj)
>   ~~~~                                                ~~~~
> 
> Typo?

Your typo: I copied the Linux commit message verbatim, to make
it possible to match the two commits. The adjustments done in
the actual patch eliminate the copy-n-paste mistake corrected by
Linux commit 7ed28f2ed43ece424ff2fa4dedac7928bb37a23a.

Jan

>> but Yinghai reported that on his machine,
>> 
>> erst_tab->header_length == sizeof(struct acpi_table_einj) -
>> sizeof(struct acpi_table_header)
>> 
>> To make erst table size checking code works on all systems, both
>> testing are treated as PASS.
>> 
>> Same situation applies to einj_tab->header_length, so corresponding
>> table size checking is changed in similar way too.
>> 
>> Originally-by: Yinghai Lu <yinghai@xxxxxxxxxx>
>> Signed-off-by: Huang Ying <ying.huang@xxxxxxxxx>
>> 
>> - use switch() for better readability
>> - add comment explaining why a formally invalid size it also being
>>   accepted
>> - check erst_tab->header.length before even looking at
>>   erst_tab->header_length
>> - prefer sizeof(*erst_tab) over sizeof(struct acpi_table_erst)
>> 
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> 
>> --- a/xen/drivers/acpi/apei/erst.c
>> +++ b/xen/drivers/acpi/apei/erst.c
>> @@ -715,12 +715,23 @@ int erst_clear(u64 record_id)
>>  
>>  static int __init erst_check_table(struct acpi_table_erst *erst_tab)
>>  {
>> -    if (erst_tab->header_length != sizeof(struct acpi_table_erst))
>> +    if (erst_tab->header.length < sizeof(*erst_tab))
>>              return -EINVAL;
>> -    if (erst_tab->header.length < sizeof(struct acpi_table_erst))
>> +
>> +    switch (erst_tab->header_length) {
>> +    case sizeof(*erst_tab) - sizeof(erst_tab->header):
>> +    /*
>> +     * While invalid per specification, there are (early?) systems
>> +     * indicating the full header size here, so accept that value too.
>> +     */
>> +    case sizeof(*erst_tab):
>> +            break;
>> +    default:
>>              return -EINVAL;
>> +    }
>> +
>>      if (erst_tab->entries !=
>> -        (erst_tab->header.length - sizeof(struct acpi_table_erst)) /
>> +        (erst_tab->header.length - sizeof(*erst_tab)) /
>>          sizeof(struct acpi_erst_entry))
>>              return -EINVAL;
>>  
>> 
>> 
>> 




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel