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

[Huang Ying <ying.huang@xxxxxxxxx>]

On Wed, 2012-10-17 at 07:56 +0100, Jan Beulich wrote:
> >>> 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.

Sorry, my bad.

Best Regards,
Huang Ying

> 
> >> 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