Repository navigation
vhost_user: use opaque guest addresses instead of exposing HVAs (#7190) - #9016
adilalperenciftci wants to merge 1 commit into
Conversation
83dde6b to
64fdf33
Compare
rbradford
left a comment
rbradford
left a comment
There was a problem hiding this comment.
We already have a system for mapping like this which we use for the translating for VIRTIO_F_ACCESS_PLATFORM which should be reused instead.
New contributors are strongly discouraged from using LLMs as it does not help them understand the project and develop their skills. Please spend time reviewing the existing code and PRs before starting to try and contribute.
| #[derive(Clone, Copy, Default)] | ||
| struct VhostUserAddressMapper { | ||
| gpa_addresses: bool, | ||
| } |
There was a problem hiding this comment.
Please don't submit PRs with LLM slop. Creating a struct for a single bool that is only embedded into one struct just so that the LLM can satisfy "always have a test" is classic LLM slop.
There was a problem hiding this comment.
yes, you're right. i missed the existing AccessPlatform mapping path. i kept this separate because GPA_ADDRESSES is negotiated with the backend while VIRTIO_F_ACCESS_PLATFORM is negotiated with the guest, and i wanted the vhost-user wire-address handling and range checks to stay together. the struct was a deliberate design choice, not something added just for testing, but i agree that it duplicates an existing abstraction. i'll rework it to reuse the current mapping system. also, i didn't use an llm to write this change.
The memory table and the vring addresses sent to a vhost-user backend carried the frontend's host virtual addresses. A backend only needs those values to be consistent with each other so it can derive offsets into its own mappings, so there is no reason to hand out frontend HVAs. Advertise the GPA_ADDRESSES protocol feature and reuse the existing AccessPlatform translation that VIRTIO_F_ACCESS_PLATFORM already relies on. A backend that negotiates GPA_ADDRESSES needs no translation and receives guest physical addresses unchanged. A backend that does not negotiate it keeps working against a biased guest address, which stays consistent across the memory table and the vrings while remaining non-zero. The vring bounds check keeps rejecting rings that are not backed by a single guest memory region, and the ring sizes now include the event field only when VIRTIO_F_RING_EVENT_IDX has been negotiated, so a region-boundary-aligned ring is no longer rejected for a trailer that the guest never allocated. Signed-off-by: Adil Alperen Çiftci <adilerta54@gmail.com>
1103cc9 to
842719c
Compare
|
Thanks for the pointer to the existing translation path, and for the review. Reworked to reuse it. The handle now carries an Two other things changed while doing that:
The event-field sizing fix is folded into the same commit, since the sizes it Local verification: |
vhost-user backends were receiving actual host virtual addresses. This patch prevents HVA disclosure while keeping legacy compatibility and adding support for
GPA_ADDRESSESnegotiation.VhostUserAddressMapperinvu_common_ctrlto handle GPA/HVA translation safely.GPA_ADDRESSESis supported, we map directly to GPAs.0x1000synthetic bias to prevent NULL pointer issues without leaking the HVA.Tested via:
cargo test -p virtio-devices -p vhost_user_block -p vhost_user_net --libcargo check --locked -p virtio-devices --all-targets --testsSET_MEM_TABLEandSET_VRING_ADDRensuring no HVA leakage in either mode.vhost-user-backendmissingGPA_ADDRESSES(verifying synthetic namespace works).Fixes #7190