* [PATCH 1/3] rtc: mt6397: fix possible race condition
@ 2018-09-09 20:38 Alexandre Belloni
2018-09-09 20:38 ` [PATCH 2/3] rtc: pl030: " Alexandre Belloni
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Alexandre Belloni @ 2018-09-09 20:38 UTC (permalink / raw)
To: linux-rtc
Cc: linux-arm-kernel, linux-kernel, linux-mediatek,
Alexandre Belloni, Eddie Huang, Sean Wang
The IRQ is requested before the struct rtc is allocated and registered, but
this struct is used in the IRQ handler. This may lead to a NULL pointer
dereference.
Switch to devm_rtc_allocate_device/rtc_register_device to allocate the rtc
before requesting the IRQ.
Cc: Eddie Huang <eddie.huang@mediatek.com>
Cc: Sean Wang <sean.wang@mediatek.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-mt6397.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 385f8303bb41..e9a25ec4d434 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -332,6 +332,10 @@ static int mtk_rtc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, rtc);
+ rtc->rtc_dev = devm_rtc_allocate_device(rtc->dev);
+ if (IS_ERR(rtc->rtc_dev))
+ return PTR_ERR(rtc->rtc_dev);
+
ret = request_threaded_irq(rtc->irq, NULL,
mtk_rtc_irq_handler_thread,
IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
@@ -344,11 +348,11 @@ static int mtk_rtc_probe(struct platform_device *pdev)
device_init_wakeup(&pdev->dev, 1);
- rtc->rtc_dev = rtc_device_register("mt6397-rtc", &pdev->dev,
- &mtk_rtc_ops, THIS_MODULE);
- if (IS_ERR(rtc->rtc_dev)) {
+ rtc->rtc_dev->ops = &mtk_rtc_ops;
+
+ ret = rtc_register_device(rtc->rtc_dev);
+ if (ret) {
dev_err(&pdev->dev, "register rtc device failed\n");
- ret = PTR_ERR(rtc->rtc_dev);
goto out_free_irq;
}
@@ -365,7 +369,6 @@ static int mtk_rtc_remove(struct platform_device *pdev)
{
struct mt6397_rtc *rtc = platform_get_drvdata(pdev);
- rtc_device_unregister(rtc->rtc_dev);
free_irq(rtc->irq, rtc->rtc_dev);
irq_dispose_mapping(rtc->irq);
--
2.19.0.rc2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] rtc: pl030: fix possible race condition
2018-09-09 20:38 [PATCH 1/3] rtc: mt6397: fix possible race condition Alexandre Belloni
@ 2018-09-09 20:38 ` Alexandre Belloni
2018-09-09 20:38 ` [PATCH 3/3] rtc: pl031: switch to devm_rtc_allocate_device/rtc_register_device Alexandre Belloni
2018-09-11 11:32 ` [PATCH 1/3] rtc: mt6397: fix possible race condition Eddie Huang
2 siblings, 0 replies; 5+ messages in thread
From: Alexandre Belloni @ 2018-09-09 20:38 UTC (permalink / raw)
To: linux-rtc
Cc: linux-arm-kernel, linux-kernel, linux-mediatek, Alexandre Belloni
The IRQ is requested before the struct rtc is allocated and registered, but
this struct is used in the IRQ handler. This may lead to a NULL pointer
dereference.
Switch to devm_rtc_allocate_device/rtc_register_device to allocate the rtc
before requesting the IRQ.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-pl030.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/rtc/rtc-pl030.c b/drivers/rtc/rtc-pl030.c
index f85a1a93e669..343bb6ed1783 100644
--- a/drivers/rtc/rtc-pl030.c
+++ b/drivers/rtc/rtc-pl030.c
@@ -112,6 +112,13 @@ static int pl030_probe(struct amba_device *dev, const struct amba_id *id)
goto err_rtc;
}
+ rtc->rtc = devm_rtc_allocate_device(&dev->dev);
+ if (IS_ERR(rtc->rtc)) {
+ ret = PTR_ERR(rtc->rtc);
+ goto err_rtc;
+ }
+
+ rtc->rtc->ops = &pl030_ops;
rtc->base = ioremap(dev->res.start, resource_size(&dev->res));
if (!rtc->base) {
ret = -ENOMEM;
@@ -128,12 +135,9 @@ static int pl030_probe(struct amba_device *dev, const struct amba_id *id)
if (ret)
goto err_irq;
- rtc->rtc = rtc_device_register("pl030", &dev->dev, &pl030_ops,
- THIS_MODULE);
- if (IS_ERR(rtc->rtc)) {
- ret = PTR_ERR(rtc->rtc);
+ ret = rtc_register_device(rtc->rtc);
+ if (ret)
goto err_reg;
- }
return 0;
@@ -154,7 +158,6 @@ static int pl030_remove(struct amba_device *dev)
writel(0, rtc->base + RTC_CR);
free_irq(dev->irq[0], rtc);
- rtc_device_unregister(rtc->rtc);
iounmap(rtc->base);
amba_release_regions(dev);
--
2.19.0.rc2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] rtc: pl031: switch to devm_rtc_allocate_device/rtc_register_device
2018-09-09 20:38 [PATCH 1/3] rtc: mt6397: fix possible race condition Alexandre Belloni
2018-09-09 20:38 ` [PATCH 2/3] rtc: pl030: " Alexandre Belloni
@ 2018-09-09 20:38 ` Alexandre Belloni
2018-09-10 6:44 ` Linus Walleij
2018-09-11 11:32 ` [PATCH 1/3] rtc: mt6397: fix possible race condition Eddie Huang
2 siblings, 1 reply; 5+ messages in thread
From: Alexandre Belloni @ 2018-09-09 20:38 UTC (permalink / raw)
To: linux-rtc
Cc: linux-arm-kernel, linux-kernel, linux-mediatek,
Alexandre Belloni, Linus Walleij
Switch to devm_rtc_allocate_device to simplify the erro and driver removal
paths.
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-pl031.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index 82eb7da2c478..30943d200c5e 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -310,7 +310,6 @@ static int pl031_remove(struct amba_device *adev)
device_init_wakeup(&adev->dev, false);
if (adev->irq[0])
free_irq(adev->irq[0], ldata);
- rtc_device_unregister(ldata->rtc);
amba_release_regions(adev);
return 0;
@@ -383,24 +382,25 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
}
device_init_wakeup(&adev->dev, true);
- ldata->rtc = rtc_device_register("pl031", &adev->dev, ops,
- THIS_MODULE);
- if (IS_ERR(ldata->rtc)) {
- ret = PTR_ERR(ldata->rtc);
+ ldata->rtc = devm_rtc_allocate_device(&adev->dev);
+ if (IS_ERR(ldata->rtc))
+ return PTR_ERR(ldata->rtc);
+
+ ldata->rtc->ops = ops;
+
+ ret = rtc_register_device(ldata->rtc);
+ if (ret)
goto out;
- }
if (adev->irq[0]) {
ret = request_irq(adev->irq[0], pl031_interrupt,
vendor->irqflags, "rtc-pl031", ldata);
if (ret)
- goto out_no_irq;
+ goto out;
dev_pm_set_wake_irq(&adev->dev, adev->irq[0]);
}
return 0;
-out_no_irq:
- rtc_device_unregister(ldata->rtc);
out:
amba_release_regions(adev);
err_req:
--
2.19.0.rc2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/3] rtc: mt6397: fix possible race condition
2018-09-09 20:38 [PATCH 1/3] rtc: mt6397: fix possible race condition Alexandre Belloni
2018-09-09 20:38 ` [PATCH 2/3] rtc: pl030: " Alexandre Belloni
2018-09-09 20:38 ` [PATCH 3/3] rtc: pl031: switch to devm_rtc_allocate_device/rtc_register_device Alexandre Belloni
@ 2018-09-11 11:32 ` Eddie Huang
2 siblings, 0 replies; 5+ messages in thread
From: Eddie Huang @ 2018-09-11 11:32 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-rtc, linux-arm-kernel, linux-kernel, linux-mediatek, Sean Wang
On Sun, 2018-09-09 at 22:38 +0200, Alexandre Belloni wrote:
> The IRQ is requested before the struct rtc is allocated and registered, but
> this struct is used in the IRQ handler. This may lead to a NULL pointer
> dereference.
>
> Switch to devm_rtc_allocate_device/rtc_register_device to allocate the rtc
> before requesting the IRQ.
>
> Cc: Eddie Huang <eddie.huang@mediatek.com>
> Cc: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> drivers/rtc/rtc-mt6397.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index 385f8303bb41..e9a25ec4d434 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -332,6 +332,10 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, rtc);
>
> + rtc->rtc_dev = devm_rtc_allocate_device(rtc->dev);
> + if (IS_ERR(rtc->rtc_dev))
> + return PTR_ERR(rtc->rtc_dev);
> +
> ret = request_threaded_irq(rtc->irq, NULL,
> mtk_rtc_irq_handler_thread,
> IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
> @@ -344,11 +348,11 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>
> device_init_wakeup(&pdev->dev, 1);
>
> - rtc->rtc_dev = rtc_device_register("mt6397-rtc", &pdev->dev,
> - &mtk_rtc_ops, THIS_MODULE);
> - if (IS_ERR(rtc->rtc_dev)) {
> + rtc->rtc_dev->ops = &mtk_rtc_ops;
> +
> + ret = rtc_register_device(rtc->rtc_dev);
> + if (ret) {
> dev_err(&pdev->dev, "register rtc device failed\n");
> - ret = PTR_ERR(rtc->rtc_dev);
> goto out_free_irq;
> }
>
> @@ -365,7 +369,6 @@ static int mtk_rtc_remove(struct platform_device *pdev)
> {
> struct mt6397_rtc *rtc = platform_get_drvdata(pdev);
>
> - rtc_device_unregister(rtc->rtc_dev);
> free_irq(rtc->irq, rtc->rtc_dev);
> irq_dispose_mapping(rtc->irq);
>
Thanks
Acked-by: Eddie Huang <eddie.huang@mediatek.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-11 16:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-09 20:38 [PATCH 1/3] rtc: mt6397: fix possible race condition Alexandre Belloni
2018-09-09 20:38 ` [PATCH 2/3] rtc: pl030: " Alexandre Belloni
2018-09-09 20:38 ` [PATCH 3/3] rtc: pl031: switch to devm_rtc_allocate_device/rtc_register_device Alexandre Belloni
2018-09-10 6:44 ` Linus Walleij
2018-09-11 11:32 ` [PATCH 1/3] rtc: mt6397: fix possible race condition Eddie Huang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).