Re: acpi_nfit_query_poison() broken on non-ARS systems

On Fri, Apr 29, 2016 at 5:03 PM, Vishal Verma <vishal(a)kernel.org&gt; wrote: > > On Fri, 2016-04-29 at 19:36 -0400, Linda Knippers wrote: > > [snip] > > > > > > > > > > static int acpi_nfit_query_poison(struct acpi_nfit_desc > > > *acpi_desc, > > >                 struct nfit_spa *nfit_spa) > > > { > > >         struct acpi_nfit_system_address *spa = nfit_spa->spa; > > >         int rc; > > > > > >         if (!nfit_spa->max_ars) { > > On a platform that doesn't support ARS, max_ars will always be > > zero > > so you'll keep executing this code when it seems like you're > > trying > > to avoid that. > > > > > > > > > > >                 struct nd_cmd_ars_cap ars_cap; > > > > > >                 memset(&ars_cap, 0, sizeof(ars_cap)); > > >                 rc = ars_get_cap(acpi_desc, &ars_cap, > > > nfit_spa); > > >                 if (rc < 0) > > >                         return rc; > > The call succeeds so this return isn't taken, but then the code > > assumes > > everything is good.  It should check ars_cap.status so see if the > > function > > is supported or if there was an error and return something > > appropriate. > > In previous version of the code, that's what > > acpi_nfit_find_poison() > > did. > > Instead, this function continues, using data that's not right and > > making > > more calls that also aren't supported. > Good point - I think we should add a check here and make sure ARS > is > supported using the status field. I can work on a patch. Thanks for > the > report! ...but we do, or are supposed to, fail on any non-zero status:                 /* Command failed */                 if (ars_cap->status & 0xffff)                         return -EIO;

From ACPI 6.1: