Skip to content

Fix potential overflow in FdtWriter::begin_node() #46

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
Merged
merged 3 commits into from
Oct 19, 2021
Merged

Fix potential overflow in FdtWriter::begin_node() #46

merged 3 commits into from
Oct 19, 2021

Conversation

@gsserge
Copy link

@gsserge gsserge commented Oct 18, 2021 •
edited
Loading

Add a check to make sure than node depth does not
exceed FDT_MAX_NODE_DEPTH.

Signed-off-by: Sergii Glushchenko gsserge@amazon.com
Fix potential overflow in FdtWriter::begin_node()
b90841d 
Add a check to make sure than node depth does not exceed usize::MAX. Signed-off-by: Sergii Glushchenko <gsserge@amazon.com>
@gsserge
Copy link
Author

gsserge commented Oct 18, 2021 •
edited
Loading

I'm using 'if' block instead of usize::checked_add to keep the style compatible with FdtWriter::end_node; please let me know if checked_add is preferred.
@gsserge gsserge marked this pull request as draft October 18, 2021 14:20
danielverkamp
danielverkamp previously approved these changes Oct 18, 2021
View reviewed changes
Copy link
Collaborator

@danielverkamp danielverkamp left a comment

Choose a reason for hiding this comment

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

This style of check looks fine to me; thanks for adding the comment explaining it at the addition as well.
Limit max node depth to 64
e393661 
Signed-off-by: Sergii Glushchenko <gsserge@amazon.com>
@gsserge gsserge marked this pull request as ready for review October 19, 2021 11:37
Adjust test coverage
a568a59 
Signed-off-by: Sergii Glushchenko <gsserge@amazon.com>
lauralt
lauralt approved these changes Oct 19, 2021
View reviewed changes
src/writer.rs
const FDT_HEADER_SIZE: usize = 40;
const FDT_VERSION: u32 = 17;
const FDT_LAST_COMP_VERSION: u32 = 16;
/// The same max depth as in the Linux kernel.
Copy link
Contributor

@lauralt lauralt Oct 19, 2021

Choose a reason for hiding this comment

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

nit: this should be part of the previous commit.
@andreeaflorescu andreeaflorescu merged commit bb53be4 into rust-vmm:main Oct 19, 2021
@gsserge gsserge deleted the depth_overflow branch October 19, 2021 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants

@gsserge @danielverkamp @andreeaflorescu @lauralt