diff mbox

[v2] cpufreq: Fix kmemleak in cppc_cpufreq_init failure path

Message ID 1520228438-12840-1-git-send-email-chuhu@redhat.com (mailing list archive)
State Mainlined
Delegated to: Rafael Wysocki
Headers show

Commit Message

Chunyu Hu March 5, 2018, 5:40 a.m. UTC
Kmemleak reported the below leak. When cppc_cpufreq_init went into
failure path, the cpu mask is not freed. After fix, this report is
gone. And to avaoid potential NULL pointer reference, check the cpu
value first.

unreferenced object 0xffff800fd5ea4880 (size 128):
  comm "swapper/0", pid 1, jiffies 4294939510 (age 668.680s)
  hex dump (first 32 bytes):
    00 00 00 00 20 00 00 00 00 00 00 00 00 00 00 00  .... ...........
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffff0000082c4ae4>] __kmalloc_node+0x278/0x634
    [<ffff0000088f4a74>] alloc_cpumask_var_node+0x28/0x60
    [<ffff0000088f4af0>] zalloc_cpumask_var+0x14/0x1c
    [<ffff000008d20254>] cppc_cpufreq_init+0xd0/0x19c
    [<ffff000008083828>] do_one_initcall+0xec/0x15c
    [<ffff000008cd1018>] kernel_init_freeable+0x1f4/0x2a4
    [<ffff0000089099b0>] kernel_init+0x18/0x10c
    [<ffff000008084d50>] ret_from_fork+0x10/0x18
    [<ffffffffffffffff>] 0xffffffffffffffff

V2->V1:
	remove the useless 'cpu' check in exit code.

CC: "Rafael J. Wysocki" <rjw@rjwysocki.net>
CC: Viresh Kumar <viresh.kumar@linaro.org>
CC: linux-pm@vger.kernel.org
Signed-off-by: Chunyu Hu <chuhu@redhat.com>
---
 drivers/cpufreq/cppc_cpufreq.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Viresh Kumar March 5, 2018, 6:01 a.m. UTC | #1
On 05-03-18, 13:40, Chunyu Hu wrote:
> Kmemleak reported the below leak. When cppc_cpufreq_init went into
> failure path, the cpu mask is not freed. After fix, this report is
> gone. And to avaoid potential NULL pointer reference, check the cpu
> value first.
> 
> unreferenced object 0xffff800fd5ea4880 (size 128):
>   comm "swapper/0", pid 1, jiffies 4294939510 (age 668.680s)
>   hex dump (first 32 bytes):
>     00 00 00 00 20 00 00 00 00 00 00 00 00 00 00 00  .... ...........
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffff0000082c4ae4>] __kmalloc_node+0x278/0x634
>     [<ffff0000088f4a74>] alloc_cpumask_var_node+0x28/0x60
>     [<ffff0000088f4af0>] zalloc_cpumask_var+0x14/0x1c
>     [<ffff000008d20254>] cppc_cpufreq_init+0xd0/0x19c
>     [<ffff000008083828>] do_one_initcall+0xec/0x15c
>     [<ffff000008cd1018>] kernel_init_freeable+0x1f4/0x2a4
>     [<ffff0000089099b0>] kernel_init+0x18/0x10c
>     [<ffff000008084d50>] ret_from_fork+0x10/0x18
>     [<ffffffffffffffff>] 0xffffffffffffffff
> 
> V2->V1:
> 	remove the useless 'cpu' check in exit code.
> 
> CC: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> CC: Viresh Kumar <viresh.kumar@linaro.org>
> CC: linux-pm@vger.kernel.org
> Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index a1c3025..8f7b21a 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -230,8 +230,13 @@ static int __init cppc_cpufreq_init(void)
>  	return ret;
>  
>  out:
> -	for_each_possible_cpu(i)
> -		kfree(all_cpu_data[i]);
> +	for_each_possible_cpu(i) {
> +		cpu = all_cpu_data[i];
> +		if (!cpu)
> +			break;
> +		free_cpumask_var(cpu->shared_cpu_map);
> +		kfree(cpu);
> +	}
>  
>  	kfree(all_cpu_data);
>  	return -ENODEV;

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Rafael J. Wysocki March 22, 2018, 11:27 p.m. UTC | #2
On Monday, March 5, 2018 7:01:26 AM CET Viresh Kumar wrote:
> On 05-03-18, 13:40, Chunyu Hu wrote:
> > Kmemleak reported the below leak. When cppc_cpufreq_init went into
> > failure path, the cpu mask is not freed. After fix, this report is
> > gone. And to avaoid potential NULL pointer reference, check the cpu
> > value first.
> > 
> > unreferenced object 0xffff800fd5ea4880 (size 128):
> >   comm "swapper/0", pid 1, jiffies 4294939510 (age 668.680s)
> >   hex dump (first 32 bytes):
> >     00 00 00 00 20 00 00 00 00 00 00 00 00 00 00 00  .... ...........
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >   backtrace:
> >     [<ffff0000082c4ae4>] __kmalloc_node+0x278/0x634
> >     [<ffff0000088f4a74>] alloc_cpumask_var_node+0x28/0x60
> >     [<ffff0000088f4af0>] zalloc_cpumask_var+0x14/0x1c
> >     [<ffff000008d20254>] cppc_cpufreq_init+0xd0/0x19c
> >     [<ffff000008083828>] do_one_initcall+0xec/0x15c
> >     [<ffff000008cd1018>] kernel_init_freeable+0x1f4/0x2a4
> >     [<ffff0000089099b0>] kernel_init+0x18/0x10c
> >     [<ffff000008084d50>] ret_from_fork+0x10/0x18
> >     [<ffffffffffffffff>] 0xffffffffffffffff
> > 
> > V2->V1:
> > 	remove the useless 'cpu' check in exit code.
> > 
> > CC: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > CC: Viresh Kumar <viresh.kumar@linaro.org>
> > CC: linux-pm@vger.kernel.org
> > Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> > ---
> >  drivers/cpufreq/cppc_cpufreq.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> > index a1c3025..8f7b21a 100644
> > --- a/drivers/cpufreq/cppc_cpufreq.c
> > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > @@ -230,8 +230,13 @@ static int __init cppc_cpufreq_init(void)
> >  	return ret;
> >  
> >  out:
> > -	for_each_possible_cpu(i)
> > -		kfree(all_cpu_data[i]);
> > +	for_each_possible_cpu(i) {
> > +		cpu = all_cpu_data[i];
> > +		if (!cpu)
> > +			break;
> > +		free_cpumask_var(cpu->shared_cpu_map);
> > +		kfree(cpu);
> > +	}
> >  
> >  	kfree(all_cpu_data);
> >  	return -ENODEV;
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Applied, thanks!
diff mbox

Patch

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index a1c3025..8f7b21a 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -230,8 +230,13 @@  static int __init cppc_cpufreq_init(void)
 	return ret;
 
 out:
-	for_each_possible_cpu(i)
-		kfree(all_cpu_data[i]);
+	for_each_possible_cpu(i) {
+		cpu = all_cpu_data[i];
+		if (!cpu)
+			break;
+		free_cpumask_var(cpu->shared_cpu_map);
+		kfree(cpu);
+	}
 
 	kfree(all_cpu_data);
 	return -ENODEV;