Skip to content

Commit 1f8d922

Browse files
authored
[spi-hdlc-adapter] validate RCP frame lengths against the MTU (openthread#13206)
The slave (RCP) controls the `data_len` and `accept_len` fields of the SPI header. Both carry a payload length that excludes the 5-byte header, as shown by `sMTU = MAX_FRAME_SIZE - HEADER_LEN`, so the largest valid value is the MTU, `MAX_FRAME_SIZE - HEADER_LEN`. The two sanity checks in `push_pull_spi()` only rejected values greater than `MAX_FRAME_SIZE`, allowing an RCP to advertise a length in the range (MAX_FRAME_SIZE - HEADER_LEN, MAX_FRAME_SIZE]. That value flows into `spi_xfer_bytes`, and `do_spi_xfer()` then transfers `spi_xfer_bytes + HEADER_LEN + sSpiRxAlignAllowance` bytes into `sSpiRxFrameBuffer` / `sSpiTxFrameBuffer`. Those buffers are sized `MAX_FRAME_SIZE + SPI_RX_ALIGN_ALLOWANCE_MAX`, so the extra HEADER_LEN added by the transfer can write up to 5 bytes past the end of both. Clamp both checks to `MAX_FRAME_SIZE - HEADER_LEN` so the advertised payload plus the header always fits within the existing buffers.
1 parent 5e68d00 commit 1f8d922

1 file changed

Lines changed: 5 additions & 4 deletions

File tree

tools/spi-hdlc-adapter/spi-hdlc-adapter.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,9 @@ static int push_pull_spi(void)
531531
spi_header_set_accept_len(sSpiTxFrameBuffer, 0);
532532
spi_header_set_data_len(sSpiTxFrameBuffer, 0);
533533

534-
// Sanity check.
535-
if (slave_data_len > MAX_FRAME_SIZE)
534+
// Sanity check. The header `data_len` carries the payload length only
535+
// (it excludes HEADER_LEN), so the largest valid value is the MTU.
536+
if (slave_data_len > MAX_FRAME_SIZE - HEADER_LEN)
536537
{
537538
slave_data_len = 0;
538539
}
@@ -630,8 +631,8 @@ static int push_pull_spi(void)
630631
slave_max_rx = spi_header_get_accept_len(spiRxFrameBuffer);
631632
slave_data_len = spi_header_get_data_len(spiRxFrameBuffer);
632633

633-
if (((slave_header & SPI_HEADER_PATTERN_MASK) != SPI_HEADER_PATTERN_VALUE) || (slave_max_rx > MAX_FRAME_SIZE) ||
634-
(slave_data_len > MAX_FRAME_SIZE))
634+
if (((slave_header & SPI_HEADER_PATTERN_MASK) != SPI_HEADER_PATTERN_VALUE) ||
635+
(slave_max_rx > MAX_FRAME_SIZE - HEADER_LEN) || (slave_data_len > MAX_FRAME_SIZE - HEADER_LEN))
635636
{
636637
sSpiGarbageFrameCount++;
637638
sSpiTxRefusedCount++;

0 commit comments

Comments
 (0)