Re: [PATCH] rtc: spear: initialize IRQ state before requesting alarm IRQ

[sashiko-bot@kernel.org]

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