Re: [PATCH v4 3/3] fpga: microchip-spi: fix OOB read in mpf_ops_parse_header()

[Xu Yilun]

On Tue, Apr 07, 2026 at 11:22:17AM -0600, Sebastian Alba Vives wrote:
> mpf_ops_parse_header() reads header_size at MPF_HEADER_SIZE_OFFSET (24)
> without first checking that count is large enough, leading to an
> out-of-bounds read if the buffer is smaller than 25 bytes.
> 
> Additionally, when header_size is zero, the expression
> *(buf + header_size - 1) reads one byte before the buffer start.
> Since a zero header_size cannot be resolved by providing a larger
> buffer, return -EINVAL instead of falling through.
> 
> Fixes: 5f8d4a9008307 ("fpga: microchip-spi: add Microchip MPF FPGA manager")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sebastian Alba Vives <sebasjosue84@xxxxxxxxx>
> ---
> Changes in v4:
>   - Reduce to two minimal fixes only: minimum count check before
>     reading header_size, and -EINVAL for zero header_size.
>     Drop redundant block loop checks — the pre-loop bounds extension
>     already ensures all block offsets are within count.
>     Suggested by Xu Yilun.
> Changes in v3:
>   - Add overflow check for 32-bit in component size loop.
> Changes in v2:
>   - Return -EINVAL for header_size == 0, -EAGAIN in block loop,
>     add count check before MPF_HEADER_SIZE_OFFSET read.
> ---
>  drivers/fpga/microchip-spi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
> index 6134cea..dca1a5d 100644
> --- a/drivers/fpga/microchip-spi.c
> +++ b/drivers/fpga/microchip-spi.c
> @@ -115,7 +115,13 @@ static int mpf_ops_parse_header(struct fpga_manager *mgr,
>  		return -EINVAL;
>  	}
>  
> +	if (count < MPF_HEADER_SIZE_OFFSET + 1)
> +		return -EINVAL;

Why do you think the fpga-mgr core would provide a buffer that is not
good for low-lever driver to start with? There should be contract
between the core and the driver, if you've found anyone breaks the
contract, fix the breaker, not to add defensive code here.

Thanks,
Yilun

> +
>  	header_size = *(buf + MPF_HEADER_SIZE_OFFSET);
> +	if (!header_size)
> +		return -EINVAL;
> +
>  	if (header_size > count) {
>  		info->header_size = header_size;
>  		return -EAGAIN;
> -- 
> 2.43.0
> 
>