Messages in this thread | | | Subject | Re: [PATCH] drivers: thermal: Add NULL pointer check before using cooling device stats | From | Daniel Lezcano <> | Date | Fri, 4 Dec 2020 20:21:01 +0100 |
| |
On 04/12/2020 20:09, Manaf Meethalavalappu Pallikunhi wrote: > There is a possible chance that some cooling device stats buffer > allocation fails due to very high cooling device max state value. > Later cooling device update or cooling stats sysfs will try to > access stats data for the same cooling device. It will lead to > NULL pointer dereference issue. > > Add a NULL pointer check before accessing thermal cooling device > stats data. It fixes the following bug
Why not create the 'stats' dir if the setup fails ?
> [ 26.812833] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004 > [ 27.122960] Call trace: > [ 27.122963] do_raw_spin_lock+0x18/0xe8 > [ 27.122966] _raw_spin_lock+0x24/0x30 > [ 27.128157] thermal_cooling_device_stats_update+0x24/0x98 > [ 27.128162] cur_state_store+0x88/0xb8 > [ 27.128166] dev_attr_store+0x40/0x58 > [ 27.128169] sysfs_kf_write+0x50/0x68 > [ 27.133358] kernfs_fop_write+0x12c/0x1c8 > [ 27.133362] __vfs_write+0x54/0x160 > [ 27.152297] vfs_write+0xcc/0x188 > [ 27.157132] ksys_write+0x78/0x108 > [ 27.162050] ksys_write+0xf8/0x108 > [ 27.166968] __arm_smccc_hvc+0x158/0x4b0 > [ 27.166973] __arm_smccc_hvc+0x9c/0x4b0 > [ 27.186005] el0_svc+0x8/0xc > > Signed-off-by: Manaf Meethalavalappu Pallikunhi <manafm@codeaurora.org> > --- > drivers/thermal/thermal_sysfs.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > index 473449b..a5e4855 100644 > --- a/drivers/thermal/thermal_sysfs.c > +++ b/drivers/thermal/thermal_sysfs.c > @@ -827,6 +827,9 @@ void thermal_cooling_device_stats_update(struct thermal_cooling_device *cdev, > { > struct cooling_dev_stats *stats = cdev->stats; > > + if (!stats) > + return; > + > spin_lock(&stats->lock); > > if (stats->state == new_state) > @@ -848,6 +851,9 @@ static ssize_t total_trans_show(struct device *dev, > struct cooling_dev_stats *stats = cdev->stats; > int ret; > > + if (!stats) > + return -ENODEV; > + > spin_lock(&stats->lock); > ret = sprintf(buf, "%u\n", stats->total_trans); > spin_unlock(&stats->lock); > @@ -864,6 +870,9 @@ time_in_state_ms_show(struct device *dev, struct device_attribute *attr, > ssize_t len = 0; > int i; > > + if (!stats) > + return -ENODEV; > + > spin_lock(&stats->lock); > update_time_in_state(stats); > > @@ -882,8 +891,12 @@ reset_store(struct device *dev, struct device_attribute *attr, const char *buf, > { > struct thermal_cooling_device *cdev = to_cooling_device(dev); > struct cooling_dev_stats *stats = cdev->stats; > - int i, states = stats->max_states; > + int i, states; > + > + if (!stats) > + return -ENODEV; > > + states = stats->max_states; > spin_lock(&stats->lock); > > stats->total_trans = 0; > @@ -907,6 +920,9 @@ static ssize_t trans_table_show(struct device *dev, > ssize_t len = 0; > int i, j; > > + if (!stats) > + return -ENODEV; > + > len += snprintf(buf + len, PAGE_SIZE - len, " From : To\n"); > len += snprintf(buf + len, PAGE_SIZE - len, " : "); > for (i = 0; i < stats->max_states; i++) { >
-- <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
| |