[PATCH 0/2] thermal: rcar: Fix crash and restore symmetry

[PATCH 0/2] thermal: rcar: Fix crash and restore symmetry

[Geert Uytterhoeven]

	Hi,

This series fixes an intermittent crash (NULL pointer dereference) in
the rcar-thermal driver due to a race condition, and restores symmetry
in the error and remove paths.

So far I could trigger the race condition only on r8a73a4, and only
after a soft power-on reset using the rmobile-reset driver, which is
queued for v3.20. I could not trigger it on r8a7791.
I haven't tested this on any other r8a779x, or on r8a7779.
Hence I'm not sure it's worthwhile adding this to -stable yet.

This was tested on r8a73a4/ape6evm (fixes the crash) and
r8a7791/koelsch (crash not seen).

Thanks!

Geert Uytterhoeven (2):
  thermal: rcar: Fix race condition between init and interrupt
  thermal: rcar: Restore symmetry in error and remove paths

 drivers/thermal/rcar_thermal.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/2] thermal: rcar: Fix race condition between init and interrupt

[Geert Uytterhoeven]

As soon as the interrupt has been enabled by devm_request_irq(), the
interrupt routine may be called, depending on the current status of the
hardware.

However, at that point rcar_thermal_common hasn't been initialized
complely yet. E.g. rcar_thermal_common.base is still NULL, causing a
NULL pointer dereference:

    Unable to handle kernel NULL pointer dereference at virtual address 0000000c
    pgd = c0004000
    [0000000c] *pgd=00000000
    Internal error: Oops: 5 [#1] SMP ARM
    CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc7-ape6evm-04564-gb6e46cb7cbe82389 #30
    Hardware name: Generic R8A73A4 (Flattened Device Tree)
    task: ee8953c0 ti: ee896000 task.ti: ee896000
    PC is at rcar_thermal_irq+0x1c/0xf0
    LR is at _raw_spin_lock_irqsave+0x48/0x54

Postpone the call to devm_request_irq() until all initialization has
been done to fix this.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This triggers sporadically on r8a73a4/ape6evm when starting a new
kernel after a soft power-on reset.

So far I couldn't trigger it on r8a7791/koelsch, not even by adding a
delay just after the call to devm_request_irq().
Probably none of the reboot methods on koelsch are sufficiently "soft"
(da9063-watchdog power-cycles the da9063 regulator, reset button resets
the r2a11302 regulator, even rwdt-restart on SMP=n seems fine)
---
 drivers/thermal/rcar_thermal.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 2580a4872f90febe..3c2c1720ba4a4a72 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -387,21 +387,9 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (irq) {
-		int ret;
-
 		/*
 		 * platform has IRQ support.
 		 * Then, driver uses common registers
-		 */
-
-		ret = devm_request_irq(dev, irq->start, rcar_thermal_irq, 0,
-				       dev_name(dev), common);
-		if (ret) {
-			dev_err(dev, "irq request failed\n ");
-			return ret;
-		}
-
-		/*
 		 * rcar_has_irq_support() will be enabled
 		 */
 		res = platform_get_resource(pdev, IORESOURCE_MEM, mres++);
@@ -456,8 +444,16 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 	}
 
 	/* enable temperature comparation */
-	if (irq)
+	if (irq) {
+		ret = devm_request_irq(dev, irq->start, rcar_thermal_irq, 0,
+				       dev_name(dev), common);
+		if (ret) {
+			dev_err(dev, "irq request failed\n ");
+			goto error_unregister;
+		}
+
 		rcar_thermal_common_write(common, ENR, enr_bits);
+	}
 
 	platform_set_drvdata(pdev, common);
 
-- 
1.9.1

[PATCH 2/2] thermal: rcar: Restore symmetry in error and remove paths

[Geert Uytterhoeven]

Swap interrupt disable and thermal zone unregistration in the error and
remove paths, to make them more symmetrical with the initialization
path.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/thermal/rcar_thermal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 3c2c1720ba4a4a72..fe4e767018c4cf73 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -463,9 +463,9 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 
 error_unregister:
 	rcar_thermal_for_each_priv(priv, common) {
-		thermal_zone_device_unregister(priv->zone);
 		if (rcar_has_irq_support(priv))
 			rcar_thermal_irq_disable(priv);
+		thermal_zone_device_unregister(priv->zone);
 	}
 
 	pm_runtime_put(dev);
@@ -481,9 +481,9 @@ static int rcar_thermal_remove(struct platform_device *pdev)
 	struct rcar_thermal_priv *priv;
 
 	rcar_thermal_for_each_priv(priv, common) {
-		thermal_zone_device_unregister(priv->zone);
 		if (rcar_has_irq_support(priv))
 			rcar_thermal_irq_disable(priv);
+		thermal_zone_device_unregister(priv->zone);
 	}
 
 	pm_runtime_put(dev);
-- 
1.9.1

Re: [PATCH 0/2] thermal: rcar: Fix crash and restore symmetry

[Kuninori Morimoto]

Hi Geert

> This series fixes an intermittent crash (NULL pointer dereference) in
> the rcar-thermal driver due to a race condition, and restores symmetry
> in the error and remove paths.
> 
> So far I could trigger the race condition only on r8a73a4, and only
> after a soft power-on reset using the rmobile-reset driver, which is
> queued for v3.20. I could not trigger it on r8a7791.
> I haven't tested this on any other r8a779x, or on r8a7779.
> Hence I'm not sure it's worthwhile adding this to -stable yet.
> 
> This was tested on r8a73a4/ape6evm (fixes the crash) and
> r8a7791/koelsch (crash not seen).

Indeed it seems bug. Thank you for your patch

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>