| Message ID | 1444254577-23744-2-git-send-email-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Vishal Verma <vishal.l.verma@intel.com> writes: > If acpi_nfit_init is called (such as from nfit_test), with an nfit table > that has more memory allocated than it needs (and a similarly large > 'size' field, add_tables would happily keep adding null SPA Range tables > filling up all available memory. > > Make it friendlier by breaking out if a 0-length header is found in any > of the tables. Shouldn't that at least spew a warning? Or does the spec allow for zero-length tables? -Jeff > > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: <linux-acpi@vger.kernel.org> > Cc: <linux-nvdimm@lists.01.org> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > drivers/acpi/nfit.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c > index c1b8d03..ed599d1 100644 > --- a/drivers/acpi/nfit.c > +++ b/drivers/acpi/nfit.c > @@ -335,6 +335,9 @@ static void *add_table(struct acpi_nfit_desc *acpi_desc, void *table, > return NULL; > > hdr = table; > + if (!hdr->length) > + return NULL; > + > switch (hdr->type) { > case ACPI_NFIT_TYPE_SYSTEM_ADDRESS: > if (!add_spa(acpi_desc, table))
On Fri, Oct 9, 2015 at 10:23 AM, Jeff Moyer <jmoyer@redhat.com> wrote: > Vishal Verma <vishal.l.verma@intel.com> writes: > >> If acpi_nfit_init is called (such as from nfit_test), with an nfit table >> that has more memory allocated than it needs (and a similarly large >> 'size' field, add_tables would happily keep adding null SPA Range tables >> filling up all available memory. >> >> Make it friendlier by breaking out if a 0-length header is found in any >> of the tables. > > Shouldn't that at least spew a warning? Or does the spec allow for > zero-length tables? > The spec allows for zero length tables but the firmware implementation should be self consistent and not report a total NFIT size that is greater than the sum of the size of the sub-structures. A warning / nudge to firmware developers to fix their stuff seems appropriate.
On Fri, 2015-10-09 at 10:27 -0700, Dan Williams wrote: > On Fri, Oct 9, 2015 at 10:23 AM, Jeff Moyer <jmoyer@redhat.com> > wrote: > > Vishal Verma <vishal.l.verma@intel.com> writes: > > > > > If acpi_nfit_init is called (such as from nfit_test), with an > > > nfit table > > > that has more memory allocated than it needs (and a similarly > > > large > > > 'size' field, add_tables would happily keep adding null SPA Range > > > tables > > > filling up all available memory. > > > > > > Make it friendlier by breaking out if a 0-length header is found > > > in any > > > of the tables. > > > > Shouldn't that at least spew a warning? Or does the spec allow for > > zero-length tables? > > > > The spec allows for zero length tables but the firmware > implementation > should be self consistent and not report a total NFIT size that is > greater than the sum of the size of the sub-structures. A warning / > nudge to firmware developers to fix their stuff seems appropriate. Agreed, I'll add a warning. The caveat is, the following (3/3) patch to nfit test will trigger the warning for the first, non-hotplug pass, as I just calculate the size and allocate the buffer once, and the first pass will have the regular nfit, and the second pass will have new hotplug entries.. Is that OK or should I look at reworking that?
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c index c1b8d03..ed599d1 100644 --- a/drivers/acpi/nfit.c +++ b/drivers/acpi/nfit.c @@ -335,6 +335,9 @@ static void *add_table(struct acpi_nfit_desc *acpi_desc, void *table, return NULL; hdr = table; + if (!hdr->length) + return NULL; + switch (hdr->type) { case ACPI_NFIT_TYPE_SYSTEM_ADDRESS: if (!add_spa(acpi_desc, table))
If acpi_nfit_init is called (such as from nfit_test), with an nfit table that has more memory allocated than it needs (and a similarly large 'size' field, add_tables would happily keep adding null SPA Range tables filling up all available memory. Make it friendlier by breaking out if a 0-length header is found in any of the tables. Cc: Dan Williams <dan.j.williams@intel.com> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Cc: <linux-acpi@vger.kernel.org> Cc: <linux-nvdimm@lists.01.org> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- drivers/acpi/nfit.c | 3 +++ 1 file changed, 3 insertions(+)