PM / devfreq: fix mem leak and missing check of return value in devfreq_add_device()
PATCH 1/1 PM / devfreq: fix mem leak and missing check of return value in devfreq_add_device()
'devfreq' is malloced in devfreq_add_device() and should be freed in the error handling cases, otherwise it will cause memory leak.
devm_kzalloc() could fail, so insert a check of its return value. And if it fails, returns -ENOMEM.
--- drivers/devfreq/devfreq.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-)
drivers/devfreq/devfreq.c +22 -11
@@ -651,7 +651,7 @@ struct devfreq *devfreq_add_device(struct device *dev,mutex_unlock (&devfreq->lock); err =set_freq_table (devfreq);if (err <0 ) - goto err_out; + goto err_dev;mutex_lock (&devfreq->lock); }
@@ -683,16 +683,27 @@ struct devfreq *devfreq_add_device(struct device *dev, goto err_out; } - devfreq->trans_table = -devm_kzalloc (&devfreq->dev, -array3_size (sizeof (unsigned int ), - devfreq->profile->max_state, - devfreq->profile->max_state), -GFP_KERNEL ); + devfreq->trans_table =devm_kzalloc (&devfreq->dev, +array3_size (sizeof (unsigned int ), + devfreq->profile->max_state, + devfreq->profile->max_state), +GFP_KERNEL ); +if (!devfreq->trans_table) { +mutex_unlock (&devfreq->lock); + err = -ENOMEM ; + goto err_devfreq; + } + devfreq->time_in_state =devm_kcalloc (&devfreq->dev, - devfreq->profile->max_state, -sizeof (unsigned long ), -GFP_KERNEL ); + devfreq->profile->max_state, +sizeof (unsigned long ), +GFP_KERNEL ); +if (!devfreq->time_in_state) { +mutex_unlock (&devfreq->lock); + err = -ENOMEM ; + goto err_devfreq; + } + devfreq->last_stat_updated = jiffies;srcu_init_notifier_head (&devfreq->transition_notifier_list);
@@ -726,7 +737,7 @@ struct devfreq *devfreq_add_device(struct device *dev, err_init:mutex_unlock (&devfreq_list_lock); - +err_devfreq:devfreq_remove_device (devfreq); devfreq =NULL ; err_dev:
Signature · 1 line
2.17.0
Hi Yangtao,
2019년 1월 19일 (토) 오후 2:03, Yangtao Li <[email protected]>님이 작성:
Quoted 30 lines
'devfreq' is malloced in
devfreq_add_device()and should be freed in the error handling cases, otherwise it will cause memory leak.
devm_kzalloc()could fail, so insert a check of its return value. And if it fails, returns-ENOMEM.Signed-off-by Yangtao Li <[email protected]>--- drivers/devfreq/devfreq.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-)drivers/devfreq/devfreq.c +1
-2@@ -651,7 +651,7 @@ struct devfreq *devfreq_add_device(struct device *dev,mutex_unlock (&devfreq->lock); err =set_freq_table (devfreq);if (err <0 ) - goto err_out; + goto err_dev;mutex_lock (&devfreq->lock); }@@ -683,16 +683,27 @@ struct devfreq *devfreq_add_device(struct device *dev, goto err_out; } - devfreq->trans_table =
drivers/devfreq/devfreq.c struct devfreq *devfreq_add_device(struct device *dev, · line 690
Quoted 10 lines
drivers/devfreq/devfreq.c +5
-5-devm_kzalloc (&devfreq->dev, -array3_size (sizeof (unsigned int ), - devfreq->profile->max_state, - devfreq->profile->max_state), -GFP_KERNEL ); + devfreq->trans_table =devm_kzalloc (&devfreq->dev, +array3_size (sizeof (unsigned int ), + devfreq->profile->max_state, + devfreq->profile->max_state), +GFP_KERNEL );
The above 10 ten lines are not related to the memory leak for this patch. If you want to fix the indentation, you have to make it separately.
drivers/devfreq/devfreq.c struct devfreq *devfreq_add_device(struct device *dev, · line 700
Quoted 13 lines
drivers/devfreq/devfreq.c +9
-3+if (!devfreq->trans_table) { +mutex_unlock (&devfreq->lock); + err = -ENOMEM ; + goto err_devfreq; + } + devfreq->time_in_state =devm_kcalloc (&devfreq->dev, - devfreq->profile->max_state, -sizeof (unsigned long ), -GFP_KERNEL ); + devfreq->profile->max_state, +sizeof (unsigned long ), +GFP_KERNEL );
ditto. The above 6 ten lines are not related to the memory leak for this patch. If you want to fix the indentation, you have to make it separately.
Quoted 10 lines
drivers/devfreq/devfreq.c +6
-0+if (!devfreq->time_in_state) { +mutex_unlock (&devfreq->lock); + err = -ENOMEM ; + goto err_devfreq; + } + devfreq->last_stat_updated = jiffies;srcu_init_notifier_head (&devfreq->transition_notifier_list);
drivers/devfreq/devfreq.c struct devfreq *devfreq_add_device(struct device *dev, · line 743
Quoted 8 lines
drivers/devfreq/devfreq.c +1
-1@@ -726,7 +737,7 @@ struct devfreq *devfreq_add_device(struct device *dev, err_init:mutex_unlock (&devfreq_list_lock); - +err_devfreq:devfreq_remove_device (devfreq); devfreq =NULL ; err_dev:
Also, you better to add following codes to free the memory of 'devfreq->trans_table' and 'devfreq->time_in_state' because this patch handles the memory leak of 'trans_table' and 'time_in_state' variables.
drivers/devfreq/devfreq.c +2 -0
@@ -593,6 +593,9 @@ static void devfreq_dev_release(struct device *dev) devfreq->profile->exit (devfreq->dev.parent);mutex_destroy (&devfreq->lock); +kfree (devfreq->trans_table); +kfree (devfreq->time_in_state);kfree (devfreq); }
Signature · 3 lines
Best Regards, Chanwoo Choi Samsung Electronics
Quoted 4 lines
Also, you better to add following codes to free the memory of '
devfreq->trans_table' and 'devfreq->time_in_state' because this patch handles the memory leak of 'trans_table' and 'time_in_state' variables.
Hi Chanwoo:
Isn't that the mem requested by the devm_ API automatically freed when the device is detached?
So we don't need to pay attention to free these mem.
Thanks，
Yangtao
Quoted 18 lines
drivers/devfreq/devfreq.c +2
-1@@ -593,6 +593,9 @@ static void devfreq_dev_release(struct device *dev) devfreq->profile->exit (devfreq->dev.parent);mutex_destroy (&devfreq->lock); +kfree (devfreq->trans_table); +kfree (devfreq->time_in_state);kfree (devfreq); } --Best Regards,
Chanwoo Choi
Samsung Electronics
MyungJoo Ham
maintainerQuoted 7 lines
'devfreq' is malloced in
devfreq_add_device()and should be freed in the error handling cases, otherwise it will cause memory leak.
devm_kzalloc()could fail, so insert a check of its return value. And if it fails, returns-ENOMEM.Signed-off-by Yangtao Li <[email protected]>
Dear Yangtao,
Could you please make your patch without indentation style changes?
The label, "err_devfreq", would fit more if it's renamed "err_kzalloc".
Cheers,
MyungJoo.