Re: [PATCH] mtd: rawnand: r852: prevent card work from being requeued during shutdown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] mtd: rawnand: r852: unprotected register access in
  r852_update_card_detect() [1]
--

commit 9d313af3cdbf139593a845ec29f559aa0df5bf6b
Author: James Hilliard <james.hilliard1@gmail.com>

mtd: rawnand: r852: prevent card work from being requeued during shutdown

This commit replaces cancel_delayed_work_sync() with
disable_delayed_work_sync() in r852_shutdown() so card_detect_work cannot
be requeued before interrupts are disabled.

> diff --git a/drivers/mtd/nand/raw/r852.c b/drivers/mtd/nand/raw/r852.c
> index 92c47d351c278..bf1c1be7f2aa7 100644
> --- a/drivers/mtd/nand/raw/r852.c
> +++ b/drivers/mtd/nand/raw/r852.c
> @@ -1007,7 +1007,7 @@ static void r852_shutdown(struct pci_dev *pci_dev)
>  {
>  	struct r852_device *dev = pci_get_drvdata(pci_dev);
>  
> -	cancel_delayed_work_sync(&dev->card_detect_work);
> +	disable_delayed_work_sync(&dev->card_detect_work);
>  	r852_disable_irqs(dev);

[Severity: High]
This problem wasn't introduced by this patch, but can
r852_update_card_detect() [1] race with r852_irq() on
R852_CARD_IRQ_ENABLE and unmask disabled interrupts?

drivers/mtd/nand/raw/r852.c:r852_update_card_detect() {
	int card_detect_reg = r852_read_reg(dev, R852_CARD_IRQ_ENABLE);
	...
	r852_write_reg(dev, R852_CARD_IRQ_ENABLE, card_detect_reg);
}

card_detect_work calls r852_update_card_detect(), which performs this
read-modify-write without holding dev->irqlock:

CPU0 (kworker)                    CPU1 (<hardirq>)
r852_card_detect_work()
  r852_update_card_detect()
    reg = r852_read_reg(...)
                                  r852_irq()
                                    spin_lock_irqsave(&dev->irqlock)
                                    r852_disable_irqs()
                                      r852_write_reg(...)
                                    spin_unlock_irqrestore(&dev->irqlock)
    r852_write_reg(..., reg)  <-- overwrites masked IRQs

When the worker writes back its stale value, it re-enables interrupts
that r852_irq() just masked, which can trigger an IRQ storm.

[1] https://sashiko.dev/#/bug/linux-c0fe7604-3210-4aa5-9a00-b60066cbe1db

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108-submit-r852-shutdown-work-v1-v1-1-8e448ad7c54c@gmail.com?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/