[PATCH v3 0/2] mfd/spi: sprd: fix ADI transfer length handling

[PATCH v3 0/2] mfd/spi: sprd: fix ADI transfer length handling

[Weibin Liu]

Thanks to Sashiko for reviewing v2.

v3 only rate-limits the two new invalid-length error messages in
patch 2. The exact transfer length checks and the SC27xx caller fix
are unchanged.

The separate, pre-existing issue about writing back an uninitialized
value after a failed ADI read is intentionally not included here. It
is independent of the transfer length fix and can be handled in a
separate patch.

The series was rebuilt with W=1 and retested in QEMU (aarch64 virt).
The length matrix and SC27xx software path remain unchanged. A burst
of 32 invalid reads and 32 invalid writes emitted only 10 messages at
each new driver call site. QEMU has no model of this hardware, and no
Spreadtrum hardware was available.

v2: https://lore.kernel.org/r/20261015-spi-sprd-adi-exactlen-v2-0-13203b6aef7e@xiaopeng.com

Changes in v3:
- Rate-limit invalid-length errors as suggested by Sashiko's review.

Signed-off-by: Weibin Liu <liuwb@xiaopeng.com>
---
 drivers/mfd/sprd-sc27xx-spi.c |  2 +-
 drivers/spi/spi-sprd-adi.c    | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

[PATCH v3 1/2] mfd: sprd-sc27xx: Pass byte length to spi_read()

[Weibin Liu]

sprd_pmic_spi_read() passes 1 as the transfer length to spi_read(),
but the length parameter is a byte count, and the ADI controller
consumes a u32 register address from the buffer. The ADI driver
ignores the transfer length and always operates on the first u32,
so this happens to work with the 8-byte stack buffer, but the
advertised length does not describe what the controller actually
accesses.

Pass sizeof(u32) so the length matches the bytes touched by the
controller. The companion patch to the ADI driver rejects lengths
that do not exactly match the ADI register access size.

Tested in QEMU (aarch64 virt) together with the companion patch:
the SC27xx driver's register transfers still pass the new length
checks once it advertises the byte length.

Fixes: 25ca4ae43466 ("mfd: Add Spreadtrum SC27xx series PMICs driver")
Cc: stable@vger.kernel.org # 4.15+
Signed-off-by: Weibin Liu <liuwb@xiaopeng.com>
Reviewed-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/mfd/sprd-sc27xx-spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
index 9a8e6add8fca..8759cee052b0 100644
--- a/drivers/mfd/sprd-sc27xx-spi.c
+++ b/drivers/mfd/sprd-sc27xx-spi.c
@@ -156,7 +156,7 @@ static int sprd_pmic_spi_read(void *context,
 
 	/* Copy address to read from into first element of SPI buffer. */
 	memcpy(rx_buf, reg, sizeof(u32));
-	ret = spi_read(spi, rx_buf, 1);
+	ret = spi_read(spi, rx_buf, sizeof(u32));
 	if (ret < 0)
 		return ret;
 
-- 
2.50.1

[PATCH v3 2/2] spi: sprd-adi: enforce the exact transfer length

[Weibin Liu]

sprd_adi_transfer_one() never looks at the transfer length: the
read path dereferences t->rx_buf as one u32 and stores the result
back, and the write path reads two consecutive u32 values from
t->tx_buf. A transfer whose length does not exactly match what the
controller touches is a bug in the caller: a short transfer reads
or writes past the end of the buffer, and a long transfer is
silently truncated.

The SPI core does not enforce the expected length, so reject reads
whose length is not exactly sizeof(u32) and writes whose length is
not exactly 2 * sizeof(u32) with -EINVAL before touching the
buffers. If the controller is exposed through spidev, a userspace
client can submit invalid transfers in a tight loop, so use
dev_err_ratelimited() for the driver's rejection messages.

The only in-tree user, the SC27xx PMIC driver, is updated by the
previous patch to advertise the byte length of an ADI register
access.

Tested in QEMU (aarch64 virt) against Linux 7.3-rc7, with the
controller node added to the guest device tree. QEMU has no model
of this hardware (the MMIO window is backed by RAM), so this only
exercises the software path. Transfers submitted through spidev
confirm the new checks: a 4-byte read and an 8-byte write are
accepted, every other length from 1 to 16 is rejected with
-EINVAL. A burst of 32 invalid reads and 32 invalid writes emits
only 10 messages at each new driver call site. The SC27xx driver's
transfers pass once it advertises the byte length. No Spreadtrum
hardware was available.

Fixes: 7e2903cb91df ("spi: Add ADI driver for Spreadtrum platform")
Cc: stable@vger.kernel.org # 4.15+
Signed-off-by: Weibin Liu <liuwb@xiaopeng.com>
Reviewed-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
Changes in v3:
- Rate-limit invalid-length errors as suggested by Sashiko's review.

 drivers/spi/spi-sprd-adi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index a6baaf02b3c8..32e445cea0d3 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -339,10 +339,24 @@ static int sprd_adi_transfer_one(struct spi_controller *ctlr,
 	int ret;
 
 	if (t->rx_buf) {
+		/* ADI reads consume exactly one u32: reg in, value out. */
+		if (t->len != sizeof(u32)) {
+			dev_err_ratelimited(sadi->dev,
+					    "invalid read length %u\n",
+					    t->len);
+			return -EINVAL;
+		}
 		reg = *(u32 *)t->rx_buf;
 		ret = sprd_adi_read(sadi, reg, &val);
 		*(u32 *)t->rx_buf = val;
 	} else if (t->tx_buf) {
+		/* ADI writes consume exactly two u32: reg and value. */
+		if (t->len != 2 * sizeof(u32)) {
+			dev_err_ratelimited(sadi->dev,
+					    "invalid write length %u\n",
+					    t->len);
+			return -EINVAL;
+		}
 		u32 *p = (u32 *)t->tx_buf;
 		reg = *p++;
 		val = *p;
-- 
2.50.1