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

The acpi crate unmaps mappings that contain other mappings #79

Closed
JCapucho opened this issue Nov 22, 2020 · 7 comments
Closed

The acpi crate unmaps mappings that contain other mappings #79

JCapucho opened this issue Nov 22, 2020 · 7 comments

Comments

@JCapucho
Copy link

The acpi crate while calling search_for_rsdp_bios will map 0xF5000 with a size of 0x1000 and then map 0xF5B20 with a size of 0x24 and immediately after unmap the first mapping causing the second mapping to be invalid and cause a page fault while reading

@IsaacWoods
Copy link
Member

Huh, turns out Rust's drop order when replacing a field is different to what I expected. Basically, to fix this we'll need to make sure to drop the old mapping before we make the new one here, and also probably deal with the fact that this same problem will also trip this up.

I'm not sure when I'll have time to work on a fix for this, so anyone is free to take this, or I'll get around to it when I have a chance in the next week or so.

@JCapucho
Copy link
Author

Thanks for the quick response, I've also found out that reverse happens already mapped regions are mapped again

@IsaacWoods
Copy link
Member

I've published rsdp v1.1.0, which should fix both of these issues. However, I don't have anything booting from BIOS with which I can test this easily, so I'd appreciate if you can confirm if this works, and then I'll publish it as a proper version. If you're using acpi instead of rsdp directly, you can use a [patch.crates-io] in your Cargo.toml to override the version of rsdp that acpi uses.

@JCapucho
Copy link
Author

JCapucho commented Dec 30, 2020

Sorry for the late reply, I tried to patch the rsdp crate but i couldn't because acpi requires rsdp=1 and right now the current version has -pre0 which doesn't fulfill the requirement

@IsaacWoods
Copy link
Member

Ah my bad, I thought you'd be able to override that. I've published acpi v2.2.0-pre0, which pulls in the prerelease of rsdp. Could you give that a try?

@JCapucho
Copy link
Author

JCapucho commented Jan 1, 2021

Just tested and it looks like it's working correctly

@JCapucho JCapucho closed this as completed Jan 1, 2021
@IsaacWoods
Copy link
Member

IsaacWoods commented Jan 1, 2021

Great, thanks for testing.

Published as rsdp v1.1.0 and acpi v2.2.0.

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