Skip to content

[Bug] rpmi_shmem_mailbox_controller_init() copies FDT queue names into 16-byte qctx->name without checking length #414

@neosys007

Description

@neosys007


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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions