From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org
Subject: [chanwoo:devfreq-testing-passive-gov 14/15] drivers/devfreq/governor_passive.c:306 cpufreq_passive_register_notifier() warn: possible memory leak of 'cpu_data'
Date: Wed, 31 Mar 2021 22:02:33 +0300 [thread overview]
Message-ID: <20210331190232.GM2065@kadam> (raw)
[-- Attachment #1: Type: text/plain, Size: 6887 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git devfreq-testing-passive-gov
head: 801b60f356489d7a4525d95b615e00544ba34729
commit: 58c52e7878cc305830bc4c784ebdb6cba7a94c87 [14/15] PM / devfreq: Add cpu based scaling support to passive governor
config: x86_64-randconfig-m001-20210330 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/devfreq/governor_passive.c:306 cpufreq_passive_register_notifier() warn: possible memory leak of 'cpu_data'
vim +/cpu_data +306 drivers/devfreq/governor_passive.c
58c52e7878cc30 Saravana Kannan 2021-03-02 242 static int cpufreq_passive_register_notifier(struct devfreq *devfreq)
58c52e7878cc30 Saravana Kannan 2021-03-02 243 {
58c52e7878cc30 Saravana Kannan 2021-03-02 244 struct devfreq_passive_data *p_data
58c52e7878cc30 Saravana Kannan 2021-03-02 245 = (struct devfreq_passive_data *)devfreq->data;
58c52e7878cc30 Saravana Kannan 2021-03-02 246 struct device *dev = devfreq->dev.parent;
58c52e7878cc30 Saravana Kannan 2021-03-02 247 struct opp_table *opp_table = NULL;
58c52e7878cc30 Saravana Kannan 2021-03-02 248 struct devfreq_cpu_data *cpu_data;
58c52e7878cc30 Saravana Kannan 2021-03-02 249 struct cpufreq_policy *policy;
58c52e7878cc30 Saravana Kannan 2021-03-02 250 struct device *cpu_dev;
58c52e7878cc30 Saravana Kannan 2021-03-02 251 unsigned int cpu;
58c52e7878cc30 Saravana Kannan 2021-03-02 252 int ret;
58c52e7878cc30 Saravana Kannan 2021-03-02 253
58c52e7878cc30 Saravana Kannan 2021-03-02 254 get_online_cpus();
58c52e7878cc30 Saravana Kannan 2021-03-02 255
58c52e7878cc30 Saravana Kannan 2021-03-02 256 p_data->nb.notifier_call = cpufreq_passive_notifier_call;
58c52e7878cc30 Saravana Kannan 2021-03-02 257 ret = cpufreq_register_notifier(&p_data->nb, CPUFREQ_TRANSITION_NOTIFIER);
58c52e7878cc30 Saravana Kannan 2021-03-02 258 if (ret) {
58c52e7878cc30 Saravana Kannan 2021-03-02 259 dev_err(dev, "failed to register cpufreq notifier\n");
58c52e7878cc30 Saravana Kannan 2021-03-02 260 p_data->nb.notifier_call = NULL;
58c52e7878cc30 Saravana Kannan 2021-03-02 261 goto out;
58c52e7878cc30 Saravana Kannan 2021-03-02 262 }
58c52e7878cc30 Saravana Kannan 2021-03-02 263
58c52e7878cc30 Saravana Kannan 2021-03-02 264 for_each_online_cpu(cpu) {
58c52e7878cc30 Saravana Kannan 2021-03-02 265 if (p_data->cpu_data[cpu])
58c52e7878cc30 Saravana Kannan 2021-03-02 266 continue;
58c52e7878cc30 Saravana Kannan 2021-03-02 267
58c52e7878cc30 Saravana Kannan 2021-03-02 268 policy = cpufreq_cpu_get(cpu);
58c52e7878cc30 Saravana Kannan 2021-03-02 269 if (policy) {
Better to flip this condition:
if (!policy) {
ret = -EPROBE_DEFER;
goto out;
}
The problem with goto out is that it doesn't unregister the notifier.
58c52e7878cc30 Saravana Kannan 2021-03-02 270 cpu_data = kzalloc(sizeof(*cpu_data), GFP_KERNEL);
58c52e7878cc30 Saravana Kannan 2021-03-02 271 if (!cpu_data) {
^^^^^^^^
Allocated here.
58c52e7878cc30 Saravana Kannan 2021-03-02 272 ret = -ENOMEM;
58c52e7878cc30 Saravana Kannan 2021-03-02 273 goto out;
The other problem is that these error paths don't call cpufreq_cpu_put(policy);
58c52e7878cc30 Saravana Kannan 2021-03-02 274 }
58c52e7878cc30 Saravana Kannan 2021-03-02 275
58c52e7878cc30 Saravana Kannan 2021-03-02 276 cpu_dev = get_cpu_device(cpu);
58c52e7878cc30 Saravana Kannan 2021-03-02 277 if (!cpu_dev) {
58c52e7878cc30 Saravana Kannan 2021-03-02 278 dev_err(dev, "failed to get cpu device\n");
58c52e7878cc30 Saravana Kannan 2021-03-02 279 ret = -ENODEV;
58c52e7878cc30 Saravana Kannan 2021-03-02 280 goto out;
^^^^^^^^
Leaked on this path. Move the allocation so it happens after
opp_table = dev_pm_opp_get_opp_table(cpu_dev); succeeds.
58c52e7878cc30 Saravana Kannan 2021-03-02 281 }
58c52e7878cc30 Saravana Kannan 2021-03-02 282
58c52e7878cc30 Saravana Kannan 2021-03-02 283 opp_table = dev_pm_opp_get_opp_table(cpu_dev);
58c52e7878cc30 Saravana Kannan 2021-03-02 284 if (IS_ERR(opp_table)) {
58c52e7878cc30 Saravana Kannan 2021-03-02 285 ret = PTR_ERR(opp_table);
58c52e7878cc30 Saravana Kannan 2021-03-02 286 goto out;
58c52e7878cc30 Saravana Kannan 2021-03-02 287 }
58c52e7878cc30 Saravana Kannan 2021-03-02 288
58c52e7878cc30 Saravana Kannan 2021-03-02 289 cpu_data->dev = cpu_dev;
58c52e7878cc30 Saravana Kannan 2021-03-02 290 cpu_data->opp_table = opp_table;
58c52e7878cc30 Saravana Kannan 2021-03-02 291 cpu_data->first_cpu = cpumask_first(policy->related_cpus);
58c52e7878cc30 Saravana Kannan 2021-03-02 292 cpu_data->cur_freq = policy->cur;
58c52e7878cc30 Saravana Kannan 2021-03-02 293 cpu_data->min_freq = policy->cpuinfo.min_freq;
58c52e7878cc30 Saravana Kannan 2021-03-02 294 cpu_data->max_freq = policy->cpuinfo.max_freq;
58c52e7878cc30 Saravana Kannan 2021-03-02 295
58c52e7878cc30 Saravana Kannan 2021-03-02 296 p_data->cpu_data[cpu] = cpu_data;
58c52e7878cc30 Saravana Kannan 2021-03-02 297 cpufreq_cpu_put(policy);
58c52e7878cc30 Saravana Kannan 2021-03-02 298 } else {
58c52e7878cc30 Saravana Kannan 2021-03-02 299 ret = -EPROBE_DEFER;
58c52e7878cc30 Saravana Kannan 2021-03-02 300 goto out;
58c52e7878cc30 Saravana Kannan 2021-03-02 301 }
58c52e7878cc30 Saravana Kannan 2021-03-02 302 }
58c52e7878cc30 Saravana Kannan 2021-03-02 303 out:
58c52e7878cc30 Saravana Kannan 2021-03-02 304 put_online_cpus();
58c52e7878cc30 Saravana Kannan 2021-03-02 305 if (ret)
58c52e7878cc30 Saravana Kannan 2021-03-02 @306 return ret;
58c52e7878cc30 Saravana Kannan 2021-03-02 307
58c52e7878cc30 Saravana Kannan 2021-03-02 308 mutex_lock(&devfreq->lock);
58c52e7878cc30 Saravana Kannan 2021-03-02 309 ret = devfreq_update_target(devfreq, 0L);
58c52e7878cc30 Saravana Kannan 2021-03-02 310 mutex_unlock(&devfreq->lock);
58c52e7878cc30 Saravana Kannan 2021-03-02 311 if (ret)
58c52e7878cc30 Saravana Kannan 2021-03-02 312 dev_err(dev, "failed to update the frequency\n");
Do we need to unwind anything if devfreq_update_target() fails?
58c52e7878cc30 Saravana Kannan 2021-03-02 313
58c52e7878cc30 Saravana Kannan 2021-03-02 314 return ret;
58c52e7878cc30 Saravana Kannan 2021-03-02 315 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45343 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild-all@lists.01.org
Subject: [chanwoo:devfreq-testing-passive-gov 14/15] drivers/devfreq/governor_passive.c:306 cpufreq_passive_register_notifier() warn: possible memory leak of 'cpu_data'
Date: Wed, 31 Mar 2021 22:02:33 +0300 [thread overview]
Message-ID: <20210331190232.GM2065@kadam> (raw)
[-- Attachment #1: Type: text/plain, Size: 6887 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git devfreq-testing-passive-gov
head: 801b60f356489d7a4525d95b615e00544ba34729
commit: 58c52e7878cc305830bc4c784ebdb6cba7a94c87 [14/15] PM / devfreq: Add cpu based scaling support to passive governor
config: x86_64-randconfig-m001-20210330 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/devfreq/governor_passive.c:306 cpufreq_passive_register_notifier() warn: possible memory leak of 'cpu_data'
vim +/cpu_data +306 drivers/devfreq/governor_passive.c
58c52e7878cc30 Saravana Kannan 2021-03-02 242 static int cpufreq_passive_register_notifier(struct devfreq *devfreq)
58c52e7878cc30 Saravana Kannan 2021-03-02 243 {
58c52e7878cc30 Saravana Kannan 2021-03-02 244 struct devfreq_passive_data *p_data
58c52e7878cc30 Saravana Kannan 2021-03-02 245 = (struct devfreq_passive_data *)devfreq->data;
58c52e7878cc30 Saravana Kannan 2021-03-02 246 struct device *dev = devfreq->dev.parent;
58c52e7878cc30 Saravana Kannan 2021-03-02 247 struct opp_table *opp_table = NULL;
58c52e7878cc30 Saravana Kannan 2021-03-02 248 struct devfreq_cpu_data *cpu_data;
58c52e7878cc30 Saravana Kannan 2021-03-02 249 struct cpufreq_policy *policy;
58c52e7878cc30 Saravana Kannan 2021-03-02 250 struct device *cpu_dev;
58c52e7878cc30 Saravana Kannan 2021-03-02 251 unsigned int cpu;
58c52e7878cc30 Saravana Kannan 2021-03-02 252 int ret;
58c52e7878cc30 Saravana Kannan 2021-03-02 253
58c52e7878cc30 Saravana Kannan 2021-03-02 254 get_online_cpus();
58c52e7878cc30 Saravana Kannan 2021-03-02 255
58c52e7878cc30 Saravana Kannan 2021-03-02 256 p_data->nb.notifier_call = cpufreq_passive_notifier_call;
58c52e7878cc30 Saravana Kannan 2021-03-02 257 ret = cpufreq_register_notifier(&p_data->nb, CPUFREQ_TRANSITION_NOTIFIER);
58c52e7878cc30 Saravana Kannan 2021-03-02 258 if (ret) {
58c52e7878cc30 Saravana Kannan 2021-03-02 259 dev_err(dev, "failed to register cpufreq notifier\n");
58c52e7878cc30 Saravana Kannan 2021-03-02 260 p_data->nb.notifier_call = NULL;
58c52e7878cc30 Saravana Kannan 2021-03-02 261 goto out;
58c52e7878cc30 Saravana Kannan 2021-03-02 262 }
58c52e7878cc30 Saravana Kannan 2021-03-02 263
58c52e7878cc30 Saravana Kannan 2021-03-02 264 for_each_online_cpu(cpu) {
58c52e7878cc30 Saravana Kannan 2021-03-02 265 if (p_data->cpu_data[cpu])
58c52e7878cc30 Saravana Kannan 2021-03-02 266 continue;
58c52e7878cc30 Saravana Kannan 2021-03-02 267
58c52e7878cc30 Saravana Kannan 2021-03-02 268 policy = cpufreq_cpu_get(cpu);
58c52e7878cc30 Saravana Kannan 2021-03-02 269 if (policy) {
Better to flip this condition:
if (!policy) {
ret = -EPROBE_DEFER;
goto out;
}
The problem with goto out is that it doesn't unregister the notifier.
58c52e7878cc30 Saravana Kannan 2021-03-02 270 cpu_data = kzalloc(sizeof(*cpu_data), GFP_KERNEL);
58c52e7878cc30 Saravana Kannan 2021-03-02 271 if (!cpu_data) {
^^^^^^^^
Allocated here.
58c52e7878cc30 Saravana Kannan 2021-03-02 272 ret = -ENOMEM;
58c52e7878cc30 Saravana Kannan 2021-03-02 273 goto out;
The other problem is that these error paths don't call cpufreq_cpu_put(policy);
58c52e7878cc30 Saravana Kannan 2021-03-02 274 }
58c52e7878cc30 Saravana Kannan 2021-03-02 275
58c52e7878cc30 Saravana Kannan 2021-03-02 276 cpu_dev = get_cpu_device(cpu);
58c52e7878cc30 Saravana Kannan 2021-03-02 277 if (!cpu_dev) {
58c52e7878cc30 Saravana Kannan 2021-03-02 278 dev_err(dev, "failed to get cpu device\n");
58c52e7878cc30 Saravana Kannan 2021-03-02 279 ret = -ENODEV;
58c52e7878cc30 Saravana Kannan 2021-03-02 280 goto out;
^^^^^^^^
Leaked on this path. Move the allocation so it happens after
opp_table = dev_pm_opp_get_opp_table(cpu_dev); succeeds.
58c52e7878cc30 Saravana Kannan 2021-03-02 281 }
58c52e7878cc30 Saravana Kannan 2021-03-02 282
58c52e7878cc30 Saravana Kannan 2021-03-02 283 opp_table = dev_pm_opp_get_opp_table(cpu_dev);
58c52e7878cc30 Saravana Kannan 2021-03-02 284 if (IS_ERR(opp_table)) {
58c52e7878cc30 Saravana Kannan 2021-03-02 285 ret = PTR_ERR(opp_table);
58c52e7878cc30 Saravana Kannan 2021-03-02 286 goto out;
58c52e7878cc30 Saravana Kannan 2021-03-02 287 }
58c52e7878cc30 Saravana Kannan 2021-03-02 288
58c52e7878cc30 Saravana Kannan 2021-03-02 289 cpu_data->dev = cpu_dev;
58c52e7878cc30 Saravana Kannan 2021-03-02 290 cpu_data->opp_table = opp_table;
58c52e7878cc30 Saravana Kannan 2021-03-02 291 cpu_data->first_cpu = cpumask_first(policy->related_cpus);
58c52e7878cc30 Saravana Kannan 2021-03-02 292 cpu_data->cur_freq = policy->cur;
58c52e7878cc30 Saravana Kannan 2021-03-02 293 cpu_data->min_freq = policy->cpuinfo.min_freq;
58c52e7878cc30 Saravana Kannan 2021-03-02 294 cpu_data->max_freq = policy->cpuinfo.max_freq;
58c52e7878cc30 Saravana Kannan 2021-03-02 295
58c52e7878cc30 Saravana Kannan 2021-03-02 296 p_data->cpu_data[cpu] = cpu_data;
58c52e7878cc30 Saravana Kannan 2021-03-02 297 cpufreq_cpu_put(policy);
58c52e7878cc30 Saravana Kannan 2021-03-02 298 } else {
58c52e7878cc30 Saravana Kannan 2021-03-02 299 ret = -EPROBE_DEFER;
58c52e7878cc30 Saravana Kannan 2021-03-02 300 goto out;
58c52e7878cc30 Saravana Kannan 2021-03-02 301 }
58c52e7878cc30 Saravana Kannan 2021-03-02 302 }
58c52e7878cc30 Saravana Kannan 2021-03-02 303 out:
58c52e7878cc30 Saravana Kannan 2021-03-02 304 put_online_cpus();
58c52e7878cc30 Saravana Kannan 2021-03-02 305 if (ret)
58c52e7878cc30 Saravana Kannan 2021-03-02 @306 return ret;
58c52e7878cc30 Saravana Kannan 2021-03-02 307
58c52e7878cc30 Saravana Kannan 2021-03-02 308 mutex_lock(&devfreq->lock);
58c52e7878cc30 Saravana Kannan 2021-03-02 309 ret = devfreq_update_target(devfreq, 0L);
58c52e7878cc30 Saravana Kannan 2021-03-02 310 mutex_unlock(&devfreq->lock);
58c52e7878cc30 Saravana Kannan 2021-03-02 311 if (ret)
58c52e7878cc30 Saravana Kannan 2021-03-02 312 dev_err(dev, "failed to update the frequency\n");
Do we need to unwind anything if devfreq_update_target() fails?
58c52e7878cc30 Saravana Kannan 2021-03-02 313
58c52e7878cc30 Saravana Kannan 2021-03-02 314 return ret;
58c52e7878cc30 Saravana Kannan 2021-03-02 315 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45343 bytes --]
next reply other threads:[~2021-03-31 19:02 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-31 19:02 Dan Carpenter [this message]
2021-03-31 19:02 ` [chanwoo:devfreq-testing-passive-gov 14/15] drivers/devfreq/governor_passive.c:306 cpufreq_passive_register_notifier() warn: possible memory leak of 'cpu_data' Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2021-03-31 17:14 kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210331190232.GM2065@kadam \
--to=dan.carpenter@oracle.com \
--cc=kbuild@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.