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

Call to VirtAddr::new() causes panic when CR2 does not contain a canonical address. #332

Closed
RevolutionXenon opened this issue Dec 29, 2021 · 4 comments · Fixed by #334 or #335
Closed

Comments

@RevolutionXenon
Copy link

https://docs.rs/x86_64/latest/src/x86_64/registers/control.rs.html#267

When CR2 contains a malformed address, this panic occurs:
"panicked at 'address passed to VirtAddr::new must not contain any data in bits 48 to 64"

CR2 can obviously contain a non-canonical address after a page fault. This panic should not occur. I'm not sure what the best fix would be, but I ran into this because I tried to print CR2 in my panic handler and it just starts looping when you do that.

@Freax13
Copy link
Member

Freax13 commented Dec 29, 2021

Good catch we should definitely not panic in that situation. We could change Cr2::read to return Result<VirtAddr, VirtAddrNotValid> and add a new method Cr2::read_raw that returns u64.

Changing Cr2::read would be a breaking change, but adding Cr2::read_raw could be done right now.

This was referenced Jan 16, 2022
@josephlr
Copy link
Contributor

Should this stay open to track the (breaking) change to Cr2::read?

@Freax13 Freax13 reopened this Jan 18, 2022
@josephlr
Copy link
Contributor

Whoops I just saw #335 I guess that PR (when merged) should close this issue.

@Freax13 Freax13 linked a pull request Feb 5, 2022 that will close this issue
@toku-sa-n
Copy link
Member

I think this issue can be closed.

@Freax13 Freax13 closed this as completed Feb 7, 2022
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

Successfully merging a pull request may close this issue.

4 participants