Messages in this thread Patch in this message | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH -next] thermal/drivers/thermal_hwmon: Fix a kernel NULL pointer dereference | Date | Wed, 29 Mar 2023 19:43:33 +0200 |
| |
On Wednesday, March 29, 2023 6:18:31 PM CEST Daniel Lezcano wrote: > On 29/03/2023 18:03, Rafael J. Wysocki wrote: > > On Wed, Mar 29, 2023 at 5:59 PM Daniel Lezcano > > <daniel.lezcano@linaro.org> wrote: > >> > >> On 29/03/2023 16:38, Rafael J. Wysocki wrote: > >>> On Wed, Mar 29, 2023 at 4:16 PM Daniel Lezcano > >>> <daniel.lezcano@linaro.org> wrote: > >>>> > >>>> On 29/03/2023 14:06, Rafael J. Wysocki wrote: > >>>>> On Wed, Mar 29, 2023 at 11:57 AM Daniel Lezcano > >>>>> <daniel.lezcano@linaro.org> wrote: > >>>>>> > >>>>>> On 29/03/2023 11:00, Zhang Rui wrote: > >>>>>>> When the hwmon device node of a thermal zone device is not found, > >>>>>>> using hwmon->device causes a kernel NULL pointer dereference. > >>>>>>> > >>>>>>> Reported-by: Preble Adam C <adam.c.preble@intel.com> > >>>>>>> Signed-off-by: Zhang Rui <rui.zhang@intel.com> > >>>>>>> --- > >>>>>>> Fixes: dec07d399cc8 ("thermal: Don't use 'device' internal thermal zone structure field") > >>>>>>> dec07d399cc8 is a commit in the linux-next branch of linux-pm repo. > >>>>>>> I'm not sure if the Fix tag applies to such commit or not. > >>>>>> > >>>>>> Actually it reverts the work done to encapsulate the thermal zone device > >>>>>> structure. > >>>>> > >>>>> So maybe instead of the wholesale switch to using "driver-specific" > >>>>> device pointers for printing messages, something like > >>>>> thermal_zone_debug/info/warn/error() taking a thermal zone pointer as > >>>>> the first argument can be defined? > >>>>> > >>>>> At least this particular bug could be avoided this way. > >>>> > >>>> Actually we previously said the thermal_hwmon can be considered as part > >>>> of the thermal core code, so we can keep using tz->device. > >>>> > >>>> I'll drop this change from the series. > >>> > >>> But it's there in my thermal branch already. > >>> > >>> Do you want to revert the thermal_hwmon.c part of commit dec07d399cc8? > >> > >> Oh, right. Fair enough. > >> > >> I think Rui's patch is fine then. > > > > I guess you mean the $subject one, that is: > > > > https://patchwork.kernel.org/project/linux-pm/patch/20230329090055.7537-1-rui.zhang@intel.com > > Correct > > > What about the message printed when temp is NULL. Should the original > > form of it be restored too? > > Yes, you are right, for the sake of consistency we should restore also > this one.
So I'm going to apply the appended patch.
Please let me know if there are any concerns regarding it.
--- From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Subject: [PATCH] thermal: thermal_hwmon: Revert recent message adjustment
For the sake of consistency, revert the second part of the thermal_hwmon.c hunk from commit dec07d399cc8 ("thermal: Don't use 'device' internal thermal zone structure field") after the first part of it has been reverted.
Link: https://lore.kernel.org/linux-pm/5b084360-898b-aad0-0b8e-33acc585d71d@linaro.org Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/thermal/thermal_hwmon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-pm/drivers/thermal/thermal_hwmon.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_hwmon.c +++ linux-pm/drivers/thermal/thermal_hwmon.c @@ -236,7 +236,7 @@ void thermal_remove_hwmon_sysfs(struct t temp = thermal_hwmon_lookup_temp(hwmon, tz); if (unlikely(!temp)) { /* Should never happen... */ - dev_dbg(hwmon->device, "temperature input lookup failed!\n"); + dev_dbg(&tz->device, "temperature input lookup failed!\n"); return; }
| |