On Tue, Jul 05, 2022 at 03:00:02PM +0000, Varad Gautam wrote:
> Check that a user-provided thermal state is within the maximum
> thermal states supported by a given driver before attempting to
> apply it. This prevents a subsequent OOB access in
> thermal_cooling_device_stats_update() while performing
> state-transition accounting on drivers that do not have this check
> in their set_cur_state() handle.
>
> Signed-off-by: Varad Gautam <varadgautam@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/thermal/thermal_sysfs.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index 1c4aac8464a7..0c6b0223b133 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -607,7 +607,7 @@ cur_state_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct thermal_cooling_device *cdev = to_cooling_device(dev);
> - unsigned long state;
> + unsigned long state, max_state;
> int result;
>
> if (sscanf(buf, "%ld\n", &state) != 1)
> @@ -618,10 +618,20 @@ cur_state_store(struct device *dev, struct device_attribute *attr,
>
> mutex_lock(&cdev->lock);
>
> + result = cdev->ops->get_max_state(cdev, &max_state);
> + if (result)
> + goto unlock;
> +
> + if (state > max_state) {
> + result = -EINVAL;
> + goto unlock;
> + }
> +
> result = cdev->ops->set_cur_state(cdev, state);
Why doesn't set_cur_state() check the max state before setting it? Why
are the callers forced to always check it before? That feels wrong...
thanks,
greg k-h
