-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: System shared memory boundary check #8335
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
Conversation
| create_byte_size=create_byte_size, | ||
| register_offset=register_offset, | ||
| ) | ||
| offset = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smaller offset for stricter check.
| error_msg = [] | ||
| self._configure_server() | ||
| create_byte_size = self.SYS_PAGE_SIZE + self.DEFAULT_SHM_BYTE_SIZE | ||
| register_offset = self.SYS_PAGE_SIZE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this fix, out-of-bound would not be triggered if register_offset is non-zero.
| size_t shm_region_size = 0; | ||
| if (it->second->byte_size_ > 0) { | ||
| shm_region_end += it->second->byte_size_ - 1; | ||
| shm_region_size += it->second->byte_size_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove -1 for better readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This actually avoids a potential unsigned integer underflow if both offset and byte_size are 0. I had just encountered this underflow last week.
What does the PR do?
Followup from the original request. Similar bug in shared memory boundary checks.
Modify test cases to include
register_offsetin boundary checks.Checklist
<commit_type>: <Title>Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
Related PRs:
#8334
Where should the reviewer start?
Test plan:
L0_shared_memory
32964778
Caveats:
Background
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)