Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsoundness in LR register access. #570

Open
thomasw04 opened this issue Dec 30, 2024 · 5 comments
Open

Unsoundness in LR register access. #570

thomasw04 opened this issue Dec 30, 2024 · 5 comments

Comments

@thomasw04
Copy link

If I interpret this correctly, LR can basically contain an undefined value when reading it. (see https://doc.rust-lang.org/reference/inline-assembly.html#r-asm.rules.reg-not-input) In addition, the behavior is undefined when writing to LR without declaring it as out register. (see https://doc.rust-lang.org/reference/inline-assembly.html#r-asm.rules.reg-not-output)

Furthermore, specifically with the link register (LR) in the context of function calls, (see https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#subroutine-calls) this then completely relies on the compiler inlining this function, as otherwise, LR would contain the return address of the read/write function. (apart from that, by the rules above, LR does not even need to contain anything meaningful).

I am unsure how to solve this right now. If I am not mistaken, the inlining problem could be solved by using a macro.
However, regarding the undefined behavior in general, I am unsure if Rust currently has a way to make this reliably safe.

/// Reads the CPU register
#[cfg(cortex_m)]
#[inline]
pub fn read() -> u32 {
let r;
unsafe { asm!("mov {}, lr", out(reg) r, options(nomem, nostack, preserves_flags)) };
r
}
/// Writes `bits` to the CPU register
#[cfg(cortex_m)]
#[inline]
pub unsafe fn write(bits: u32) {
asm!("mov lr, {}", in(reg) bits, options(nomem, nostack, preserves_flags));
}

@jannic
Copy link
Member

jannic commented Jan 7, 2025

I'm not sure that reading lr is undefined behavior. It returns an undefined value, but the page you cited actually states that this is not the same as LLVM undef. Instead it is just some non-deterministic, but valid, value:
"An “undefined value” in the context of inline assembly means that the register can (non-deterministically) have any one of the possible values allowed by the architecture. Notably it is not the same as an LLVM undef which can have a different value every time you read it (since such a concept does not exist in assembly code)."

@thomasw04
Copy link
Author

I'm not sure that reading lr is an undefined behavior.

Yes. I only said that writing (so the write() function) is undefined behavior. Reading yields an undefined value, as you correctly stated. To give a concrete example, to my understanding, it would be perfectly valid if the compiler generates instructions that write some undefined value into lr before the inline asm and restores the old value after the inline asm.

@thomasw04
Copy link
Author

I think it makes sense to remove this functionality altogether. For me, it does not make sense to read/write this value in the context of a rust program, as almost nothing can be assumed about which instructions the compiler generates in the surroundings of the inline asm block.

@jannic
Copy link
Member

jannic commented Jan 7, 2025

As write is marked as unsafe, technically it's not unsound. It's just impossible to use it safely.
Therefore, the best approach may be to tell so in a comment, and mark the function as deprecated. That would avoid a breaking change, and solve the issue for all practical purposes.

@thomasw04
Copy link
Author

Okay, I see your point. Other than that, I really want to emphasize that this is not some theoretical construct that rarely has practical implications. Using this functionality for e.g. changing the return address of a function or extracting the return address of a function can and very well will break your code.

Easy example, where the stack will get corrupted on using write: https://godbolt.org/z/eK7rb16Th

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants