[PATCH] clockevent: sun4i: Fix race condition in the probe code

[PATCH] clockevent: sun4i: Fix race condition in the probe code

[Maxime Ripard]

The interrupts were activated and the handler registered before the clockevent
was registered in the probe function.

The interrupt handler, however, was making the assumption that the clockevent
device was registered.

That could cause a null pointer dereferenc if the timer interrupt was firing
during this narrow window.

Fix that by moving the clockevent registration before the interrupt is enabled.

Reported-by: Roman Byshko <rbyshko@gmail.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: stable@vger.kernel.org
---
 drivers/clocksource/sun4i_timer.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index efb17c3ee120..f4a9c0058b4d 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -182,6 +182,12 @@ static void __init sun4i_timer_init(struct device_node *node)
 	/* Make sure timer is stopped before playing with interrupts */
 	sun4i_clkevt_time_stop(0);
 
+	sun4i_clockevent.cpumask = cpu_possible_mask;
+	sun4i_clockevent.irq = irq;
+
+	clockevents_config_and_register(&sun4i_clockevent, rate,
+					TIMER_SYNC_TICKS, 0xffffffff);
+
 	ret = setup_irq(irq, &sun4i_timer_irq);
 	if (ret)
 		pr_warn("failed to setup irq %d\n", irq);
@@ -189,12 +195,6 @@ static void __init sun4i_timer_init(struct device_node *node)
 	/* Enable timer0 interrupt */
 	val = readl(timer_base + TIMER_IRQ_EN_REG);
 	writel(val | TIMER_IRQ_EN(0), timer_base + TIMER_IRQ_EN_REG);
-
-	sun4i_clockevent.cpumask = cpu_possible_mask;
-	sun4i_clockevent.irq = irq;
-
-	clockevents_config_and_register(&sun4i_clockevent, rate,
-					TIMER_SYNC_TICKS, 0xffffffff);
 }
 CLOCKSOURCE_OF_DECLARE(sun4i, "allwinner,sun4i-a10-timer",
 		       sun4i_timer_init);
-- 
2.1.1

Re: [PATCH] clockevent: sun4i: Fix race condition in the probe code

[Daniel Lezcano]

On 11/18/2014 11:59 PM, Maxime Ripard wrote:
> The interrupts were activated and the handler registered before the clockevent
> was registered in the probe function.
>
> The interrupt handler, however, was making the assumption that the clockevent
> device was registered.
>
> That could cause a null pointer dereferenc if the timer interrupt was firing
> during this narrow window.
>
> Fix that by moving the clockevent registration before the interrupt is enabled.
>
> Reported-by: Roman Byshko <rbyshko@gmail.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: stable@vger.kernel.org

Applied to my tree as 3.18 fix.

Thanks !

   -- Daniel

> ---
>   drivers/clocksource/sun4i_timer.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
> index efb17c3ee120..f4a9c0058b4d 100644
> --- a/drivers/clocksource/sun4i_timer.c
> +++ b/drivers/clocksource/sun4i_timer.c
> @@ -182,6 +182,12 @@ static void __init sun4i_timer_init(struct device_node *node)
>   	/* Make sure timer is stopped before playing with interrupts */
>   	sun4i_clkevt_time_stop(0);
>
> +	sun4i_clockevent.cpumask = cpu_possible_mask;
> +	sun4i_clockevent.irq = irq;
> +
> +	clockevents_config_and_register(&sun4i_clockevent, rate,
> +					TIMER_SYNC_TICKS, 0xffffffff);
> +
>   	ret = setup_irq(irq, &sun4i_timer_irq);
>   	if (ret)
>   		pr_warn("failed to setup irq %d\n", irq);
> @@ -189,12 +195,6 @@ static void __init sun4i_timer_init(struct device_node *node)
>   	/* Enable timer0 interrupt */
>   	val = readl(timer_base + TIMER_IRQ_EN_REG);
>   	writel(val | TIMER_IRQ_EN(0), timer_base + TIMER_IRQ_EN_REG);
> -
> -	sun4i_clockevent.cpumask = cpu_possible_mask;
> -	sun4i_clockevent.irq = irq;
> -
> -	clockevents_config_and_register(&sun4i_clockevent, rate,
> -					TIMER_SYNC_TICKS, 0xffffffff);
>   }
>   CLOCKSOURCE_OF_DECLARE(sun4i, "allwinner,sun4i-a10-timer",
>   		       sun4i_timer_init);
>


-- 
  <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

[GIT PULL] clockevents: 3.18 fix

[Daniel Lezcano]

Hi Thomas,

This pull request, for timers/urgent, contains one fix for the sun4i 
board where the interrupt handler was registered before the timer 
itself, thus opening a window where the interrupt could be fired and 
leading to an uninitialized data access.

Thanks !

   -- Daniel


The following changes since commit 59aa896db80479dec29f471a7ca2b9eeeeb7d38e:

   ARM/ARM64: arch-timer: fix arch_timer_probed logic (2014-10-26 
20:50:00 +0100)

are available in the git repository at:

   http://git.linaro.org/git-ro/people/daniel.lezcano/linux.git 
clockevents/3.18-fixes

for you to fetch changes up to 6bab4a8a1888729f17f4923cc5867e4674f66333:

   clockevent: sun4i: Fix race condition in the probe code (2014-11-19 
10:43:51 +0100)

----------------------------------------------------------------
Maxime Ripard (1):
       clockevent: sun4i: Fix race condition in the probe code

  drivers/clocksource/sun4i_timer.c | 12 ++++++------
  1 file changed, 6 insertions(+), 6 deletions(-)


-- 
  <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

[PATCH] clockevent: sun4i: Fix race condition in the probe code

[Daniel Lezcano]

From: Maxime Ripard <maxime.ripard@free-electrons.com>

The interrupts were activated and the handler registered before the clockevent
was registered in the probe function.

The interrupt handler, however, was making the assumption that the clockevent
device was registered.

That could cause a null pointer dereference if the timer interrupt was firing
during this narrow window.

Fix that by moving the clockevent registration before the interrupt is enabled.

Reported-by: Roman Byshko <rbyshko@gmail.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/sun4i_timer.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index efb17c3..f4a9c00 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -182,6 +182,12 @@ static void __init sun4i_timer_init(struct device_node *node)
 	/* Make sure timer is stopped before playing with interrupts */
 	sun4i_clkevt_time_stop(0);
 
+	sun4i_clockevent.cpumask = cpu_possible_mask;
+	sun4i_clockevent.irq = irq;
+
+	clockevents_config_and_register(&sun4i_clockevent, rate,
+					TIMER_SYNC_TICKS, 0xffffffff);
+
 	ret = setup_irq(irq, &sun4i_timer_irq);
 	if (ret)
 		pr_warn("failed to setup irq %d\n", irq);
@@ -189,12 +195,6 @@ static void __init sun4i_timer_init(struct device_node *node)
 	/* Enable timer0 interrupt */
 	val = readl(timer_base + TIMER_IRQ_EN_REG);
 	writel(val | TIMER_IRQ_EN(0), timer_base + TIMER_IRQ_EN_REG);
-
-	sun4i_clockevent.cpumask = cpu_possible_mask;
-	sun4i_clockevent.irq = irq;
-
-	clockevents_config_and_register(&sun4i_clockevent, rate,
-					TIMER_SYNC_TICKS, 0xffffffff);
 }
 CLOCKSOURCE_OF_DECLARE(sun4i, "allwinner,sun4i-a10-timer",
 		       sun4i_timer_init);
-- 
1.9.1