DoS issue when using virtio with rust-vmm/vm-memory #95
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 <[email protected]> Signed-off-by: Andreea Florescu <[email protected]> Signed-off-by: Alexandru Agache <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
andreeaflorescu
requested review from
alexandruag,
bonzini and
jiangliu
as code owners
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 <[email protected]>
|
Bogus branch, will re-open with the correct one. |
|
I think the fix was not included in the master branch? |
|
@bonzini nop, it is not in master. This PR was opened from the wrong branch, that's why I closed it. The PR for master is this one: #98. I didn't merge it because we didn't manage to get in contact with the owner of the Windows container and all subsequent PRs on the master branch will fail. Should we disable the Windows pipeline until we manage to get in contact with them and merge #98? |
|
Sorry for the late reply, I'm updating the Windows docker image to use the right Rust version right now. |
|
@petrutlucian94 thanks! let us know when it's updated so that we can re-trigger the CI and merge that PR. |
|
Takes a bit longer than expected, Windows containers can be ... fun :) Had some issues with .net/powershell, now git won't install on the latest Windows image. I'll have it ready by Monday. If necessary, feel free to temporarily disable the Windows pipeline. |
|
@petrutlucian94 Monday is a bank holiday in Romania. Tuesday is also fine by me. |
|
Fixed: https://buildkite.com/rust-vmm/vm-memory-ci/builds/314 Thanks for bearing with us. |
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
added a commit
to jiangliu/vm-memory
that referenced
this pull request
Jan 4, 2021
rust-vmm#95 fixed the problem of torn reads/writes caused by the implementation of read/write_obj essentially leveraging the environment specific memcpy, and the implicit assumption that read/write_obj perform atomic accesses up to a certain size at aligned addresses. Meanwhile, we added the load/store operations to Bytes which provide explicit atomic access semantics. Now there are three possible types of memory access methods: 1) atomic access: `VolatileSlice`::load/store() for integer atomic data types 2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()| copy_from<T>|copy_from<T>} when size_of::<T>() > 1 3) normal access: all other byte stream oriented memory accesses Callers need to be care to choose the access method, in preferrence for both safety and high performance. Signed-off-by: Liu Jiang <[email protected]>
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.