Skip to content

lib: utils/mailbox: clamp RPMI shmem queue name to RPMI_NAME_CHARS_MAX - #423

Open
Yudis-bit wants to merge 1 commit into
riscv-software-src:masterfrom
Yudis-bit:fix/rpmi-shmem-name-bounds
Open

Yudis-bit wants to merge 1 commit into
riscv-software-src:masterfrom
Yudis-bit:fix/rpmi-shmem-name-bounds

Conversation

@Yudis-bit

Copy link
Copy Markdown
Contributor

Problem

rpmi_shmem_transport_init() copies a device-tree reg-names string into qctx->name (a char[RPMI_NAME_CHARS_MAX=16] buffer) using sbi_memcpy(qctx->name, name, len) without validating the length. If the device tree supplies a reg-names string longer than 15 characters, the copy overflows the fixed-size 16-byte buffer.

This is the same class of buffer-length issue as #416.

Reproduction

A device-tree node with a reg-names entry longer than 15 characters (e.g., "a2p-db-and-shmem" at 17 bytes including null) will overflow the 16-byte qctx->name buffer.

Fix

Add a length clamp before the sbi_memcpy:

if (len >= RPMI_NAME_CHARS_MAX)
    len = RPMI_NAME_CHARS_MAX - 1;
sbi_memcpy(qctx->name, name, len);
qctx->name[len] = '\0';

The name is truncated if it exceeds 15 characters, and the buffer is always null-terminated.

Test

The RPMI mailbox subsystem does not have a standalone unit-test suite, but the fix is validated by review against the existing pattern in the codebase (e.g., the rpmi_shmem_transport_init() function itself already validates the reg-names count against RPMI_QUEUE_IDX_MAX_COUNT).

Limitations

  • The RPMI_NAME_CHARS_MAX constant is defined in include/sbi_utils/mailbox/rpmi_msgprot.h as 16. The existing code already uses this constant for the buffer declaration.
  • No new sbi_unit test is added because the RPMI subsystem currently has no unit-test infrastructure in the tree.
  • Cross-compilation was not performed (requires RISC-V toolchain).

rpmi_shmem_transport_init() copies a device-tree reg-names string into
qctx->name (char[RPMI_NAME_CHARS_MAX=16]) using sbi_memcpy() without
validating the length first. If the device tree supplies a reg-names
string longer than 15 characters, the copy overflows the fixed-size
buffer.

Clamp the copy length to RPMI_NAME_CHARS_MAX - 1 and ensure explicit
null termination.

Fixes: 91f46fb ("lib/utils: Add RPMI messaging protocol and shared memory transport support")
Closes: riscv-software-src#417
Signed-off-by: Yudistira Putra <pyudistira519@gmail.com>
@Yudis-bit
Yudis-bit force-pushed the fix/rpmi-shmem-name-bounds branch from 7044a03 to 065cae7 Compare September 9, 2026 06:22
@Yudis-bit Yudis-bit changed the title lib: utils: mailbox: clamp RPMSI shmem queue name to RPMI_NAME_CHARS_MAX lib: utils/mailbox: clamp RPMI shmem queue name to RPMI_NAME_CHARS_MAX Sep 9, 2026
@Yudis-bit

Copy link
Copy Markdown
Contributor Author

A correction to the description: a2p-db-and-shmem is 16 characters, and fdt_stringlist_get() returns a length excluding the terminating NUL. That example fills the 16-byte destination without terminating it; it does not itself demonstrate a write past the buffer. I should have distinguished those cases in the report.

The proposal bounds the stored name and explicitly terminates it. Would maintainers prefer truncating an overlong queue name, as proposed here, or rejecting it as invalid input?

The validation limitation in the description still applies: no cross-build or runtime validation is reported. I also note the mailing-list submission requirement in docs/contributing.md; this GitHub discussion does not replace that review.

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.

1 participant