On 07/14/16 20:28, Dan Williams wrote:
acpi_evaluate_object() allocates memory. Free the buffer allocated
during acpi_nfit_add().
Cc: <stable(a)vger.kernel.org>
Cc: Vishal Verma <vishal.l.verma(a)intel.com>
Reported-by: Xiao Guangrong <guangrong.xiao(a)intel.com>
Reported-by: Haozhong Zhang <haozhong.zhang(a)intel.com>
Signed-off-by: Dan Williams <dan.j.williams(a)intel.com>
---
drivers/acpi/nfit.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index 0497175ee6cb..008dbaaa2b75 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -2414,12 +2414,15 @@ static int acpi_nfit_add(struct acpi_device *adev)
acpi_desc->nfit =
(struct acpi_nfit_header *)obj->buffer.pointer;
sz = obj->buffer.length;
+ rc = acpi_nfit_init(acpi_desc, sz);
} else
dev_dbg(dev, "%s invalid type %d, ignoring _FIT\n",
__func__, (int) obj->type);
'rc' is not set in this path, so it maybe used uninitialized by 'if (rc)'
below.
Should we set it to a non-zero value in this path?
- }
+ kfree(buf.pointer);
+ acpi_desc->nfit = NULL;
I notice the following code in acpi_nfit_notify():
nfit_saved = acpi_desc->nfit;
obj = buf.pointer;
if (obj->type == ACPI_TYPE_BUFFER) {
acpi_desc->nfit =
(struct acpi_nfit_header *)obj->buffer.pointer;
ret = acpi_nfit_init(acpi_desc, obj->buffer.length);
if (ret) {
/* Merge failed, restore old nfit, and exit */
acpi_desc->nfit = nfit_saved;
dev_err(dev, "failed to merge updated NFIT\n");
}
...
If we set acpi_desc->nfit to NULL in acpi_nfit_add() and
acpi_nfit_init() in acpi_nfit_notify() fails, it will be impossible to
restore the old nfit, because nfit_saved is NULL.
Thanks,
Haozhong
+ } else
+ rc = acpi_nfit_init(acpi_desc, sz);
- rc = acpi_nfit_init(acpi_desc, sz);
if (rc) {
nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
return rc;