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

fix: page_align_up overflow #1078

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Marsman1996
Copy link
Contributor

Fix #1077 by using wrapping_add.

However, RedoxOS and Asterinas use checked_add.
But change to checked_add in DragonOS will lead to a lot of code changes.

https://gitlab.redox-os.org/redox-os/relibc/-/merge_requests/569

https://github.com/asterinas/asterinas/blob/c0e572becdac63d814b460875dccb9d833d2b06d/ostd/libs/align_ext/src/lib.rs#L51-L54

@dragonosbot
Copy link

感谢您的pull request,欢迎加入!🎉 DragonOS社区很兴奋地期待审核您的更改,您将在接下来的两周内收到 @fslongjin @GnoCiYeH @Chiichen (NB. this repo may be misconfigured) 的回复。💬😊

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-等待审查 and S-等待作者修改) stays updated, invoking these commands when appropriate:

  • @dragonosbot author: 审查结束后,PR的作者应检查评论并采取相应行动
  • @dragonosbot review: 作者已完成修改,将此PR提交给reviewer进行审阅,此PR将再次在审阅者队列中排队

@dragonosbot dragonosbot added the S-等待审查 Status: 等待assignee以及相关方的审查。 label Dec 11, 2024
@github-actions github-actions bot added the Bug fix A bug is fixed in this pull request label Dec 11, 2024
@fslongjin
Copy link
Member

checked_add 的话,有什么影响吗~

@Marsman1996
Copy link
Contributor Author

如果改用 checked_add,发生溢出时 checked_add 返回 None,那么我们需要类似 RedoxOS 的这个 PR
修改 page_align_up 的返回类型并在每个调用位置进行异常处理

@Marsman1996
Copy link
Contributor Author

Marsman1996 commented Dec 11, 2024

It seems DragonOS shall not call page_align_up for start of msync.

let start = page_align_up(args[0]);

Linux just return EINVAL when start unaligned.

https://elixir.bootlin.com/linux/v6.10.5/source/mm/msync.c#L44-L45


如果与 Linux 兼容的话,DragonOS 似乎不应该为 msyncstart 做页对齐,因为当不对齐时 Linux 直接返回 EINVAL 了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug fix A bug is fixed in this pull request S-等待审查 Status: 等待assignee以及相关方的审查。
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG REPORT] Integer overflow in page_align_up
3 participants