Skip to content

Fix mremap#807

Open
CvvT wants to merge 2 commits intomainfrom
weiteng/fix_mremap
Open

Fix mremap#807
CvvT wants to merge 2 commits intomainfrom
weiteng/fix_mremap

Conversation

@CvvT
Copy link
Copy Markdown
Contributor

@CvvT CvvT commented Apr 24, 2026

This PR fixes three issues in mremap found by AI.

  1. old_size and new_size are rounded down to page boundary instead of up (Linux expects round-up).
  2. The default impl of remap_pages copies wrong chunk size, reading out-of-bounds on multi-page remaps.
  3. Fix overlap check in kernel platform.

@CvvT
Copy link
Copy Markdown
Contributor Author

CvvT commented Apr 24, 2026

The second issue is also fixed in #754, but I feel it might be better to include it in this PR.

@github-actions
Copy link
Copy Markdown

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

@CvvT CvvT marked this pull request as ready for review April 24, 2026 22:15
let new_range = PageRange::new(new_range.start, new_range.end)
.ok_or(litebox::platform::page_mgmt::RemapError::Unaligned)?;
if old_range.start.max(new_range.start) <= old_range.end.min(new_range.end) {
if old_range.start.max(new_range.start) < old_range.end.min(new_range.end) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

litebox_platform_lvbs/src/lib.rs:1269 does have the same problem. It would be better to fix these two together.

Comment on lines +181 to +182
let old_size = align_up(old_size, PAGE_SIZE);
let new_size = align_up(new_size, PAGE_SIZE);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

checked_next_multiple_of() might be better?

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 this pull request may close these issues.

2 participants