| Message ID | 20221017130910.2307118-7-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | thermal/core: Protect thermal device operations against removal | expand |
On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@roeck-us.net> wrote: > > In preparation to protecting access to thermal operations against thermal > zone device removal, protect hwmon accesses to thermal zone operations > with the thermal zone mutex. After acquiring the mutex, ensure that the > thermal zone device is registered before proceeding. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/thermal/thermal_hwmon.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c > index f53f4ceb6a5d..33bfbaed4236 100644 > --- a/drivers/thermal/thermal_hwmon.c > +++ b/drivers/thermal/thermal_hwmon.c > @@ -77,11 +77,19 @@ temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf) > int temperature; > int ret; > > + mutex_lock(&tz->lock); > + > + if (!device_is_registered(&tz->device)) { > + ret = -ENODEV; > + goto unlock; > + } > + > ret = tz->ops->get_crit_temp(tz, &temperature); Again, I would do it this way: if (device_is_registered(&tz->device)) ret = tz->ops->get_crit_temp(tz, &temperature); else ret = -ENODEV; And I wouldn't change the code below (the ternary operator is out of fashion in particular). > - if (ret) > - return ret; > > - return sprintf(buf, "%d\n", temperature); > +unlock: > + mutex_unlock(&tz->lock); > + > + return ret ? ret : sprintf(buf, "%d\n", temperature); > } > > > --
On Wed, Nov 09, 2022 at 08:19:13PM +0100, Rafael J. Wysocki wrote: > On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@roeck-us.net> wrote: [ ... ] > > And I wouldn't change the code below (the ternary operator is out of > fashion in particular). > I tried to introduce some consistency; the ternary operator is used in some of the existing thermal code. Guess I went the wrong direction. Never mind; I don't have a strong opinion either way. I updated the series patches to no longer use ternary operators in updated code, but I left existing code alone (changing that should not be part of this patch set anyway). Thanks, Guenter
On Thu, Nov 10, 2022 at 3:21 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On Wed, Nov 09, 2022 at 08:19:13PM +0100, Rafael J. Wysocki wrote: > > On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@roeck-us.net> wrote: > [ ... ] > > > > And I wouldn't change the code below (the ternary operator is out of > > fashion in particular). > > > > I tried to introduce some consistency; the ternary operator is used > in some of the existing thermal code. Guess I went the wrong direction. > Never mind; I don't have a strong opinion either way. > I updated the series patches to no longer use ternary operators in > updated code, but I left existing code alone (changing that should not > be part of this patch set anyway). Thanks, that's what I would have done too.
diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c index f53f4ceb6a5d..33bfbaed4236 100644 --- a/drivers/thermal/thermal_hwmon.c +++ b/drivers/thermal/thermal_hwmon.c @@ -77,11 +77,19 @@ temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf) int temperature; int ret; + mutex_lock(&tz->lock); + + if (!device_is_registered(&tz->device)) { + ret = -ENODEV; + goto unlock; + } + ret = tz->ops->get_crit_temp(tz, &temperature); - if (ret) - return ret; - return sprintf(buf, "%d\n", temperature); +unlock: + mutex_unlock(&tz->lock); + + return ret ? ret : sprintf(buf, "%d\n", temperature); }
In preparation to protecting access to thermal operations against thermal zone device removal, protect hwmon accesses to thermal zone operations with the thermal zone mutex. After acquiring the mutex, ensure that the thermal zone device is registered before proceeding. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/thermal/thermal_hwmon.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)