Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

unwinding: perform more bounds checking #185

Open
japaric opened this issue Apr 29, 2021 · 1 comment
Open

unwinding: perform more bounds checking #185

japaric opened this issue Apr 29, 2021 · 1 comment
Labels
difficulty: easy Pretty easy to solve priority: low Low priority for the Knurling team status: needs PR Issue just needs a Pull Request implementing the changes topic: unwinding type: enhancement Enhancement or feature request

Comments

@japaric
Copy link
Member

japaric commented Apr 29, 2021

as seen in PR #184, the probe-rs API does not perform bounds checking so reading invalid (RAM) memory returns 0 (observed behavior so far; it may return junk with different probes). Thus we should do some bound checking on the probe-run side. That PR adds bound checking to the operation of "read stacked registers" but we should be more thorough and validate all memory operations (be it RAM or Flash). (The "read core register" operation does not need to be validated I think)

Instances that should be checked:

@japaric japaric added status: needs PR Issue just needs a Pull Request implementing the changes type: enhancement Enhancement or feature request labels Apr 29, 2021
@Urhengulas Urhengulas added difficulty: easy Pretty easy to solve priority: low Low priority for the Knurling team labels May 5, 2021
@Urhengulas
Copy link
Member

Using VS Code global search, I found the following read operations (excluding read_core_reg):


canary.rs: Should be okay since the addresses are derived from StackInfo.

main.rs: Should be okay as well, since we get the address from the __SEGGER_RTT symbol.

registers.rs: It should be sufficient to store the StackInfo (or just the ram_range) in the Registers struct and use that for the bounds check.

stacked.rs: Already performs a bounds check.


Should write operations perform a bounds check as well?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
difficulty: easy Pretty easy to solve priority: low Low priority for the Knurling team status: needs PR Issue just needs a Pull Request implementing the changes topic: unwinding type: enhancement Enhancement or feature request
Projects
None yet
Development

No branches or pull requests

2 participants