[otp_ctrl] Bug in `dif_otp_ctrl_read_blocking` causes out of range check to not execute when it should
Nobody has claimed this yet.
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
Description
### Description
The function `dif_otp_ctrl_read_blocking` in `sw/device/lib/dif/dif_otp_ctrl.c` reads from the memory-mapped software config OTP partitions. It takes a parameter `size_t len`, which is documented as "The number of words to read".
The last few lines of `dif_otp_ctrl_read_blocking` look like this:
```c
if (address + len >= kPartitions[partition].len) {
return kDifOutOfRange;
}
uint32_t reg_offset = OTP_CTRL_SW_CFG_WINDOW_REG_OFFSET +
kPartitions[partition].start_addr + address;
mmio_region_memcpy_from_mmio32(otp->base_addr, reg_offset, buf,
len * sizeof(uint32_t));
return kDifOk;
```
The out of range check here is broken; `kPartitions[partition].len` represents a quantity of bytes, per the documentation for the `uint32_t len` field in `partition_info_t`: "The length of this partition, in bytes, including the digest". `address + len` should instead be `address + len * sizeof(uint32_t)` so that all units are converted to bytes before comparison.
The resulting behavior here is that the caller can read beyond the boundary of a memory-mapped partition, when they should not be able to. "Garbage" data is read into the buffer; no type of fault occurs.
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 in sw/device/lib/dif/dif_otp_ctrl.c at dif_otp_ctrl_read_blocking and inspect the partition length units alongside the read offset and copy length. Confirm that boundary checks reject reads beyond a partition while still allowing valid reads, and verify the behavior with the relevant DIF tests if available.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- embedded-iot, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100