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
#414 0 comments 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.