Re: [PATCH v3 13/15] mtd: rawnand: sunxi: avoid duplicate chip setup before page commands

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Wait queue spinlock corruption and false completion due to concurrent complete() and init_completion() after a timeout.
--

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

mtd: rawnand: sunxi: avoid duplicate chip setup before page commands

The page helpers invoke exec_op(), which selects the chip and programs
its timing and spare-area registers. Remove the identical setup directly
before those helpers in the read callbacks and PIO write callbacks.

> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index dc5b3814657e7..0d829103b144b 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -1743,8 +1743,6 @@ static int sunxi_nfc_hw_ecc_read_page(struct nand_chip *nand, uint8_t *buf,
>  	int ret, i, cur_off = 0;
>  	bool erased_chunk_found = false;
>  
> -	sunxi_nfc_select_chip(nand, nand->cur_cs);
> -
>  	ret = nand_read_page_op(nand, page, 0, NULL, 0);
>  	if (ret)
>  		return ret;

[ ... ]

While reviewing this code, I noticed a separate issue in sunxi_nfc_wait_events():

drivers/mtd/nand/raw/sunxi_nand.c:sunxi_nfc_wait_events() {
    ...
	if (!use_polling) {
		init_completion(&nfc->complete);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but is a pre-existing issue. Can
this lead to wait queue spinlock corruption and false completions?

If a NAND operation times out in sunxi_nfc_wait_events(), CPU A clears the
interrupt enable register and returns -ETIMEDOUT.

Concurrently, if the IRQ handler is delayed, CPU B executing
sunxi_nfc_interrupt() may read the active status before CPU A clears it, but
get preempted before calling complete().

CPU A then starts a new NAND operation and re-enters sunxi_nfc_wait_events():

drivers/mtd/nand/raw/sunxi_nand.c:sunxi_nfc_wait_events() {
    ...
	if (!use_polling) {
		init_completion(&nfc->complete);
    ...
}

When CPU B resumes and calls complete(), wouldn't it access and modify the
lock concurrently, causing lock corruption?

Furthermore, wouldn't it set the completion flag for the new command, causing
it to return prematurely and process garbage data?

Since there is no structural prevention like synchronize_irq() or
disable_irq() on the timeout path, does this need to be addressed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-submit-sunxi-nand-vendor-oob-layout-v1-v3-0-838cb0ba1547@gmail.com?part=13