Re: [PATCH] spi: ljca: validate the response length before copying the data

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Weibin,

On Tue, Sep 29, 2026 at 04:01:26PM +0800, Weibin Liu wrote:
> ljca_spi_read_write() only checks that the response from the adapter
> carries the packet header and a non-zero payload length before copying
> r_packet->len bytes from the response into the transfer buffer.

I agree the check is incomplete, but you need to still perform the check
for the header before accessing it.

> 
> The payload length is taken from the packet the device sent back and is
> neither bounded by the number of bytes actually received nor by the
> size of the requested transfer: a misbehaving adapter can announce a
> payload larger than what it actually sent, and the memcpy then reads
> past the end of the 60-byte input buffer and writes past the end of the
> caller's transfer buffer.
> 
> Reject responses whose announced payload is not fully contained in the
> received data or exceeds the requested transfer length.
> 
> Fixes: caee8e38da67 ("spi: Add support for Intel LJCA USB SPI driver")
> Cc: stable@vger.kernel.org # 6.8+
> Signed-off-by: Weibin Liu <liuwb@xiaopeng.com>
> ---
> Reviewer notes:
> 
> - r_packet->len is a device-controlled field; the old code only
>   required it to be non-zero before memcpy()ing that many bytes into
>   the caller's buffer. The receive buffer is LJCA_SPI_BUF_SIZE (60)
>   bytes, so an announced payload larger than what the adapter actually
>   sent already reads past the end of the input buffer, independently
>   of the transfer length.
> - The new check bounds the payload by both the number of bytes
>   actually received (ret) and the requested transfer length (len), so
>   the copy stays inside both buffers.
> 
> Tested on x86_64: with this patch applied the driver builds, loads and
> unloads cleanly; no LJCA adapter is available to exercise the SPI
> transfer path on hardware.
> 
>  drivers/spi/spi-ljca.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-ljca.c b/drivers/spi/spi-ljca.c
> index 0c6e6248d..bd07e5df9 100644
> --- a/drivers/spi/spi-ljca.c
> +++ b/drivers/spi/spi-ljca.c
> @@ -105,7 +105,8 @@ static int ljca_spi_read_write(struct ljca_spi_dev *ljca_spi, const u8 *w_data,
>  			    (u8 *)r_packet, LJCA_SPI_BUF_SIZE);
>  	if (ret < 0)
>  		return ret;
> -	else if (ret < sizeof(*r_packet) || r_packet->len <= 0)
> +	else if (ret < sizeof(*r_packet) + r_packet->len ||
> +		 r_packet->len <= 0 || r_packet->len > len)
>  		return -EIO;
>  
>  	if (r_data)
> 
> base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
> prerequisite-patch-id: e48582f6ffe124b3806593af6e22b74682c8a83f
> prerequisite-patch-id: 369f74b9a7ecde56141b13ec671f9200345a3bab
> prerequisite-patch-id: cbf6f60473c80add5b2cddf22d142f35a4e3c834

-- 
Regards,

Sakari Ailus