On Wednesday, December 07, 2016 03:12:53 PM Tim Chen wrote: > On Wed, 2016-12-07 at 20:06 +0100, Sebastian Andrzej Siewior wrote: > > > > > > Signed-off-by: Sebastian Andrzej Siewior <[email protected]> > > --- > > drivers/acpi/cppc_acpi.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c > > index d0d0504b7c89..93252e5374c5 100644 > > --- a/drivers/acpi/cppc_acpi.c > > +++ b/drivers/acpi/cppc_acpi.c > > @@ -803,6 +803,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) > > if (addr) > > iounmap(addr); > > } > > + per_cpu(cpc_desc_ptr, pr->id) = NULL; > > kfree(cpc_ptr); > > > > out_buf_free: > > @@ -824,6 +825,8 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr) > > void __iomem *addr; > > > > cpc_ptr = per_cpu(cpc_desc_ptr, pr->id); > > + if (!cpc_ptr) > > + return; > > I agree that not handling null pointer here is a bug that should be fixed. > The cpc_ptr is checked at other places like acpi_get_psd_map. > We could potentially have a null cpc_ptr say when > the parsing of CPC table failed. We should handle such cases gracefully.
Agreed, but the bug fixed by the first hunk is real too. I'd fix it a bit differently, though: Tentatively-signed-off-by: Rafael J. Wysocki <[email protected]> --- drivers/acpi/cppc_acpi.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) Index: linux-pm/drivers/acpi/cppc_acpi.c =================================================================== --- linux-pm.orig/drivers/acpi/cppc_acpi.c +++ linux-pm/drivers/acpi/cppc_acpi.c @@ -776,9 +776,6 @@ int acpi_cppc_processor_probe(struct acp init_waitqueue_head(&pcc_data.pcc_write_wait_q); } - /* Plug PSD data into this CPUs CPC descriptor. */ - per_cpu(cpc_desc_ptr, pr->id) = cpc_ptr; - /* Everything looks okay */ pr_debug("Parsed CPC struct for CPU: %d\n", pr->id); @@ -789,10 +786,15 @@ int acpi_cppc_processor_probe(struct acp goto out_free; } + /* Plug PSD data into this CPUs CPC descriptor. */ + per_cpu(cpc_desc_ptr, pr->id) = cpc_ptr; + ret = kobject_init_and_add(&cpc_ptr->kobj, &cppc_ktype, &cpu_dev->kobj, "acpi_cppc"); - if (ret) + if (ret) { + per_cpu(cpc_desc_ptr, pr->id) = NULL; goto out_free; + } kfree(output.pointer); return 0; @@ -826,6 +828,8 @@ void acpi_cppc_processor_exit(struct acp void __iomem *addr; cpc_ptr = per_cpu(cpc_desc_ptr, pr->id); + if (!cpc_ptr) + return; /* Free all the mapped sys mem areas for this CPU */ for (i = 2; i < cpc_ptr->num_entries; i++) {
