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

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

[Weibin Liu]

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.

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
-- 
2.50.1

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

[Sakari Ailus]

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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 583 bytes --]

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.

> base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
> prerequisite-patch-id: e48582f6ffe124b3806593af6e22b74682c8a83f
> prerequisite-patch-id: 369f74b9a7ecde56141b13ec671f9200345a3bab
> prerequisite-patch-id: cbf6f60473c80add5b2cddf22d142f35a4e3c834

Why would a change like this depend on out of tree code?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]