On 17/11/2020 12:27, Zhang Rui wrote: > On Tue, 2020-11-17 at 09:57 +0100, Daniel Lezcano wrote: >> On 17/11/2020 08:18, Zhang Rui wrote: >>> On Mon, 2020-11-16 at 21:59 +0530, Mukesh Ojha wrote: >>>> Cooling stats variable inside >>>> thermal_cooling_device_stats_update() >>>> can get NULL. We should add a NULL check on stat inside for >>>> sanity. >>>> >>>> Signed-off-by: Mukesh Ojha <[email protected]> >>>> --- >>>> drivers/thermal/thermal_sysfs.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/drivers/thermal/thermal_sysfs.c >>>> b/drivers/thermal/thermal_sysfs.c >>>> index a6f371f..f52708f 100644 >>>> --- a/drivers/thermal/thermal_sysfs.c >>>> +++ b/drivers/thermal/thermal_sysfs.c >>>> @@ -754,6 +754,9 @@ void >>>> thermal_cooling_device_stats_update(struct >>>> thermal_cooling_device *cdev, >>>> { >>>> struct cooling_dev_stats *stats = cdev->stats; >>>> >>>> + if (!stats) >>>> + return; >>>> + >>> >>> May I know in which case stats can be NULL? >>> The only possibility I can see is that cdev->ops->get_max_state() >>> fails >>> in cooling_device_stats_setup(), right? >> >> A few lines below, the allocation could fail. >> >> stats = kzalloc(var, GFP_KERNEL); >> if (!stats) >> return; >> >> Some drivers define themselves as a cooling device state to let the >> userspace to act on their power. The screen brightness is one example >> with a cdev with 1024 states, the resulting stats table to be >> allocated >> is very big and the kzalloc is prone to fail. >> > Oh, right. > As we're not going to fix the cdev, so I think we do need this patch, > right?
If the allocation fails at this level if initialization there is clearly something wrong. I'm wondering if it would make sense to report back an error and make thermal_cooling_device_register to fail. Having an allocation failing and silently ignore it sounds like not very robust IMO. -- <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
