| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <200812051444.18611.npalix@diku.dk>
Date: Fri, 5 Dec 2008 14:44:18 +0100
From: Nicolas Palix <npalix@...u.dk>
To: Stephen Rothwell <sfr@...b.auug.org.au>
Cc: Jean Delvare <khali@...ux-fr.org>,
Paul Mackerras <paulus@...ba.org>,
Stelian Pop <stelian@...ies.net>, lm-sensors@...sensors.org,
linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
Julia Lawall <julia@...u.dk>
Subject: Re: [PATCH linux-next] hwmon/ams: Add missing of_node_put in ams-core.c
On Wednesday 03 December 2008 00:30:51 Stephen Rothwell wrote:
> On Tue, 2 Dec 2008 14:48:13 +0100 Nicolas Palix <npalix@...u.dk> wrote:
> This is wrong because if CONFIG_SENSORS_AMS_PMU is set, np is used again
> below.
>
np is indeed reused in the second #ifdef, but there it is immediately
assigned a new value. So of_node_put is needed in the first #ifdef to
decrement the reference count on the value it has there.
> Also, I don't like this code since each of ams_i2c_init and ams_pmu_init
> keep a reference to np, so they should be doing an of_node_get(np) and
> this code above should be unconditionally doing an of_node_put(np). This
> is not your problem, though.
>
Following your remarks, I changed the patch. ams_i2c_init and ams_pmu_init
still keep a reference but they're now doing a call to of_node_get in their
init functions. The reference is then released in the corresponding exit
function or in error handling code.
Finally, ams_init releases the np reference for each of the two
of_find_node_by_name calls.
Regards,
Signed-off-by: Nicolas Palix <npalix@...u.dk>
Signed-off-by: Julia Lawall <julia@...u.dk>
---
ams-core.c | 17 +++++++++++++----
ams-i2c.c | 7 ++++++-
ams-pmu.c | 13 ++++++++++---
3 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/ams/ams-core.c b/drivers/hwmon/ams/ams-core.c
index 6c9ace1..9856c3f 100644
--- a/drivers/hwmon/ams/ams-core.c
+++ b/drivers/hwmon/ams/ams-core.c
@@ -192,6 +192,7 @@ release_freefall:
int __init ams_init(void)
{
struct device_node *np;
+ int ret;
spin_lock_init(&ams_info.irq_lock);
mutex_init(&ams_info.lock);
@@ -199,16 +200,24 @@ int __init ams_init(void)
#ifdef CONFIG_SENSORS_AMS_I2C
np = of_find_node_by_name(NULL, "accelerometer");
- if (np && of_device_is_compatible(np, "AAPL,accelerometer_1"))
+ if (np && of_device_is_compatible(np, "AAPL,accelerometer_1")) {
/* Found I2C motion sensor */
- return ams_i2c_init(np);
+ ret = ams_i2c_init(np);
+ of_node_put(np);
+ return ret;
+ }
+ of_node_put(np);
#endif
#ifdef CONFIG_SENSORS_AMS_PMU
np = of_find_node_by_name(NULL, "sms");
- if (np && of_device_is_compatible(np, "sms"))
+ if (np && of_device_is_compatible(np, "sms")) {
/* Found PMU motion sensor */
- return ams_pmu_init(np);
+ ret = ams_pmu_init(np);
+ of_node_put(np);
+ return ret;
+ }
+ of_node_put(np);
#endif
return -ENODEV;
}
diff --git a/drivers/hwmon/ams/ams-i2c.c b/drivers/hwmon/ams/ams-i2c.c
index 2cbf8a6..636db87 100644
--- a/drivers/hwmon/ams/ams-i2c.c
+++ b/drivers/hwmon/ams/ams-i2c.c
@@ -255,6 +255,9 @@ static int ams_i2c_remove(struct i2c_client *client)
static void ams_i2c_exit(void)
{
i2c_del_driver(&ams_i2c_driver);
+
+ /* Release device node */
+ of_node_put(ams_info.of_node);
}
int __init ams_i2c_init(struct device_node *np)
@@ -262,7 +265,7 @@ int __init ams_i2c_init(struct device_node *np)
int result;
/* Set implementation stuff */
- ams_info.of_node = np;
+ ams_info.of_node = of_node_get(np);
ams_info.exit = ams_i2c_exit;
ams_info.get_vendor = ams_i2c_get_vendor;
ams_info.get_xyz = ams_i2c_get_xyz;
@@ -270,6 +273,8 @@ int __init ams_i2c_init(struct device_node *np)
ams_info.bustype = BUS_I2C;
result = i2c_add_driver(&ams_i2c_driver);
+ if (result < 0)
+ of_node_put(ams_info.of_node);
return result;
}
diff --git a/drivers/hwmon/ams/ams-pmu.c b/drivers/hwmon/ams/ams-pmu.c
index fb18b3d..babf3d5 100644
--- a/drivers/hwmon/ams/ams-pmu.c
+++ b/drivers/hwmon/ams/ams-pmu.c
@@ -141,6 +141,9 @@ static void ams_pmu_exit(void)
ams_info.has_device = 0;
+ /* Release device node */
+ of_node_put(ams_info.of_node);
+
printk(KERN_INFO "ams: Unloading\n");
}
@@ -150,7 +153,7 @@ int __init ams_pmu_init(struct device_node *np)
int result;
/* Set implementation stuff */
- ams_info.of_node = np;
+ ams_info.of_node = of_node_get(np);
ams_info.exit = ams_pmu_exit;
ams_info.get_vendor = ams_pmu_get_vendor;
ams_info.get_xyz = ams_pmu_get_xyz;
@@ -159,8 +162,10 @@ int __init ams_pmu_init(struct device_node *np)
/* Get PMU command, should be 0x4e, but we can never know */
prop = of_get_property(ams_info.of_node, "reg", NULL);
- if (!prop)
+ if (!prop) {
+ of_node_put(ams_info.of_node);
return -ENODEV;
+ }
ams_pmu_cmd = ((*prop) >> 8) & 0xff;
@@ -171,8 +176,10 @@ int __init ams_pmu_init(struct device_node *np)
ams_pmu_clear_irq(AMS_IRQ_ALL);
result = ams_sensor_attach();
- if (result < 0)
+ if (result < 0) {
+ of_node_put(ams_info.of_node);
return result;
+ }
/* Set default values */
ams_pmu_set_register(AMS_FF_LOW_LIMIT, 0x15);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists