riscv-software-src / riscv-software-src/opensbi
[Bug] rpmi_shmem_mailbox_controller_init() copies FDT queue names into 16-byte qctx->name without checking length
Open
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 1.5k
- Forks
- 712
- PR merge metrics
- No merged PRs in 30d
Description
Hello,
I reviewed current upstream head and found a real missing length check in the RPMI shared-memory mailbox FDT parser.
The queue context stores the name in a fixed field:
```c
#define RPMI_NAME_CHARS_MAX (16)
struct rpmi_mbox_queue_context {
...
char name[RPMI_NAME_CHARS_MAX];
};
```
The initialization code later does:
```c
count = fdt_stringlist_count(fdt, nodeoff, "reg-names");
if (count < 0 || count > (RPMI_QUEUE_IDX_MAX_COUNT + RPMI_REG_IDX_MAX_COUNT))
return SBI_EINVAL;
...
name = fdt_stringlist_get(fdt, nodeoff, "reg-names", qid, &len);
if (!name || (name && len < 0))
return len;
sbi_memcpy(qctx->name, name, len);
```
The important point is that the existing check only validates the number of strings, not the length of each string. `fdt_stringlist_get(..., &len)` returns the actual string length, and current head copies that length directly into `qctx->name[16]` with no `len <= RPMI_NAME_CHARS_MAX` check.
So the current-head claim is narrow and concrete:
- queue names are stored in a fixed 16-byte field,
- the FDT parser obtains an arbitrary string length for each entry,
- and the copy uses that length directly.
A straightforward fix would be to reject any queue name with `len >= RPMI_NAME_CHARS_MAX`, or clamp and explicitly terminate if truncation is acceptable.
Best regards
Pengpeng Hou
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at rpmi_shmem_mailbox_controller_init() and inspect how reg-names lengths are returned and copied into qctx->name. Check the surrounding RPMI mailbox code for the expected handling of oversized names, then ensure an overlong FDT queue name cannot overflow the fixed field. Verify the behavior with an oversized queue-name input if the repository provides relevant parser tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100