Skip to content

usb_uram: fix SPI array out-of-bounds indexing#110

Open
cvalentine99 wants to merge 1 commit intowavelet-lab:mainfrom
cvalentine99:fix/usb-spi-oob-indexing
Open

usb_uram: fix SPI array out-of-bounds indexing#110
cvalentine99 wants to merge 1 commit intowavelet-lab:mainfrom
cvalentine99:fix/usb-spi-oob-indexing

Conversation

@cvalentine99
Copy link
Copy Markdown

Summary

In usb_uram_generic.c, spi_core[] (8 elements) and spi_int_number[]
are indexed with raw ls_op_addr instead of SPIEXT_LSOP_GET_BUS(ls_op_addr).
Since ls_op_addr encodes (cfg << 16) | (eaddr << 8) | bus, SPI transactions
with non-zero config or extended address fields can index these arrays out of
bounds.

The PCIe implementation (pcie_uram_main.c) correctly uses the extraction
macro throughout; this patch aligns the USB path. When ls_op_addr encodes
only a bus value in the low bits, behavior is unchanged; this patch only
corrects array indexing for packed addresses with non-zero higher fields.

Note: in usb_uram_read_wait, the reg parameter is UNUSED in the
current usb_read_bus implementation. I kept the existing spi_core field
rather than changing to spi_base to keep the diff minimal, but this may
warrant a follow-up if io_read_bus_fn implementations that use reg are
added later.

Validation

  • Verified the USB path now uses SPIEXT_LSOP_GET_BUS(ls_op_addr) consistently for all SPI bus-indexed arrays
  • Confirmed this matches the PCIe transport's indexing pattern in pcie_uram_main.c
  • Confirmed SPIEXT_LSOP_GET_BUS() result is already bounds-checked at line 166
  • Build verification: clean build with gcc 15.2.0, zero warnings

Hardware follow-up

  • If USB hardware is available, exercise SPI read/write transactions on a device using the USB transport path (e.g., LMS7002M register access)

Use SPIEXT_LSOP_GET_BUS() to extract bus index before accessing
spi_core[] and spi_int_number[], matching the PCIe implementation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sergforce
Copy link
Copy Markdown
Contributor

Alter it for feature_pe_sync branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants