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

Closed
bchalios opened this issue Apr 9, 2024 · 2 comments · Fixed by #4556
Closed

Use u32 to describe descriptor chain lengths #4548

bchalios opened this issue Apr 9, 2024 · 2 comments · Fixed by #4556
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
@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
@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