Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1410190 > unrolled thread

[PATCH 2/3] drivers/perf: arm_pmu: Defer the setting of __oprofile_cpu_pmu

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/3] drivers/perf: arm_pmu: Defer the setting of __oprofile_cpu_pmu Julien Grall <julien.grall@arm.com> - 2016-05-31 13:50 +0200
    Re: [PATCH 2/3] drivers/perf: arm_pmu: Defer the setting of  __oprofile_cpu_pmu Will Deacon <will.deacon@arm.com> - 2016-05-31 18:30 +0200
      Re: [PATCH 2/3] drivers/perf: arm_pmu: Defer the setting of  __oprofile_cpu_pmu Mark Rutland <mark.rutland@arm.com> - 2016-05-31 19:40 +0200

#1410190 — [PATCH 2/3] drivers/perf: arm_pmu: Defer the setting of __oprofile_cpu_pmu

The global variable __oprofile_cpu_pmu is set before the PMU is fully
initialized. If an error occurs before the end of the initialization,
the PMU will be freed and the variable will contain an invalid pointer.

This will result in a kernel crash when perf will be used.

Fix it by moving the setting of __oprofile_cpu_pmu when the PMU is fully
initialized (i.e when it is no longer possible to fail).

Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
 drivers/perf/arm_pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 6401f0c..95614d2 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -992,9 +992,6 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 
 	armpmu_init(pmu);
 
-	if (!__oprofile_cpu_pmu)
-		__oprofile_cpu_pmu = pmu;
-
 	pmu->plat_device = pdev;
 
 	if (node && (of_id = of_match_node(of_table, pdev->dev.of_node))) {
@@ -1030,6 +1027,9 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 	if (ret)
 		goto out_destroy;
 
+	if (!__oprofile_cpu_pmu)
+		__oprofile_cpu_pmu = pmu;
+
 	pr_info("enabled with %s PMU driver, %d counters available\n",
 			pmu->name, pmu->num_events);
 
-- 
1.9.1

[toc] | [next] | [standalone]

#1410408 — Re: [PATCH 2/3] drivers/perf: arm_pmu: Defer the setting of __oprofile_cpu_pmu

On Tue, May 31, 2016 at 12:41:22PM +0100, Julien Grall wrote:
> The global variable __oprofile_cpu_pmu is set before the PMU is fully
> initialized. If an error occurs before the end of the initialization,
> the PMU will be freed and the variable will contain an invalid pointer.
> 
> This will result in a kernel crash when perf will be used.
> 
> Fix it by moving the setting of __oprofile_cpu_pmu when the PMU is fully
> initialized (i.e when it is no longer possible to fail).
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  drivers/perf/arm_pmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Should this one go to -stable too?

Will

> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 6401f0c..95614d2 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -992,9 +992,6 @@ int arm_pmu_device_probe(struct platform_device *pdev,
>  
>  	armpmu_init(pmu);
>  
> -	if (!__oprofile_cpu_pmu)
> -		__oprofile_cpu_pmu = pmu;
> -
>  	pmu->plat_device = pdev;
>  
>  	if (node && (of_id = of_match_node(of_table, pdev->dev.of_node))) {
> @@ -1030,6 +1027,9 @@ int arm_pmu_device_probe(struct platform_device *pdev,
>  	if (ret)
>  		goto out_destroy;
>  
> +	if (!__oprofile_cpu_pmu)
> +		__oprofile_cpu_pmu = pmu;
> +
>  	pr_info("enabled with %s PMU driver, %d counters available\n",
>  			pmu->name, pmu->num_events);
>  
> -- 
> 1.9.1
> 

[toc] | [prev] | [next] | [standalone]

#1410482 — Re: [PATCH 2/3] drivers/perf: arm_pmu: Defer the setting of __oprofile_cpu_pmu

On Tue, May 31, 2016 at 05:28:34PM +0100, Will Deacon wrote:
> On Tue, May 31, 2016 at 12:41:22PM +0100, Julien Grall wrote:
> > The global variable __oprofile_cpu_pmu is set before the PMU is fully
> > initialized. If an error occurs before the end of the initialization,
> > the PMU will be freed and the variable will contain an invalid pointer.
> > 
> > This will result in a kernel crash when perf will be used.
> > 
> > Fix it by moving the setting of __oprofile_cpu_pmu when the PMU is fully
> > initialized (i.e when it is no longer possible to fail).
> > 
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  drivers/perf/arm_pmu.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Should this one go to -stable too?

I think so.

The bug has been there at least since 76b8a0e4c8bda5f0 ("ARM: perf:
handle armpmu_register failing"), in v3.8...

Prior to that we wouldn't free the PMU, but it might not have been
initialised correctly.

Thanks,
Mark.
 
> Will
> 
> > diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> > index 6401f0c..95614d2 100644
> > --- a/drivers/perf/arm_pmu.c
> > +++ b/drivers/perf/arm_pmu.c
> > @@ -992,9 +992,6 @@ int arm_pmu_device_probe(struct platform_device *pdev,
> >  
> >  	armpmu_init(pmu);
> >  
> > -	if (!__oprofile_cpu_pmu)
> > -		__oprofile_cpu_pmu = pmu;
> > -
> >  	pmu->plat_device = pdev;
> >  
> >  	if (node && (of_id = of_match_node(of_table, pdev->dev.of_node))) {
> > @@ -1030,6 +1027,9 @@ int arm_pmu_device_probe(struct platform_device *pdev,
> >  	if (ret)
> >  		goto out_destroy;
> >  
> > +	if (!__oprofile_cpu_pmu)
> > +		__oprofile_cpu_pmu = pmu;
> > +
> >  	pr_info("enabled with %s PMU driver, %d counters available\n",
> >  			pmu->name, pmu->num_events);
> >  
> > -- 
> > 1.9.1
> > 
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web