mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] spi: ljca: validate the response length before copying the data
@ 2026-09-29  8:01 Weibin Liu
  2026-09-29 11:00 ` Sakari Ailus
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Weibin Liu @ 2026-09-29  8:01 UTC (permalink / raw)
  To: broonie
  Cc: wentong.wu, lixu.zhang, sakari.ailus, linux-spi, linux-kernel, stable

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: ljca: validate the response length before copying the data
  2026-09-29  8:01 [PATCH] spi: ljca: validate the response length before copying the data Weibin Liu
@ 2026-09-29 11:00 ` Sakari Ailus
  2026-09-29 11:02 ` Mark Brown
  2026-10-13  3:17 ` [PATCH v2] " Weibin Liu
  2 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2026-09-29 11:00 UTC (permalink / raw)
  To: Weibin Liu
  Cc: broonie, wentong.wu, lixu.zhang, linux-spi, linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: ljca: validate the response length before copying the data
  2026-09-29  8:01 [PATCH] spi: ljca: validate the response length before copying the data Weibin Liu
  2026-09-29 11:00 ` Sakari Ailus
@ 2026-09-29 11:02 ` Mark Brown
  2026-10-13  3:17 ` [PATCH v2] " Weibin Liu
  2 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2026-09-29 11:02 UTC (permalink / raw)
  To: Weibin Liu
  Cc: wentong.wu, lixu.zhang, sakari.ailus, linux-spi, linux-kernel, stable

[-- 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 --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] spi: ljca: validate the response length before copying the data
  2026-09-29  8:01 [PATCH] spi: ljca: validate the response length before copying the data Weibin Liu
  2026-09-29 11:00 ` Sakari Ailus
  2026-09-29 11:02 ` Mark Brown
@ 2026-10-13  3:17 ` Weibin Liu
  2026-10-13 19:46   ` Mark Brown
  2026-11-03 13:36   ` Arpith Alexander
  2 siblings, 2 replies; 6+ messages in thread
From: Weibin Liu @ 2026-10-13  3:17 UTC (permalink / raw)
  To: broonie
  Cc: wentong.wu, lixu.zhang, sakari.ailus, linux-spi, linux-kernel, stable

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.

Check that the response carries the full header before reading
r_packet->len, then reject responses whose payload is not contained in
the data received or is longer than the requested transfer.

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>
---
Changes in v2:

- Check for the full header before reading r_packet->len (Mark Brown).
- Regenerate on a clean tree; v1 carried prerequisite-patch-id entries
  for unrelated patches.
---
 drivers/spi/spi-ljca.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-ljca.c b/drivers/spi/spi-ljca.c
index 0c6e6248d..8fd001610 100644
--- a/drivers/spi/spi-ljca.c
+++ b/drivers/spi/spi-ljca.c
@@ -105,7 +105,12 @@ 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)
+
+	if (ret < sizeof(*r_packet))
+		return -EIO;
+
+	if (!r_packet->len || ret < sizeof(*r_packet) + r_packet->len ||
+	    r_packet->len > len)
 		return -EIO;
 
 	if (r_data)

base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
2.50.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] spi: ljca: validate the response length before copying the data
  2026-10-13  3:17 ` [PATCH v2] " Weibin Liu
@ 2026-10-13 19:46   ` Mark Brown
  2026-11-03 13:36   ` Arpith Alexander
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2026-10-13 19:46 UTC (permalink / raw)
  To: Weibin Liu
  Cc: wentong.wu, lixu.zhang, sakari.ailus, linux-spi, linux-kernel, stable

On Tue, 13 Oct 2026 11:17:17 +0800, Weibin Liu wrote:
> spi: ljca: validate the response length before copying the data

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.4

Thanks!

[1/1] spi: ljca: validate the response length before copying the data
      https://git.kernel.org/broonie/spi/c/0fe4573c822c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] spi: ljca: validate the response length before copying the data
  2026-10-13  3:17 ` [PATCH v2] " Weibin Liu
  2026-10-13 19:46   ` Mark Brown
@ 2026-11-03 13:36   ` Arpith Alexander
  1 sibling, 0 replies; 6+ messages in thread
From: Arpith Alexander @ 2026-11-03 13:36 UTC (permalink / raw)
  To: liuwb
  Cc: broonie, linux-kernel, linux-spi, lixu.zhang, sakari.ailus,
	stable, wentong.wu

On Tue, 13 Oct 2026 11:17:17 +0800, Weibin Liu wrote:
> Check that the response carries the full header before reading
> r_packet->len, then reject responses whose payload is not contained in
> the data received or is longer than the requested transfer.

This breaks SPI writes on a Dell XPS 13 9340: the LJCA acks a write with
only the packet header (len = bytes written, no payload), so every write
now fails with -EIO and the IVSC behind it no longer initialises.

Fix posted here:
https://lore.kernel.org/linux-spi/20261103131435.21838-1-arpith99@gmail.com/

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-03 13:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29  8:01 [PATCH] spi: ljca: validate the response length before copying the data Weibin Liu
2026-09-29 11:00 ` Sakari Ailus
2026-09-29 11:02 ` Mark Brown
2026-10-13  3:17 ` [PATCH v2] " Weibin Liu
2026-10-13 19:46   ` Mark Brown
2026-11-03 13:36   ` Arpith Alexander

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®