thermal: Validate new state in cur_state_store()

In cur_state_store(), the new state of the cooling device is received
from user-space and is not validated by the thermal core but the same is
left for the individual drivers to take care of. Apart from duplicating
the code it leaves possibility for introducing bugs where a driver may
not do it right.

Lets make the thermal core check the new state itself and store the max
value in the cooling device structure.

Link: https://lore.kernel.org/all/Y0ltRJRjO7AkawvE@kili/
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c
index a4ee4661e9cc44b84b0c29619cf9c908d3a19d71..1cfeac16e7ac1834721c0363d5b6da271eff719e 100644 (file)
--- a/drivers/thermal/gov_fair_share.c
+++ b/drivers/thermal/gov_fair_share.c
@@ -49,11 +49,7 @@ static int get_trip_level(struct thermal_zone_device *tz)
 static long get_target_state(struct thermal_zone_device *tz,
                struct thermal_cooling_device *cdev, int percentage, int level)
 {
-       unsigned long max_state;
-
-       cdev->ops->get_max_state(cdev, &max_state);
-
-       return (long)(percentage * level * max_state) / (100 * tz->num_trips);
+       return (long)(percentage * level * cdev->max_state) / (100 * tz->num_trips);
 }
 
 /**

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 117eeaf7dd241e82ca1401ae39501a4c46896bb5..08de59369e94d69cdd03aaa15834dff7d182b664 100644 (file)
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -603,8 +603,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
        struct thermal_instance *pos;
        struct thermal_zone_device *pos1;
        struct thermal_cooling_device *pos2;
-       unsigned long max_state;
-       int result, ret;
+       int result;
 
        if (trip >= tz->num_trips || trip < 0)
                return -EINVAL;
@@ -621,15 +620,11 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
        if (tz != pos1 || cdev != pos2)
                return -EINVAL;
 
-       ret = cdev->ops->get_max_state(cdev, &max_state);
-       if (ret)
-               return ret;
-
        /* lower default 0, upper default max_state */
        lower = lower == THERMAL_NO_LIMIT ? 0 : lower;
-       upper = upper == THERMAL_NO_LIMIT ? max_state : upper;
+       upper = upper == THERMAL_NO_LIMIT ? cdev->max_state : upper;
 
-       if (lower > upper || upper > max_state)
+       if (lower > upper || upper > cdev->max_state)
                return -EINVAL;
 
        dev = kzalloc(sizeof(*dev), GFP_KERNEL);
@@ -900,6 +895,10 @@ __thermal_cooling_device_register(struct device_node *np,
        cdev->updated = false;
        cdev->device.class = &thermal_class;
        cdev->devdata = devdata;
+
+       if (cdev->ops->get_max_state(cdev, &cdev->max_state))
+               goto out_kfree_type;
+
        thermal_cooling_device_setup_sysfs(cdev);
        ret = device_register(&cdev->device);
        if (ret)

diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index ec495c7dff035384d8312a6fa74ea16b40d15b56..bd75961254615f6a6ea9fec1e10563951cca2df2 100644 (file)
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -589,13 +589,8 @@ static ssize_t max_state_show(struct device *dev, struct device_attribute *attr,
                              char *buf)
 {
        struct thermal_cooling_device *cdev = to_cooling_device(dev);
-       unsigned long state;
-       int ret;
 
-       ret = cdev->ops->get_max_state(cdev, &state);
-       if (ret)
-               return ret;
-       return sprintf(buf, "%ld\n", state);
+       return sprintf(buf, "%ld\n", cdev->max_state);
 }
 
 static ssize_t cur_state_show(struct device *dev, struct device_attribute *attr,
@@ -625,6 +620,10 @@ cur_state_store(struct device *dev, struct device_attribute *attr,
        if ((long)state < 0)
                return -EINVAL;
 
+       /* Requested state should be less than max_state + 1 */
+       if (state > cdev->max_state)
+               return -EINVAL;
+
        mutex_lock(&cdev->lock);
 
        result = cdev->ops->set_cur_state(cdev, state);

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 9ecc128944a19ba222cbaf043ec0fa2d9f812788..5e093602e8fcd51a67664fe42eea31024e646025 100644 (file)
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -100,6 +100,7 @@ struct thermal_cooling_device_ops {
 struct thermal_cooling_device {
        int id;
        char *type;
+       unsigned long max_state;
        struct device device;
        struct device_node *np;
        void *devdata;