linux-kernel - Re: [PATCH] irqchip/sifive-plic: avoid interrupt ID 0 handling during suspend/resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87ikhc5hww.ffs@tglx>
Date: Sun, 21 Sep 2025 10:27:59 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Lucas Zampieri <lzampier@...hat.com>, linux-kernel@...r.kernel.org
Cc: Lucas Zampieri <lzampier@...hat.com>, Paul Walmsley
 <paul.walmsley@...ive.com>, Samuel Holland <samuel.holland@...ive.com>,
 stable@...r.kernel.org, linux-riscv@...ts.infradead.org, Jia Wang
 <wangjia@...rarisc.com>
Subject: Re: [PATCH] irqchip/sifive-plic: avoid interrupt ID 0 handling
 during suspend/resume

On Mon, Sep 15 2025 at 17:28, Lucas Zampieri wrote:

> To: linux-kernel@...r.kernel.org
> Cc: Paul Walmsley <paul.walmsley@...ive.com>
> Cc: Samuel Holland <samuel.holland@...ive.com>
> Cc: stable@...r.kernel.org
> Cc: linux-riscv@...ts.infradead.org
> Cc: Thomas Gleixner <tglx@...utronix.de>

How is this Cc list relevant in explaining the changes here?

> According to the PLIC specification[1], global interrupt sources are
> assigned small unsigned integer identifiers beginning at the value 1.
> An interrupt ID of 0 is reserved to mean "no interrupt".
>
> The current plic_irq_resume() and plic_irq_suspend() functions incorrectly
> starts the loop from index 0, which could access the reserved interrupt ID
> 0 register space.
> This fix changes the loop to start from index 1, skipping the reserved
> interrupt ID 0 as per the PLIC specification.

s/This fix changes/Change/

And please separate this from the explanation above.

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes

> This prevents potential undefined behavior when accessing the reserved
> register space during suspend/resume cycles.
>
> Fixes: e80f0b6a2cf3 ("irqchip/irq-sifive-plic: Add syscore callbacks for hibernation")
> Co-developed-by: Jia Wang <wangjia@...rarisc.com>
> Signed-off-by: Jia Wang <wangjia@...rarisc.com>
> Signed-off-by: Lucas Zampieri <lzampier@...hat.com>
>
> [1] https://github.com/riscv/riscv-plic-spec/releases/tag/1.0.0

Link: .....

This [1] stuff is just annoying.

> ---
>  drivers/irqchip/irq-sifive-plic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index bf69a4802b71..1c2b4d2575ac 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -252,7 +252,7 @@ static int plic_irq_suspend(void)
>  
>  	priv = per_cpu_ptr(&plic_handlers, smp_processor_id())->priv;
>  
> -	for (i = 0; i < priv->nr_irqs; i++) {
> +	for (i = 1; i < priv->nr_irqs; i++) {

This lacks a comment explaining this non-obvious 'i = 1'.

Thanks,

        tglx