All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rtc: spear: initialize IRQ state before requesting alarm IRQ
@ 2026-09-02  8:03 Runyu Xiao
  2026-09-02  8:15 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Runyu Xiao @ 2026-09-02  8:03 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, stable, Runyu Xiao, Jianhao Xu

devm_request_irq() enables the interrupt before it returns, so the handler
may run while probe is still initializing the device. Initialize the MMIO
address and spinlock before requesting the alarm IRQ.

Fixes: 0942a71e435f ("rtc: add support for spear rtc")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/rtc/rtc-spear.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-spear.c b/drivers/rtc/rtc-spear.c
index 959acff8f..d12bccb3c 100644
--- a/drivers/rtc/rtc-spear.c
+++ b/drivers/rtc/rtc-spear.c
@@ -363,6 +363,12 @@ static int spear_rtc_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
+	config->ioaddr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(config->ioaddr))
+		return PTR_ERR(config->ioaddr);
+
+	spin_lock_init(&config->lock);
+
 	status = devm_request_irq(&pdev->dev, irq, spear_rtc_irq, 0, pdev->name,
 			config);
 	if (status) {
@@ -371,10 +377,6 @@ static int spear_rtc_probe(struct platform_device *pdev)
 		return status;
 	}
 
-	config->ioaddr = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(config->ioaddr))
-		return PTR_ERR(config->ioaddr);
-
 	config->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(config->clk))
 		return PTR_ERR(config->clk);
@@ -383,7 +385,6 @@ static int spear_rtc_probe(struct platform_device *pdev)
 	if (status < 0)
 		return status;
 
-	spin_lock_init(&config->lock);
 	platform_set_drvdata(pdev, config);
 
 	config->rtc->ops = &spear_rtc_ops;
-- 
2.34.1

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

* Re: [PATCH] rtc: spear: initialize IRQ state before requesting alarm IRQ
  2026-09-02  8:03 [PATCH] rtc: spear: initialize IRQ state before requesting alarm IRQ Runyu Xiao
@ 2026-09-02  8:15 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-02  8:15 UTC (permalink / raw)
  To: Runyu Xiao; +Cc: Alexandre Belloni, linux-rtc

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware Access Before Initialization / Missing Clock / System Crash
- [High] Logic Error / Potential Denial of Service
- [High] Data Corruption in RTC Year
--

commit 4751499f6595bf942675b476a237ee52aa8c59e1
Author: Runyu Xiao <runyu.xiao@seu.edu.cn>

rtc: spear: initialize IRQ state before requesting alarm IRQ

devm_request_irq() enables the interrupt before it returns, so the handler
may run while probe is still initializing the device. Initialize the MMIO
address and spinlock before requesting the alarm IRQ.

Cc: stable@vger.kernel.org

> diff --git a/drivers/rtc/rtc-spear.c b/drivers/rtc/rtc-spear.c
> index 959acff8faff0..d12bccb3cf5ec 100644
> --- a/drivers/rtc/rtc-spear.c
> +++ b/drivers/rtc/rtc-spear.c
> @@ -363,6 +363,12 @@ static int spear_rtc_probe(struct platform_device *pdev)
[ ... ]
> +	config->ioaddr = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(config->ioaddr))
> +		return PTR_ERR(config->ioaddr);
> +
> +	spin_lock_init(&config->lock);
> +
>  	status = devm_request_irq(&pdev->dev, irq, spear_rtc_irq, 0, pdev->name,
>  			config);
>  	if (status) {

[Severity: High]
This is a pre-existing issue, but by requesting the IRQ here before the clock
is prepared and enabled later in spear_rtc_probe(), is there a risk of a
system crash?

If a pending or spurious interrupt fires immediately after devm_request_irq()
returns, spear_rtc_irq() will execute and perform:

    irq_data = readl(config->ioaddr + STATUS_REG);

Since clk_prepare_enable() has not been called yet, the clock is disabled.
Accessing an unclocked peripheral register typically causes a synchronous
external abort.

Additionally, clk_disable_unprepare() is called manually on probe failure or
during spear_rtc_remove() before devres cleans up the IRQ and unregisters
the device. Could this create a similar window where an interrupt fires or a
userspace ioctl triggers a clock-dependent MMIO read on a gated clock?

> @@ -371,10 +377,6 @@ static int spear_rtc_probe(struct platform_device *pdev)
>  		return status;
>  	}
>  
> -	config->ioaddr = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(config->ioaddr))
> -		return PTR_ERR(config->ioaddr);
> -
>  	config->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(config->clk))
>  		return PTR_ERR(config->clk);
> @@ -383,7 +385,6 @@ static int spear_rtc_probe(struct platform_device *pdev)
>  	if (status < 0)
>  		return status;

While reviewing the RTC implementation, I noticed a couple of other
pre-existing issues that might be worth addressing.

In drivers/rtc/rtc-spear.c:spear_rtc_read_time():

    do {
        time = readl(config->ioaddr + TIME_REG);
        days = readl(config->ioaddr + DATE_REG);
    } while (time == readl(config->ioaddr + TIME_REG));

[Severity: High]
This is a pre-existing issue, but does this inverted condition cause a 
busy-wait loop?

Because the TIME_REG only updates once per second, the loop will spin
continuously as long as the time has not changed, occupying a CPU core
entirely for up to 1 second. Unprivileged local users repeatedly reading
the RTC time could potentially cause a denial of service. Should this be a
!= condition to only catch rollovers?

Also in drivers/rtc/rtc-spear.c:tm2bcd():

    tm->tm_year = bin2bcd(tm->tm_year);

[Severity: High]
This is a pre-existing issue, but does this corrupt the year value?

The driver advertises support for years up to 9999 in spear_rtc_probe():

    config->rtc->range_max = RTC_TIMESTAMP_END_9999;

However, tm_year is the number of years since 1900. If userspace sets the 
time to a year like 2026, tm_year is 126. Passing 126 to bin2bcd() produces 
an invalid BCD value (0xC6), which is written directly into the lower 8 bits 
of the 16-bit hardware year field. 

Furthermore, because bin2bcd() returns an 8-bit u8, years >= 2156 
(tm_year >= 256) will be silently truncated to 0. When reading back the
value in bcd2tm(), the year will be misinterpreted.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902080318.3498434-1-runyu.xiao@seu.edu.cn?part=1

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

end of thread, other threads:[~2026-09-02  8:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02  8:03 [PATCH] rtc: spear: initialize IRQ state before requesting alarm IRQ Runyu Xiao
2026-09-02  8:15 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.