vmm: preserved FDs: prevent usage of same FD multiple times #28
Closed
phip1611 wants to merge 2 commits intocyberus-technology:gardenlinuxfrom
Closed
vmm: preserved FDs: prevent usage of same FD multiple times #28phip1611 wants to merge 2 commits intocyberus-technology:gardenlinuxfrom
phip1611 wants to merge 2 commits intocyberus-technology:gardenlinuxfrom
Conversation
f9b7d8d to
8f88c6e
Compare
2 tasks
e69055e to
109f6cd
Compare
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de> On-behalf-of: SAP philipp.schuster@sap.com
Allowing the same file descriptor (FD) to be preserved more than once can lead to severe issues. Consider the case where management software adds two virtio-net devices at runtime, both backed by the same externally provided FD. When the first device is removed during runtime, the FD for the second device becomes invalid [0]. To avoid misconfigurations, we now prevent multiple preservation attempts of the same FD. This however has some caveats: In the current model, external FDs are added to the list of preserved FDs after the corresponding device has been created successfully. Now assume the following: - VM boots - VM config has no preserved FDs - We add a device to the VM that uses an external FD for its Tap dev - The device is successfully initialized - VM config is extended with the preserved FDs - VM reboots - VM config already has preserved FDs - Same as above - CRASH Therefore, we need to know for every preserved FD whether it is "cold", i.e., only has been part of a former VmConfig without associated device, or "hot", i.e., it is currently also actively in use. This allows state transitions from cold->hot in which case we won't throw an error for a reused FD. Otherwise, we throw errors. This requires that on shutdown, all preserved FDs are marked as cold. [0] cloud-hypervisor#7371 Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de> On-behalf-of: SAP philipp.schuster@sap.com
109f6cd to
792ff49
Compare
Member
Author
|
Doesn't really solve a problem that we have right now and we have more important work. I think I will fix this upstream. Also see cloud-hypervisor#7475 |
This file contains hidden or 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
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.
About
Part of https://github.com/cobaltcore-dev/cobaltcore/issues/292. Currently not a big issue, but technically it can be a severe problem. And it might be beneficial to have this to detect misconfiguration.
Technical description: See commit message.
Why I worked on this
I worked on this in my attempt to finish some upstream PRs. I noticed this is more complicated than anticipated. So I created this PoC for us and upstreaming is future work.