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

Use u32 to describe descriptor chain lengths #4548

Open
bchalios opened this issue Apr 9, 2024 · 2 comments · May be fixed by #4556 or #4380
Open

Use u32 to describe descriptor chain lengths #4548

bchalios opened this issue Apr 9, 2024 · 2 comments · May be fixed by #4556 or #4380
Labels
Good first issue Indicates a good issue for first-time contributors

Comments

@bchalios
Copy link
Contributor

bchalios commented Apr 9, 2024

Description

Currently, parts of our virtio code use usize to describe the lengths of descriptor chains [1], while other parts use u32, which results in some ugly casts that can panic if a descriptor chain with length exceeding 2^32-1 bytes slips through validation somehow [2]. According to the virtio spec, descriptor chains can be at most 2^32-1 bytes long (as the "len" parameter in the used ring is a u32). We should thus use u32 instead of usize to describe these lengths, and upcast when interacting with non-virtio code that expects lengths to be usize.

Solution

  • Change the len parameters in IoVecBuffer[Mut] to be u32
  • Inside of the from_descriptor_chain functions, add validation that the total length does not overflow a u32, and return a new IoVecError variant if it does
@bchalios bchalios added the Good first issue Indicates a good issue for first-time contributors label Apr 9, 2024
@bchalios bchalios linked a pull request Apr 9, 2024 that will close this issue
9 tasks
@BipulLamsal
Copy link

hey i want to work in this issue do i need to make changes for IoVecBufferMut and IoVecBuffer both

@bchalios
Copy link
Contributor Author

Hey @BipulLamsal thanks for your interest in this. I think there was someone already working on this. Let me confirm with them and I will come back to you.

@bchalios bchalios self-assigned this Apr 12, 2024
@brandonpike brandonpike linked a pull request Apr 12, 2024 that will close this issue
9 tasks
@bchalios bchalios removed their assignment Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Indicates a good issue for first-time contributors
Projects
None yet
2 participants