Re: [PATCH -next] thermal/drivers/thermal_hwmon: Fix a kernel NULL pointer dereference

[Daniel Lezcano]

On 29/03/2023 16:38, Rafael J. Wysocki wrote:
> On Wed, Mar 29, 2023 at 4:16 PM Daniel Lezcano
> <daniel.lezcano@xxxxxxxxxx> wrote:
> > On 29/03/2023 14:06, Rafael J. Wysocki wrote:
> > > On Wed, Mar 29, 2023 at 11:57 AM Daniel Lezcano
> > > <daniel.lezcano@xxxxxxxxxx> 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@xxxxxxxxx>
> > > > > Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
> > > > > ---
> > > > > 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.


> > On the other side, adding more thermal_zone_debug/info.. gives
> > opportunities to external components of the core thermal framework to
> > write thermal zone device related message. I'm not sure that is a good
> > thing, each writer should stay in its namespace, no ?
>
> IMV whoever is allowed to use a thermal zone pointer should also be
> allowed to print messages related to its use, especially debug ones.
>
> "Encapsulation" means that the members of a thermal zone device object
> should not be accessed directly by its users other than the core, not
> that it cannot be used as a message tag.

Actually it is not about the encapsulation but the namespace of the 
messages. If a driver has an issue, IMO it is better it uses the device 
related messages and let thermal zone messages to be related to what is 
happening in the thermal framework, not in the back end.



--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog