3:57 p.m.
On 07/16/2016 03:32 AM, Dan Williams wrote:
acpi_evaluate_object() allocates memory. Free the buffer allocated
during acpi_nfit_add(). Also, make it clear that ->nfit is not used
outside of acpi_nfit_init() context.
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>
---
Change since v1:
* Fix unitialized use of 'rc' (Haozhong)
* Clarify that their is no use-after-free problem in acpi_nfit_notify()
(Xiao)
No... This is a real problem, please seem below.
drivers/acpi/nfit.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index d89a02d9ed10..cbdbe13bdbe8 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -2390,7 +2390,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
struct acpi_table_header *tbl;
acpi_status status = AE_OK;
acpi_size sz;
- int rc;
+ int rc = 0;
status = acpi_get_table_with_size(ACPI_SIG_NFIT, 0, &tbl, &sz);
if (ACPI_FAILURE(status)) {
@@ -2427,12 +2427,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);
- }
+ acpi_desc->nfit = NULL;
+ kfree(buf.pointer);
+ } 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;
@@ -2454,7 +2457,6 @@ static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
{
struct acpi_nfit_desc *acpi_desc = dev_get_drvdata(&adev->dev);
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
- struct acpi_nfit_header *nfit_saved;
union acpi_object *obj;
struct device *dev = &adev->dev;
acpi_status status;
@@ -2492,21 +2494,16 @@ static void acpi_nfit_notify(struct acpi_device *adev, u32
event)
goto out_unlock;
}
- 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);
The issue is in acpi_nfit_init(), there are some info constructing nfit_spa
is directly from acpi_desc->nfit, for example:
acpi_nfit_init() -> add_table() -> add_spa():
static bool add_spa(struct acpi_nfit_desc *acpi_desc,
struct nfit_table_prev *prev,
struct acpi_nfit_system_address *spa)
{
...
list_for_each_entry(nfit_spa, &prev->spas, list) {
if (memcmp(nfit_spa->spa, spa, length) == 0) { // A
list_move_tail(&nfit_spa->list, &acpi_desc->spas);
return true;
}
}
...
return false;
INIT_LIST_HEAD(&nfit_spa->list);
nfit_spa->spa = spa; // B
list_add_tail(&nfit_spa->list, &acpi_desc->spas);
...
}
Note at point B, @spa is from acpi_desc->nfit. At point A, this @spa will be used
to check if it has already existed if hotplug event happens later.