ACPI: Fix potential NULL pointer dereference in acpi_processor_add()

PATCH In review Hanjun Guo 4 participants 9 messages May 23, 2013 – May 31, 2013 ACPI

Share & tools

Hanjun Guo

author

2013-05-23 12:44 UTC #0 lore raw

PATCH 1/1 ACPI: Fix potential NULL pointer dereference in acpi_processor_add()

In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.

Signed-off-by Hanjun Guo <[email protected]>

---
 drivers/acpi/processor_driver.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

drivers/acpi/processor_driver.c +0 -0

index bec717f..dd64f23 100644
@@ -579,6 +579,10 @@ static int __cpuinit acpi_processor_add(struct acpi_device

*device)

        per_cpu(processors, pr->id) = pr;

        dev = get_cpu_device(pr->id);

+ if (!dev) {
+ result = -ENODEV;
+ goto err_clear_processor;
+ }

        if (sysfs_create_link(&device->dev.kobj, &dev->kobj, "sysdev")) {
                result = -EFAULT;
                goto err_clear_processor;

Signature · 1 line

1.7.1

Rafael J. Wysocki

2013-05-28 23:21 UTC #1 lore raw

Quoted 4 lines · Hanjun Guo

In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.
Signed-off-by Hanjun Guo <[email protected]>

This patch isn't necessary any more after the changes queued up for 3.11 in the acpi-hotplug branch of the linux-pm.git tree.

Thanks,
Rafael

Quoted 21 lines

---
 drivers/acpi/processor_driver.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

drivers/acpi/processor_driver.c +4 -0

index bec717f..dd64f23 100644
@@ -579,6 +579,10 @@ static int __cpuinit acpi_processor_add(struct acpi_device
 *device)
        per_cpu(processors, pr->id) = pr;
 
        dev = get_cpu_device(pr->id);
+       if (!dev) {
+               result = -ENODEV;
+               goto err_clear_processor;
+       }
        if (sysfs_create_link(&device->dev.kobj, &dev->kobj, "sysdev")) {
                result = -EFAULT;

                goto err_clear_processor;

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Hanjun Guo

author

2013-05-29 01:40 UTC #2 lore raw

Quoted 8 lines · Rafael J. Wysocki

Quoted 4 lines · Hanjun Guo
In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.
Signed-off-by Hanjun Guo <[email protected]>
This patch isn't necessary any more after the changes queued up for 3.11 in the acpi-hotplug branch of the linux-pm.git tree.

Ok, I noticed your patch set, just drop my patch.

Thanks
Hanjun

Quoted 26 lines

Thanks,
Rafael

Quoted 21 lines

---
 drivers/acpi/processor_driver.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

drivers/acpi/processor_driver.c +4 -0

index bec717f..dd64f23 100644
@@ -579,6 +579,10 @@ static int __cpuinit acpi_processor_add(struct acpi_device
 *device)
        per_cpu(processors, pr->id) = pr;
 
        dev = get_cpu_device(pr->id);
+       if (!dev) {
+               result = -ENODEV;
+               goto err_clear_processor;
+       }
        if (sysfs_create_link(&device->dev.kobj, &dev->kobj, "sysdev")) {
                result = -EFAULT;

                goto err_clear_processor;

Martin Mokrejs

2013-05-29 11:07 UTC #3 lore raw

Quoted 11 lines · Hanjun Guo

Quoted 8 lines · Rafael J. Wysocki
Quoted 4 lines · Hanjun Guo
In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.
Signed-off-by Hanjun Guo <[email protected]>
This patch isn't necessary any more after the changes queued up for 3.11 in the acpi-hotplug branch of the linux-pm.git tree.
Ok, I noticed your patch set, just drop my patch.

But shouldn't this go to stable at least? I checked linux-3.9.4 and it applies fine. Whether this is relevant for other stable series I will leave up to somebody else. ;)
Martin

Quoted 37 lines

Thanks
Hanjun
Quoted 17 lines
Thanks,
Rafael
Quoted 12 lines
---
 drivers/acpi/processor_driver.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/acpi/processor_driver.c +0 -0
index bec717f..dd64f23 100644
@@ -579,6 +579,10 @@ static int __cpuinit acpi_processor_add(struct acpi_device
 *device)
        per_cpu(processors, pr->id) = pr;
 
Quoted 9 lines
Quoted 9 lines
drivers/acpi/processor_driver.c +4 -0
        dev = get_cpu_device(pr->id);
+       if (!dev) {
+               result = -ENODEV;
+               goto err_clear_processor;
+       }
        if (sysfs_create_link(&device->dev.kobj, &dev->kobj, "sysdev")) {
                result = -EFAULT;
                goto err_clear_processor;
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hanjun Guo

author

2013-05-30 02:30 UTC #4 lore raw

Quoted 8 lines · Martin Mokrejs

Quoted 7 lines · Hanjun Guo
Quoted 6 lines · Rafael J. Wysocki
Quoted 4 lines · Hanjun Guo
In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.
Signed-off-by Hanjun Guo <[email protected]>

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
This patch isn't necessary any more after the changes queued up for 3.11 in the acpi-hotplug branch of the linux-pm.git tree.
Ok, I noticed your patch set, just drop my patch.
But shouldn't this go to stable at least? I checked linux-3.9.4 and it applies fine. Whether this is relevant for other stable series I will leave up to somebody else. ;)

Hi Rafeal,

What's your opinion on Martin's suggestion?

Thanks
Hanjun

Quoted 1 line

Martin

Rafael J. Wysocki

2013-05-30 13:49 UTC #5 lore raw

Quoted 14 lines · Hanjun Guo

Quoted 13 lines · Martin Mokrejs
Quoted 11 lines · Hanjun Guo
Quoted 8 lines · Rafael J. Wysocki
Quoted 4 lines · Hanjun Guo
In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.
Signed-off-by Hanjun Guo <[email protected]>
This patch isn't necessary any more after the changes queued up for 3.11 in the acpi-hotplug branch of the linux-pm.git tree.
Ok, I noticed your patch set, just drop my patch.

Quoted 7 lines

Quoted 3 lines
But shouldn't this go to stable at least? I checked linux-3.9.4 and it applies fine. Whether this is relevant for other stable series I will leave up to somebody else. ;)
Hi Rafeal,
What's your opinion on Martin's suggestion?

Well, this is kind of hard to say. We generally don't apply patches to -stable that don't have mainline counterparts.

Greg, I wonder what your opinion is?

Rafael

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Greg Kroah-Hartman

2013-05-30 22:53 UTC #6 lore raw

Quoted 19 lines · Rafael J. Wysocki

Quoted 18 lines · Hanjun Guo
Quoted 16 lines · Martin Mokrejs
Quoted 11 lines · Hanjun Guo
Quoted 8 lines · Rafael J. Wysocki
Quoted 4 lines · Hanjun Guo
In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.
Signed-off-by Hanjun Guo <[email protected]>
This patch isn't necessary any more after the changes queued up for 3.11 in the acpi-hotplug branch of the linux-pm.git tree.
Ok, I noticed your patch set, just drop my patch.
But shouldn't this go to stable at least? I checked linux-3.9.4 and it applies fine. Whether this is relevant for other stable series I will leave up to somebody else. ;)

Quoted 8 lines

Quoted 3 lines
Hi Rafeal,
What's your opinion on Martin's suggestion?
Well, this is kind of hard to say. We generally don't apply patches to -stable that don't have mainline counterparts.
Greg, I wonder what your opinion is?

We do not apply patches to -stable that are not in Linus's tree, unless there is no problem in Linus's tree due to a major rewrite of the code, and it has been confirmed that the same problem isn't there.

thanks,

greg k-h

Hanjun Guo

author

2013-05-31 03:59 UTC #7 lore raw

Quoted 24 lines · Greg Kroah-Hartman

Quoted 23 lines · Rafael J. Wysocki
Quoted 21 lines · Hanjun Guo
Quoted 8 lines · Martin Mokrejs
Quoted 7 lines · Hanjun Guo
Quoted 6 lines · Rafael J. Wysocki
Quoted 4 lines · Hanjun Guo
In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.
Signed-off-by Hanjun Guo <[email protected]>
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
This patch isn't necessary any more after the changes queued up for 3.11 in the acpi-hotplug branch of the linux-pm.git tree.
Ok, I noticed your patch set, just drop my patch.
But shouldn't this go to stable at least? I checked linux-3.9.4 and it applies fine. Whether this is relevant for other stable series I will leave up to somebody else. ;)
Hi Rafeal,
What's your opinion on Martin's suggestion?

Quoted 8 lines

Quoted 4 lines
Well, this is kind of hard to say. We generally don't apply patches to -stable that don't have mainline counterparts.
Greg, I wonder what your opinion is?
We do not apply patches to -stable that are not in Linus's tree, unless there is no problem in Linus's tree due to a major rewrite of the code, and it has been confirmed that the same problem isn't there.

Hi Rafael,

I found that the problem is still there in your acpi-hotplug tree, sorry for I didn't noticed early. I made a patch for this again, I hope it will make sense to you.

The patch is attached, based on your acpi-hotplug tree.

Thanks
Hanjun

Quoted 1 line

From 975e30f5906a883e533e4226d8eca1c449c7366b Mon Sep 17 00:00:00 2001

From: Hanjun Guo <[email protected]>
Date: Fri, 31 May 2013 11:36:08 +0800
Subject: [PATCH 1/1] ACPI / processor: Fix potential NULL pointer dereference
in acpi_processor_add()

In acpi_processor_add(), get_cpu_device() will return NULL in some cases, and ACPI_HANDLE_SET() just use the return dev directly, this will case a potential NULL pointer dereference, fix it.

Signed-off-by Hanjun Guo <[email protected]>

---
 drivers/acpi/acpi_processor.c |    5 +++++
 1 file changed, 5 insertions(+)

drivers/acpi/acpi_processor.c +5 -0

index 587d2af..4466b36 100644
@@ -389,6 +389,11 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device,
 	per_cpu(processor_device_array, pr->id) = device;
 
 	dev = get_cpu_device(pr->id);
+	if (!dev) {
+		result = -ENODEV;
+		goto err;
+	}
+
 	ACPI_HANDLE_SET(dev, pr->handle);
 	result = acpi_bind_one(dev, NULL);
 	if (result)

Signature · 1 line

1.7.9.5

Rafael J. Wysocki

2013-05-31 11:59 UTC #8 lore raw

Quoted 34 lines · Hanjun Guo

Quoted 24 lines · Greg Kroah-Hartman
Quoted 23 lines · Rafael J. Wysocki
Quoted 21 lines · Hanjun Guo
Quoted 8 lines · Martin Mokrejs
Quoted 7 lines · Hanjun Guo
Quoted 6 lines · Rafael J. Wysocki
Quoted 4 lines · Hanjun Guo
In acpi_processor_add(), get_cpu_device() will return NULL sometimes, although the chances are small, I think it should be fixed.
Signed-off-by Hanjun Guo <[email protected]>
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
This patch isn't necessary any more after the changes queued up for 3.11 in the acpi-hotplug branch of the linux-pm.git tree.
Ok, I noticed your patch set, just drop my patch.
But shouldn't this go to stable at least? I checked linux-3.9.4 and it applies fine. Whether this is relevant for other stable series I will leave up to somebody else. ;)
Hi Rafeal,
What's your opinion on Martin's suggestion?
Quoted 8 lines
Quoted 4 lines
Well, this is kind of hard to say. We generally don't apply patches to -stable that don't have mainline counterparts.
Greg, I wonder what your opinion is?
We do not apply patches to -stable that are not in Linus's tree, unless there is no problem in Linus's tree due to a major rewrite of the code, and it has been confirmed that the same problem isn't there.

Quoted 7 lines

Hi Rafael,
I found that the problem is still there in your acpi-hotplug tree, sorry for I didn't noticed early. I made a patch for this again, I hope it will make sense to you.
The patch is attached, based on your acpi-hotplug tree.

OK, thanks!

Rafael

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.