| Message ID | 20221017130910.2307118-1-linux@roeck-us.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | thermal/core: Protect thermal device operations against removal | expand |
Hi, On Mon, Oct 17, 2022 at 06:09:01AM -0700, Guenter Roeck wrote: > Accesses to thermal zones, and with it to thermal zone device operations, > are still possible after the thermal zone device has been unregistered. > For example, thermal_zone_get_temp() can be called from temp_show() > in thermal_sysfs.c if the sysfs attribute was opened before the thermal > device was unregistered. This is problematic and may result in crashes > since the operations data structure and the underlying code may be gone > when the calls are made. > > The following series solves the problem by protecting accesses to thermal > device operations with the thermal device mutex, and by verifying that the > thermal device is still registered after the mutex has been acquired. > > This was previously sent as RFC/RFT as single patch [1]. The code was reworked > to match thermal subsystem changes made between v6.0 and v6.1, and it was > split into several patches to simplify review. > Any thoughts / comments / feedback on this series ? Thanks, Guenter > [1] https://lore.kernel.org/linux-pm/20221004033936.1047691-1-linux@roeck-us.net/ > > ---------------------------------------------------------------- > Guenter Roeck (9): > thermal/core: Destroy thermal zone device mutex in release function > thermal/core: Delete device under thermal device zone lock > thermal/core: Ensure that thermal device is registered in thermal_zone_get_temp > thermal/core: Move parameter validation from __thermal_zone_get_temp to thermal_zone_get_temp > thermal/core: Introduce locked version of thermal_zone_device_update > thermal/core: Protect hwmon accesses to thermal operations with thermal zone mutex > thermal/core: Protect sysfs accesses to thermal operations with thermal zone mutex > thermal/core: Remove thermal_zone_set_trips() > thermal/core: Protect thermal device operations against thermal device removal > > drivers/thermal/thermal_core.c | 76 ++++++++++++++++++++++++-------------- > drivers/thermal/thermal_core.h | 3 +- > drivers/thermal/thermal_helpers.c | 65 ++++++++++++++++++++++----------- > drivers/thermal/thermal_hwmon.c | 14 +++++-- > drivers/thermal/thermal_sysfs.c | 77 +++++++++++++++++++++++++++++++++------ > 5 files changed, 169 insertions(+), 66 deletions(-)
On 02/11/2022 19:50, Guenter Roeck wrote: > Hi, > > On Mon, Oct 17, 2022 at 06:09:01AM -0700, Guenter Roeck wrote: >> Accesses to thermal zones, and with it to thermal zone device operations, >> are still possible after the thermal zone device has been unregistered. >> For example, thermal_zone_get_temp() can be called from temp_show() >> in thermal_sysfs.c if the sysfs attribute was opened before the thermal >> device was unregistered. This is problematic and may result in crashes >> since the operations data structure and the underlying code may be gone >> when the calls are made. >> >> The following series solves the problem by protecting accesses to thermal >> device operations with the thermal device mutex, and by verifying that the >> thermal device is still registered after the mutex has been acquired. >> >> This was previously sent as RFC/RFT as single patch [1]. The code was reworked >> to match thermal subsystem changes made between v6.0 and v6.1, and it was >> split into several patches to simplify review. >> > > Any thoughts / comments / feedback on this series ? I'm out of the office ATM, I'll have a look on the series in a few days.
Hi Guenter, Sorry for the delay. On Wed, Nov 2, 2022 at 7:50 PM Guenter Roeck <linux@roeck-us.net> wrote: > > Hi, > > On Mon, Oct 17, 2022 at 06:09:01AM -0700, Guenter Roeck wrote: > > Accesses to thermal zones, and with it to thermal zone device operations, > > are still possible after the thermal zone device has been unregistered. > > For example, thermal_zone_get_temp() can be called from temp_show() > > in thermal_sysfs.c if the sysfs attribute was opened before the thermal > > device was unregistered. This is problematic and may result in crashes > > since the operations data structure and the underlying code may be gone > > when the calls are made. > > > > The following series solves the problem by protecting accesses to thermal > > device operations with the thermal device mutex, and by verifying that the > > thermal device is still registered after the mutex has been acquired. > > > > This was previously sent as RFC/RFT as single patch [1]. The code was reworked > > to match thermal subsystem changes made between v6.0 and v6.1, and it was > > split into several patches to simplify review. > > > > Any thoughts / comments / feedback on this series ? I have reviewed the series now and haven't found any major issues in it. I've posted some minor comments in separate replies to individual patches. If they are addressed, I can queue up the series for 6.2. Cheers! > > [1] https://lore.kernel.org/linux-pm/20221004033936.1047691-1-linux@roeck-us.net/ > > > > ---------------------------------------------------------------- > > Guenter Roeck (9): > > thermal/core: Destroy thermal zone device mutex in release function > > thermal/core: Delete device under thermal device zone lock > > thermal/core: Ensure that thermal device is registered in thermal_zone_get_temp > > thermal/core: Move parameter validation from __thermal_zone_get_temp to thermal_zone_get_temp > > thermal/core: Introduce locked version of thermal_zone_device_update > > thermal/core: Protect hwmon accesses to thermal operations with thermal zone mutex > > thermal/core: Protect sysfs accesses to thermal operations with thermal zone mutex > > thermal/core: Remove thermal_zone_set_trips() > > thermal/core: Protect thermal device operations against thermal device removal > > > > drivers/thermal/thermal_core.c | 76 ++++++++++++++++++++++++-------------- > > drivers/thermal/thermal_core.h | 3 +- > > drivers/thermal/thermal_helpers.c | 65 ++++++++++++++++++++++----------- > > drivers/thermal/thermal_hwmon.c | 14 +++++-- > > drivers/thermal/thermal_sysfs.c | 77 +++++++++++++++++++++++++++++++++------ > > 5 files changed, 169 insertions(+), 66 deletions(-)
Accesses to thermal zones, and with it to thermal zone device operations, are still possible after the thermal zone device has been unregistered. For example, thermal_zone_get_temp() can be called from temp_show() in thermal_sysfs.c if the sysfs attribute was opened before the thermal device was unregistered. This is problematic and may result in crashes since the operations data structure and the underlying code may be gone when the calls are made. The following series solves the problem by protecting accesses to thermal device operations with the thermal device mutex, and by verifying that the thermal device is still registered after the mutex has been acquired. This was previously sent as RFC/RFT as single patch [1]. The code was reworked to match thermal subsystem changes made between v6.0 and v6.1, and it was split into several patches to simplify review. [1] https://lore.kernel.org/linux-pm/20221004033936.1047691-1-linux@roeck-us.net/ ---------------------------------------------------------------- Guenter Roeck (9): thermal/core: Destroy thermal zone device mutex in release function thermal/core: Delete device under thermal device zone lock thermal/core: Ensure that thermal device is registered in thermal_zone_get_temp thermal/core: Move parameter validation from __thermal_zone_get_temp to thermal_zone_get_temp thermal/core: Introduce locked version of thermal_zone_device_update thermal/core: Protect hwmon accesses to thermal operations with thermal zone mutex thermal/core: Protect sysfs accesses to thermal operations with thermal zone mutex thermal/core: Remove thermal_zone_set_trips() thermal/core: Protect thermal device operations against thermal device removal drivers/thermal/thermal_core.c | 76 ++++++++++++++++++++++++-------------- drivers/thermal/thermal_core.h | 3 +- drivers/thermal/thermal_helpers.c | 65 ++++++++++++++++++++++----------- drivers/thermal/thermal_hwmon.c | 14 +++++-- drivers/thermal/thermal_sysfs.c | 77 +++++++++++++++++++++++++++++++++------ 5 files changed, 169 insertions(+), 66 deletions(-)