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.

Sign up for GitHub

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

Jump to bottom

Frame Allocator can create duplicate free regions during initialization #1105

Merged
merged 3 commits into from
Sep 3, 2024
Merged

Frame Allocator can create duplicate free regions during initialization #1105

merged 3 commits into from
Sep 3, 2024

Conversation

Ramla-I
Copy link
Member

@Ramla-I Ramla-I commented Aug 28, 2024 •
edited
Loading

The current code catches the mistake and reports an error. The duplicate region is created due to a bug in the check_and_add_free_region function. After returning from a recursive call, the original area's end frame should be updated to not include the region that was just added to the free list.

I believe there is an unspoken assumption that the list of reserved regions will be in ascending order, but that is not always the case.

In the screenshots below, the original code adds the region 0x2098000..=0x1FFDE000 twice.

Screenshot_w_bug Screenshot_w_fix

@Ramla-I
fix
f62230b
@Ramla-I
Update lib.rs
12e288f 
commented out log
@Ramla-I
Update lib.rs
60fce22 
removed warning
kevinaboos
kevinaboos approved these changes Sep 3, 2024
View reviewed changes
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, good catch! Thanks.

@kevinaboos kevinaboos merged commit 68cc3ae into theseus-os:theseus_main Sep 3, 2024
3 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 3, 2024
@kevinaboos
Frame Allocator: avoid creating duplicate free regions during init (#…
cf8c085 
…1105)

The duplicate region was created due to a bug
in the `check_and_add_free_region` function:
After returning from a recursive call, the original area's
end frame should be updated to *not include* th
 region that was just added to the free list. 68cc3ae
@Ramla-I Ramla-I deleted the fa_repeated_free_regions branch September 9, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinaboos kevinaboos kevinaboos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Ramla-I @kevinaboos