DoS issue when using virtio with rust-vmm/vm-memory #98
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some memcpy implementations are copying bytes one at a time, which is slow and also breaks the virtio specification by splitting writes to the fields of the virtio descriptor. So, reimplement memcpy in Rust and copy in larger pieces, according to the largest possible alignment allowed by the pointer values. Signed-off-by: Serban Iorga <seriorga@amazon.com> Signed-off-by: Andreea Florescu <fandree@amazon.com> Signed-off-by: Alexandru Agache <aagch@amazon.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
andreeaflorescu
requested review from
alexandruag,
bonzini and
jiangliu
as code owners
|
Related to: #93 |
andreeaflorescu
force-pushed
the
fix_for_master
branch
from
8c048cd
to
727491f
Compare
This crate was using a pretty old version of Rust (1.35.0) which didn't have the required features. Signed-off-by: Andreea Florescu <fandree@amazon.com>
andreeaflorescu
force-pushed
the
fix_for_master
branch
from
727491f
to
99fe300
Compare
serban300
approved these changes
May 28, 2020
alexandruag
approved these changes
May 28, 2020
|
I suspect this has an impact on the x86_64+glibc target where memcpy is surely ISA optimized. |
sameo
approved these changes
May 28, 2020
Yap, we should do that. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have identified an issue in the rust-vmm vm-memory crate that leads to a denial-of-service (DoS) issue if the crate is used in a VMM in conjunction with virtio. The issue affects both vm-memory releases (v0.1.0 and v0.2.0). In our environment, we reproduced this with musl builds on x86_64, and with all aarch64 builds. This PR fixes the issue. The fix will also be applied to the aforementioned releases. All consumers should switch to vm-memory v0.1.1 or v0.2.1.
Issue Description
In rust-vmm/vm-memory, the functions read_obj and write_obj are not doing atomic accesses for all combinations of platform and libc implementations. These reads and writes translate to memcpy, which may be performing byte-by-byte copies. Using vm-memory in the virtio implementation can cause undefined behavior, as descriptor indexes require 2-byte atomic accesses.
Impact
The issue can affect any virtio/emulated device which expects atomic writes for base types longer than 1 byte.
Observed impact: When the network stack is under load, the driver will try to clear a used descriptor before the index of the descriptor is fully written by the device. When this issue is triggered, the virtio-net device will be unable to transmit packets. This leads to VMs using rust-vmm/vm-memory having their network effectively disconnected by outside network traffic, resulting in both a DoS vector and an availability issue under normal at-load operations.
Affected Systems
For a VMM to be affected, it must run on aarch64 (built with either musl or glibc), or on x86_64 with a musl build. All VMMs using rust-vmm/vm-memory (any release) in a production scenario, and that take arbitrary traffic over the virtio-net device, are confirmed to be at risk of a DOS. All VMMs using rust-vmm/vm-memory (any release) in a production scenario with a virtio-net deice are under availability risk. All VMMs using rust-vmm/vm-memory (any release) in a production scenario using other devices that expect atomic reads for more than 1-byte values may also be affected, but we are unaware of any risk for other devices (beyond the guest freezing its own virtio stack).
Mitigation
This PR fixes the issue. The fix will also be applied to the aforementioned releases. All consumers should switch to vm-memory v0.1.1 or v0.2.1. On x86_64 glibc builds the fix may lead to a 5% network throughput degradation.