Re: [PATCH v8 2/6] spmi: apple: Validate FIFO state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsafe Data / Silenced Error: The driver returns success (0) when extra data is left in the RX FIFO, feeding potentially corrupted or stale data to the client.
--

commit bd81188aa73e785d093e758b80b16d3e34cda43c
Author: Alba Mendez <me@alba.sh>

spmi: apple: Validate FIFO state
    
Check for data before reading the body of a reply, and check for
end of data afterwards.

> diff --git a/drivers/spmi/spmi-apple-controller.c b/drivers/spmi/spmi-apple-controller.c
> index 376cf682c43e0..15721cb41d5c8 100644
> --- a/drivers/spmi/spmi-apple-controller.c
> +++ b/drivers/spmi/spmi-apple-controller.c

[ ... ]

> @@ -86,6 +101,11 @@ static int spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  		}
>  	}
>  
> +	if (!(readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY)) {
> +		dev_warn(&ctrl->dev, "FIFO has extra data\n");
> +		spmi->prev_fail = true;
> +	}
> +
>  	return 0;
>  }

[Severity: High]
When extra data is detected in the RX FIFO, spmi_read_cmd() logs a warning
and marks spmi->prev_fail = true, but continues to return 0 (success).

Does returning success here risk feeding corrupted or out-of-sync data (such
as stale data left by the bootloader) back to the caller instead of returning
an error code like -EIO?

[ ... ]

> @@ -115,6 +140,11 @@ static int spmi_write_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  	/* Discard */
>  	readl(spmi->regs + SPMI_RSP_REG);
>  
> +	if (!(readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY)) {
> +		dev_warn(&ctrl->dev, "FIFO has extra data\n");
> +		spmi->prev_fail = true;
> +	}
> +
>  	return 0;
>  }

[Severity: High]
Similarly to spmi_read_cmd(), spmi_write_cmd() detects that the FIFO is out
of sync but returns 0.

Should this also return an error code like -EIO to properly signal the
synchronization failure to the caller?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-t603x-spmi-v8-0-708d4d12e0fd@chaosmail.tech?part=2