mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20
@ 2011-05-07  1:20 John Stultz
  2011-05-07  1:20 ` [PATCH 01/11] rtc: mxc: Initialize drvdata before registering device John Stultz
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml; +Cc: John Stultz, Alessandro Zummo, Thomas Gleixner, rtc-linux

Since commit f44f7f96a20 ("RTC: Initialize kernel state from RTC")
landed in 2.6.39-rc1, we've seen a few reports of boot time hangs
cropping up. It ends up that the problematic rtc drivers were
calling rtc_device_register before it finished initializing critical
data for the rtc driver to function.

Wolfram Sang noticed that this issue was actually more common then
just the few reports seen, and motivated me to do a complete audit
of all the RTC drivers to ensure they were not being registered
before any critical drvdata was initialized.

This patchset is the result of that audit, along with earlier
noted fixes from Wolfram's and Uwe.

Thanks again to Wolfram and Uwe for the patches and helping make
these issues a priority.

CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com


John Stultz (7):
  rtc: ds1286: Initialize drvdata before registering device
  rtc: m41t80: Initialize clientdata before registering device
  rtc: max8925: Initialize drvdata before registering device
  rtc: max8998: Initialize drvdata before registering device
  rtc: msm6242: Initialize drvdata before registering device
  rtc: pcap: Initialize drvdata before registering device
  rtc: rp5c01: Initialize drvdata before registering device

Uwe Kleine-König (1):
  rtc: mc13xxx: Don't call rtc_device_register while holding lock

Wolfram Sang (3):
  rtc: mxc: Initialize drvdata before registering device
  rtc: davinci: Initialize drvdata before registering device
  rtc: ep93xx: Initialize drvdata before registering device

 drivers/rtc/rtc-davinci.c |    5 +++--
 drivers/rtc/rtc-ds1286.c  |    2 +-
 drivers/rtc/rtc-ep93xx.c  |    5 ++---
 drivers/rtc/rtc-m41t80.c  |    5 +++--
 drivers/rtc/rtc-max8925.c |    5 +++--
 drivers/rtc/rtc-max8998.c |    5 +++--
 drivers/rtc/rtc-mc13xxx.c |    8 ++++++--
 drivers/rtc/rtc-msm6242.c |    3 ++-
 drivers/rtc/rtc-mxc.c     |   19 +++++++++++--------
 drivers/rtc/rtc-pcap.c    |    4 +++-
 drivers/rtc/rtc-rp5c01.c  |    5 +++--
 11 files changed, 40 insertions(+), 26 deletions(-)

-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 01/11] rtc: mxc: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 02/11] rtc: davinci: " John Stultz
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux, John Stultz

From: Wolfram Sang <w.sang@pengutronix.de>

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
[fixed up commit log -jstultz]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-mxc.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-mxc.c b/drivers/rtc/rtc-mxc.c
index 826ab64..d814417 100644
--- a/drivers/rtc/rtc-mxc.c
+++ b/drivers/rtc/rtc-mxc.c
@@ -418,14 +418,6 @@ static int __init mxc_rtc_probe(struct platform_device *pdev)
 		goto exit_put_clk;
 	}
 
-	rtc = rtc_device_register(pdev->name, &pdev->dev, &mxc_rtc_ops,
-				  THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
-		goto exit_put_clk;
-	}
-
-	pdata->rtc = rtc;
 	platform_set_drvdata(pdev, pdata);
 
 	/* Configure and enable the RTC */
@@ -438,8 +430,19 @@ static int __init mxc_rtc_probe(struct platform_device *pdev)
 		pdata->irq = -1;
 	}
 
+	rtc = rtc_device_register(pdev->name, &pdev->dev, &mxc_rtc_ops,
+				  THIS_MODULE);
+	if (IS_ERR(rtc)) {
+		ret = PTR_ERR(rtc);
+		goto exit_clr_drvdata;
+	}
+
+	pdata->rtc = rtc;
+
 	return 0;
 
+exit_clr_drvdata:
+	platform_set_drvdata(pdev, NULL);
 exit_put_clk:
 	clk_disable(pdata->clk);
 	clk_put(pdata->clk);
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 02/11] rtc: davinci: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
  2011-05-07  1:20 ` [PATCH 01/11] rtc: mxc: Initialize drvdata before registering device John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 03/11] rtc: ep93xx: " John Stultz
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux, John Stultz

From: Wolfram Sang <w.sang@pengutronix.de>

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
[fixed up commit log -jstultz]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-davinci.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-davinci.c b/drivers/rtc/rtc-davinci.c
index 8d46838..755e1fe 100644
--- a/drivers/rtc/rtc-davinci.c
+++ b/drivers/rtc/rtc-davinci.c
@@ -524,6 +524,8 @@ static int __init davinci_rtc_probe(struct platform_device *pdev)
 		goto fail2;
 	}
 
+	platform_set_drvdata(pdev, davinci_rtc);
+
 	davinci_rtc->rtc = rtc_device_register(pdev->name, &pdev->dev,
 				    &davinci_rtc_ops, THIS_MODULE);
 	if (IS_ERR(davinci_rtc->rtc)) {
@@ -553,8 +555,6 @@ static int __init davinci_rtc_probe(struct platform_device *pdev)
 
 	rtcss_write(davinci_rtc, PRTCSS_RTC_CCTRL_CAEN, PRTCSS_RTC_CCTRL);
 
-	platform_set_drvdata(pdev, davinci_rtc);
-
 	device_init_wakeup(&pdev->dev, 0);
 
 	return 0;
@@ -562,6 +562,7 @@ static int __init davinci_rtc_probe(struct platform_device *pdev)
 fail4:
 	rtc_device_unregister(davinci_rtc->rtc);
 fail3:
+	platform_set_drvdata(pdev, NULL);
 	iounmap(davinci_rtc->base);
 fail2:
 	release_mem_region(davinci_rtc->pbase, davinci_rtc->base_size);
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 03/11] rtc: ep93xx: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
  2011-05-07  1:20 ` [PATCH 01/11] rtc: mxc: Initialize drvdata before registering device John Stultz
  2011-05-07  1:20 ` [PATCH 02/11] rtc: davinci: " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 04/11] rtc: ds1286: " John Stultz
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux, John Stultz

From: Wolfram Sang <w.sang@pengutronix.de>

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
[Fixed up commit log -jstultz]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-ep93xx.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-ep93xx.c b/drivers/rtc/rtc-ep93xx.c
index 11ae64d..335551d 100644
--- a/drivers/rtc/rtc-ep93xx.c
+++ b/drivers/rtc/rtc-ep93xx.c
@@ -151,6 +151,7 @@ static int __init ep93xx_rtc_probe(struct platform_device *pdev)
 		return -ENXIO;
 
 	pdev->dev.platform_data = ep93xx_rtc;
+	platform_set_drvdata(pdev, rtc);
 
 	rtc = rtc_device_register(pdev->name,
 				&pdev->dev, &ep93xx_rtc_ops, THIS_MODULE);
@@ -159,8 +160,6 @@ static int __init ep93xx_rtc_probe(struct platform_device *pdev)
 		goto exit;
 	}
 
-	platform_set_drvdata(pdev, rtc);
-
 	err = sysfs_create_group(&pdev->dev.kobj, &ep93xx_rtc_sysfs_files);
 	if (err)
 		goto fail;
@@ -168,9 +167,9 @@ static int __init ep93xx_rtc_probe(struct platform_device *pdev)
 	return 0;
 
 fail:
-	platform_set_drvdata(pdev, NULL);
 	rtc_device_unregister(rtc);
 exit:
+	platform_set_drvdata(pdev, NULL);
 	pdev->dev.platform_data = NULL;
 	return err;
 }
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 04/11] rtc: ds1286: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
                   ` (2 preceding siblings ...)
  2011-05-07  1:20 ` [PATCH 03/11] rtc: ep93xx: " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 05/11] rtc: m41t80: Initialize clientdata " John Stultz
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Wolfram Sang <w.sang@pengutronix.de>
CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-ds1286.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/rtc-ds1286.c b/drivers/rtc/rtc-ds1286.c
index 60ce696..47e681d 100644
--- a/drivers/rtc/rtc-ds1286.c
+++ b/drivers/rtc/rtc-ds1286.c
@@ -355,6 +355,7 @@ static int __devinit ds1286_probe(struct platform_device *pdev)
 		goto out;
 	}
 	spin_lock_init(&priv->lock);
+	platform_set_drvdata(pdev, priv);
 	rtc = rtc_device_register("ds1286", &pdev->dev,
 				  &ds1286_ops, THIS_MODULE);
 	if (IS_ERR(rtc)) {
@@ -362,7 +363,6 @@ static int __devinit ds1286_probe(struct platform_device *pdev)
 		goto out;
 	}
 	priv->rtc = rtc;
-	platform_set_drvdata(pdev, priv);
 	return 0;
 
 out:
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 05/11] rtc: m41t80: Initialize clientdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
                   ` (3 preceding siblings ...)
  2011-05-07  1:20 ` [PATCH 04/11] rtc: ds1286: " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 06/11] rtc: max8925: Initialize drvdata " John Stultz
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the clientdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the clientdata is initialized prior to registering
the rtc device.

CC: Wolfram Sang <w.sang@pengutronix.de>
CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-m41t80.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index 69fe664..eda128f 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -783,6 +783,9 @@ static int m41t80_probe(struct i2c_client *client,
 		goto exit;
 	}
 
+	clientdata->features = id->driver_data;
+	i2c_set_clientdata(client, clientdata);
+
 	rtc = rtc_device_register(client->name, &client->dev,
 				  &m41t80_rtc_ops, THIS_MODULE);
 	if (IS_ERR(rtc)) {
@@ -792,8 +795,6 @@ static int m41t80_probe(struct i2c_client *client,
 	}
 
 	clientdata->rtc = rtc;
-	clientdata->features = id->driver_data;
-	i2c_set_clientdata(client, clientdata);
 
 	/* Make sure HT (Halt Update) bit is cleared */
 	rc = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_HOUR);
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 06/11] rtc: max8925: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
                   ` (4 preceding siblings ...)
  2011-05-07  1:20 ` [PATCH 05/11] rtc: m41t80: Initialize clientdata " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 07/11] rtc: max8998: " John Stultz
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Wolfram Sang <w.sang@pengutronix.de>
CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-max8925.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-max8925.c b/drivers/rtc/rtc-max8925.c
index 20494b5..3bc046f 100644
--- a/drivers/rtc/rtc-max8925.c
+++ b/drivers/rtc/rtc-max8925.c
@@ -258,6 +258,8 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 	}
 
 	dev_set_drvdata(&pdev->dev, info);
+	/* XXX - isn't this redundant? */
+	platform_set_drvdata(pdev, info);
 
 	info->rtc_dev = rtc_device_register("max8925-rtc", &pdev->dev,
 					&max8925_rtc_ops, THIS_MODULE);
@@ -267,10 +269,9 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 		goto out_rtc;
 	}
 
-	platform_set_drvdata(pdev, info);
-
 	return 0;
 out_rtc:
+	platform_set_drvdata(pdev, NULL);
 	free_irq(chip->irq_base + MAX8925_IRQ_RTC_ALARM0, info);
 out_irq:
 	kfree(info);
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 07/11] rtc: max8998: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
                   ` (5 preceding siblings ...)
  2011-05-07  1:20 ` [PATCH 06/11] rtc: max8925: Initialize drvdata " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 08/11] rtc: msm6242: " John Stultz
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Wolfram Sang <w.sang@pengutronix.de>
CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-max8998.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index 3f7bc6b..2e48aa6 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -265,6 +265,8 @@ static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 	info->rtc = max8998->rtc;
 	info->irq = max8998->irq_base + MAX8998_IRQ_ALARM0;
 
+	platform_set_drvdata(pdev, info);
+
 	info->rtc_dev = rtc_device_register("max8998-rtc", &pdev->dev,
 			&max8998_rtc_ops, THIS_MODULE);
 
@@ -274,8 +276,6 @@ static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 		goto out_rtc;
 	}
 
-	platform_set_drvdata(pdev, info);
-
 	ret = request_threaded_irq(info->irq, NULL, max8998_rtc_alarm_irq, 0,
 			"rtc-alarm0", info);
 
@@ -293,6 +293,7 @@ static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 	return 0;
 
 out_rtc:
+	platform_set_drvdata(pdev, NULL);
 	kfree(info);
 	return ret;
 }
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 08/11] rtc: msm6242: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
                   ` (6 preceding siblings ...)
  2011-05-07  1:20 ` [PATCH 07/11] rtc: max8998: " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 09/11] rtc: pcap: " John Stultz
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Wolfram Sang <w.sang@pengutronix.de>
CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-msm6242.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/rtc-msm6242.c b/drivers/rtc/rtc-msm6242.c
index 6782062..fcb113c 100644
--- a/drivers/rtc/rtc-msm6242.c
+++ b/drivers/rtc/rtc-msm6242.c
@@ -214,6 +214,7 @@ static int __init msm6242_rtc_probe(struct platform_device *dev)
 		error = -ENOMEM;
 		goto out_free_priv;
 	}
+	platform_set_drvdata(dev, priv);
 
 	rtc = rtc_device_register("rtc-msm6242", &dev->dev, &msm6242_rtc_ops,
 				  THIS_MODULE);
@@ -223,10 +224,10 @@ static int __init msm6242_rtc_probe(struct platform_device *dev)
 	}
 
 	priv->rtc = rtc;
-	platform_set_drvdata(dev, priv);
 	return 0;
 
 out_unmap:
+	platform_set_drvdata(dev, NULL);
 	iounmap(priv->regs);
 out_free_priv:
 	kfree(priv);
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 09/11] rtc: pcap: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
                   ` (7 preceding siblings ...)
  2011-05-07  1:20 ` [PATCH 08/11] rtc: msm6242: " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 10/11] rtc: rp5c01: " John Stultz
  2011-05-07  1:20 ` [PATCH 11/11] rtc: mc13xxx: Don't call rtc_device_register while holding lock John Stultz
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Wolfram Sang <w.sang@pengutronix.de>
CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-pcap.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/rtc-pcap.c b/drivers/rtc/rtc-pcap.c
index a633abc..cd4f198 100644
--- a/drivers/rtc/rtc-pcap.c
+++ b/drivers/rtc/rtc-pcap.c
@@ -151,6 +151,8 @@ static int __devinit pcap_rtc_probe(struct platform_device *pdev)
 
 	pcap_rtc->pcap = dev_get_drvdata(pdev->dev.parent);
 
+	platform_set_drvdata(pdev, pcap_rtc);
+
 	pcap_rtc->rtc = rtc_device_register("pcap", &pdev->dev,
 				  &pcap_rtc_ops, THIS_MODULE);
 	if (IS_ERR(pcap_rtc->rtc)) {
@@ -158,7 +160,6 @@ static int __devinit pcap_rtc_probe(struct platform_device *pdev)
 		goto fail_rtc;
 	}
 
-	platform_set_drvdata(pdev, pcap_rtc);
 
 	timer_irq = pcap_to_irq(pcap_rtc->pcap, PCAP_IRQ_1HZ);
 	alarm_irq = pcap_to_irq(pcap_rtc->pcap, PCAP_IRQ_TODA);
@@ -177,6 +178,7 @@ fail_alarm:
 fail_timer:
 	rtc_device_unregister(pcap_rtc->rtc);
 fail_rtc:
+	platform_set_drvdata(pdev, NULL);
 	kfree(pcap_rtc);
 	return err;
 }
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 10/11] rtc: rp5c01: Initialize drvdata before registering device
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
                   ` (8 preceding siblings ...)
  2011-05-07  1:20 ` [PATCH 09/11] rtc: pcap: " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  2011-05-07  1:20 ` [PATCH 11/11] rtc: mc13xxx: Don't call rtc_device_register while holding lock John Stultz
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wolfram Sang, Alessandro Zummo, Thomas Gleixner, rtc-linux

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered
an issue in a number of RTC drivers, where the drivers call
rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the
rtc_device_register immediately makes use of the rtc device, calling
rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering
the rtc device.

CC: Wolfram Sang <w.sang@pengutronix.de>
CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: rtc-linux@googlegroups.com
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-rp5c01.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-rp5c01.c b/drivers/rtc/rtc-rp5c01.c
index 694da39..359da6d 100644
--- a/drivers/rtc/rtc-rp5c01.c
+++ b/drivers/rtc/rtc-rp5c01.c
@@ -249,15 +249,15 @@ static int __init rp5c01_rtc_probe(struct platform_device *dev)
 
 	spin_lock_init(&priv->lock);
 
+	platform_set_drvdata(dev, priv);
+
 	rtc = rtc_device_register("rtc-rp5c01", &dev->dev, &rp5c01_rtc_ops,
 				  THIS_MODULE);
 	if (IS_ERR(rtc)) {
 		error = PTR_ERR(rtc);
 		goto out_unmap;
 	}
-
 	priv->rtc = rtc;
-	platform_set_drvdata(dev, priv);
 
 	error = sysfs_create_bin_file(&dev->dev.kobj, &priv->nvram_attr);
 	if (error)
@@ -268,6 +268,7 @@ static int __init rp5c01_rtc_probe(struct platform_device *dev)
 out_unregister:
 	rtc_device_unregister(rtc);
 out_unmap:
+	platform_set_drvdata(dev, NULL);
 	iounmap(priv->regs);
 out_free_priv:
 	kfree(priv);
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 11/11] rtc: mc13xxx: Don't call rtc_device_register while holding lock
  2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
                   ` (9 preceding siblings ...)
  2011-05-07  1:20 ` [PATCH 10/11] rtc: rp5c01: " John Stultz
@ 2011-05-07  1:20 ` John Stultz
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2011-05-07  1:20 UTC (permalink / raw)
  To: lkml; +Cc: Uwe Kleine-König, John Stultz

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Since commit f44f7f9 (RTC: Initialize kernel state from RTC)
rtc_device_register reads the programmed alarm. As reading the alarm
needs to take the mc13xxx lock, release it before calling
rtc_device_register.

This fixes a deadlock during boot:

	INFO: task swapper:1 blocked for more than 120 seconds.
	"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
	swapper         D c02b175c     0     1      0 0x00000000
	[<c02b175c>] (schedule+0x304/0x4f4) from [<c02b25a8>] (__mutex_lock_slowpath+0x7c/0x110)
	[<c02b25a8>] (__mutex_lock_slowpath+0x7c/0x110) from [<c020b4cc>] (mc13xxx_rtc_read_time+0x1c/0x118)
	[<c020b4cc>] (mc13xxx_rtc_read_time+0x1c/0x118) from [<c0208f04>] (__rtc_read_time+0x58/0x5c)
	[<c0208f04>] (__rtc_read_time+0x58/0x5c) from [<c0209508>] (rtc_read_time+0x30/0x48)
	[<c0209508>] (rtc_read_time+0x30/0x48) from [<c0209dd4>] (__rtc_read_alarm+0x1c/0x290)
	[<c0209dd4>] (__rtc_read_alarm+0x1c/0x290) from [<c0208d58>] (rtc_device_register+0x150/0x27c)
	[<c0208d58>] (rtc_device_register+0x150/0x27c) from [<c02b0b74>] (mc13xxx_rtc_probe+0x128/0x17c)
	[<c02b0b74>] (mc13xxx_rtc_probe+0x128/0x17c) from [<c01d5280>] (platform_drv_probe+0x1c/0x24)
	[<c01d5280>] (platform_drv_probe+0x1c/0x24) from [<c01d3e58>] (driver_probe_device+0x80/0x1a8)
	[<c01d3e58>] (driver_probe_device+0x80/0x1a8) from [<c01d400c>] (__driver_attach+0x8c/0x90)
	[<c01d400c>] (__driver_attach+0x8c/0x90) from [<c01d3654>] (bus_for_each_dev+0x60/0x8c)
	[<c01d3654>] (bus_for_each_dev+0x60/0x8c) from [<c01d2f6c>] (bus_add_driver+0x180/0x248)
	[<c01d2f6c>] (bus_add_driver+0x180/0x248) from [<c01d4664>] (driver_register+0x70/0x15c)
	[<c01d4664>] (driver_register+0x70/0x15c) from [<c01d5700>] (platform_driver_probe+0x18/0x98)
	[<c01d5700>] (platform_driver_probe+0x18/0x98) from [<c00273a8>] (do_one_initcall+0x2c/0x168)
	[<c00273a8>] (do_one_initcall+0x2c/0x168) from [<c00083ac>] (kernel_init+0xa0/0x150)
	[<c00083ac>] (kernel_init+0xa0/0x150) from [<c0033ff8>] (kernel_thread_exit+0x0/0x8)

Reported-by: Vagrant Cascadian <vagrant@debian.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Closes: http://bugs.debian.org/625804
[Tweaked commit log -jstultz]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-mc13xxx.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-mc13xxx.c b/drivers/rtc/rtc-mc13xxx.c
index c5ac037..a1a278b 100644
--- a/drivers/rtc/rtc-mc13xxx.c
+++ b/drivers/rtc/rtc-mc13xxx.c
@@ -349,11 +349,15 @@ static int __devinit mc13xxx_rtc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_alarm_irq_request;
 
+	mc13xxx_unlock(mc13xxx);
+
 	priv->rtc = rtc_device_register(pdev->name,
 			&pdev->dev, &mc13xxx_rtc_ops, THIS_MODULE);
 	if (IS_ERR(priv->rtc)) {
 		ret = PTR_ERR(priv->rtc);
 
+		mc13xxx_lock(mc13xxx);
+
 		mc13xxx_irq_free(mc13xxx, MC13XXX_IRQ_TODA, priv);
 err_alarm_irq_request:
 
@@ -365,12 +369,12 @@ err_reset_irq_status:
 		mc13xxx_irq_free(mc13xxx, MC13XXX_IRQ_RTCRST, priv);
 err_reset_irq_request:
 
+		mc13xxx_unlock(mc13xxx);
+
 		platform_set_drvdata(pdev, NULL);
 		kfree(priv);
 	}
 
-	mc13xxx_unlock(mc13xxx);
-
 	return ret;
 }
 
-- 
1.7.3.2.146.gca209


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-05-07  1:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-07  1:20 [PATCH 00/11] rtc: Fix rtc breakage found by commit f44f7f96a20 John Stultz
2011-05-07  1:20 ` [PATCH 01/11] rtc: mxc: Initialize drvdata before registering device John Stultz
2011-05-07  1:20 ` [PATCH 02/11] rtc: davinci: " John Stultz
2011-05-07  1:20 ` [PATCH 03/11] rtc: ep93xx: " John Stultz
2011-05-07  1:20 ` [PATCH 04/11] rtc: ds1286: " John Stultz
2011-05-07  1:20 ` [PATCH 05/11] rtc: m41t80: Initialize clientdata " John Stultz
2011-05-07  1:20 ` [PATCH 06/11] rtc: max8925: Initialize drvdata " John Stultz
2011-05-07  1:20 ` [PATCH 07/11] rtc: max8998: " John Stultz
2011-05-07  1:20 ` [PATCH 08/11] rtc: msm6242: " John Stultz
2011-05-07  1:20 ` [PATCH 09/11] rtc: pcap: " John Stultz
2011-05-07  1:20 ` [PATCH 10/11] rtc: rp5c01: " John Stultz
2011-05-07  1:20 ` [PATCH 11/11] rtc: mc13xxx: Don't call rtc_device_register while holding lock John Stultz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome