Skip to content

Commit 11acd4a

Browse files
authored
[posix] fix SPI platform driver sanity check boundaries (openthread#13236)
This commit corrects the SPI frame sanity checks in the POSIX platform driver (`spi_interface.cpp`). Previously, the sanity checks compared `mSpiSlaveDataLen` and `slaveAcceptLen` against `kMaxFrameSize` (8192). However, `mSpiSlaveDataLen` is the payload size, which excludes the 5-byte SPI frame header. If the slave advertised a data length of exactly `kMaxFrameSize` (8192), it would pass the sanity check, but the subsequent `DoSpiTransfer` would request a transfer length of `kMaxFrameSize + kSpiFrameHeaderSize + alignment` (e.g. 8213 bytes). This would cause an out-of-bounds read on `mSpiTxFrameBuffer` which is sized `kMaxFrameSize + kSpiAlignAllowanceMax` (8208 bytes). This commit updates the sanity checks to use `kMaxFrameSize - kSpiFrameHeaderSize` as the maximum allowed payload length, ensuring that worst-case transfers always fit within the tx buffer allocation.
1 parent 1f8d922 commit 11acd4a

1 file changed

Lines changed: 5 additions & 3 deletions

File tree

src/posix/platform/spi_interface.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,9 @@ otError SpiInterface::PushPullSpi(void)
412412
txFrame.SetHeaderAcceptLen(0);
413413
txFrame.SetHeaderDataLen(0);
414414

415-
// Sanity check.
416-
if (mSpiSlaveDataLen > kMaxFrameSize)
415+
// Sanity check. The header `data_len` carries the payload length only
416+
// (it excludes header size), so the largest valid value is the MTU.
417+
if (mSpiSlaveDataLen > kMaxFrameSize - kSpiFrameHeaderSize)
417418
{
418419
mSpiSlaveDataLen = 0;
419420
}
@@ -517,7 +518,8 @@ otError SpiInterface::PushPullSpi(void)
517518
slaveAcceptLen = rxFrame.GetHeaderAcceptLen();
518519
mSpiSlaveDataLen = rxFrame.GetHeaderDataLen();
519520

520-
if (!rxFrame.IsValid() || (slaveAcceptLen > kMaxFrameSize) || (mSpiSlaveDataLen > kMaxFrameSize))
521+
if (!rxFrame.IsValid() || (slaveAcceptLen > kMaxFrameSize - kSpiFrameHeaderSize) ||
522+
(mSpiSlaveDataLen > kMaxFrameSize - kSpiFrameHeaderSize))
521523
{
522524
mInterfaceMetrics.mTransferredGarbageFrameCount++;
523525
mSpiTxRefusedCount++;

0 commit comments

Comments
 (0)