On 07/14/16 20:28, Dan Williams wrote:
> acpi_evaluate_object() allocates memory. Free the buffer allocated
> during acpi_nfit_add().
>
> Cc: <[email protected]>
> Cc: Vishal Verma <[email protected]>
> Reported-by: Xiao Guangrong <[email protected]>
> Reported-by: Haozhong Zhang <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> 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;
>
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
