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

soundness improvements around hypervisor-shared memory #451

Merged
merged 9 commits into from
Oct 2, 2024

Commits on Sep 18, 2024

  1. greq: derive AsBytes for SnpGuestRequestMsgHdr

    This makes it possible to implement get_aad_slice without any unsafe
    code.
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 18, 2024
    Configuration menu
    Copy the full SHA
    baa6142 View commit details
    Browse the repository at this point in the history
  2. hv_doorbell: wrap reserved_63_4 in UnsafeCell

    Given that the hypervisor has write access to that memory, we need to
    treat the memory as interiorly mutable.
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 18, 2024
    Configuration menu
    Copy the full SHA
    08bd0e5 View commit details
    Browse the repository at this point in the history

Commits on Sep 20, 2024

  1. ghcb: make all accesses atomic

    Cell doesn't allow concurrent accesses. This is a problem because we
    share the memory with the host and the host could write to the memory
    while we're reading it. Use atomic accesses instead. Atomic accesses
    can tolerate concurrent writes.
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 20, 2024
    Configuration menu
    Copy the full SHA
    0166046 View commit details
    Browse the repository at this point in the history

Commits on Sep 26, 2024

  1. greq: copy data before checking if it's empty

    This resolves a TOC-TOU issue. Furthermore we don't need to check the
    entire content: If the certificate data is not empty, there will be
    non-zero bytes in the first 24 bytes.
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 26, 2024
    Configuration menu
    Copy the full SHA
    f05092b View commit details
    Browse the repository at this point in the history
  2. mm: implement SharedBox

    SharedBox is a safe wrapper around memory pages shared with the host.
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 26, 2024
    Configuration menu
    Copy the full SHA
    48fe3e4 View commit details
    Browse the repository at this point in the history
  3. mm: replace HVDoorbellPage with single function

    HVDoorbellPage was only used in one place and leak was immediately
    called on it. Given that we don't ever need to free up a doorbell page
    let's just implement this in a single function returning a static
    reference.
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 26, 2024
    Configuration menu
    Copy the full SHA
    dbcdbbb View commit details
    Browse the repository at this point in the history
  4. stage2: use drop_in_place instead of shutdown

    This is better for a couple of reasons:
    1. drop_in_place destroys the object rather than mutating it to release
       resources. The downside with simply mutating but not destroying is
       that the object still has to be in a valid state and this limits the
       shutdown code (for example it can't release the memory associated
       with a PageBox)
    2. After the object has been dropped, it can't be accessed anymore.
       This means that the shutdown code doesn't have to worry about later
       accesses like the previous code had to.
    3. All resources are freed, not just the GHCB.
    
    This also fixes a soundness issue where if the shutdown were to be
    called twice on the same GHCB that would result in a double-pvalidate
    bug.
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 26, 2024
    Configuration menu
    Copy the full SHA
    9ea3c59 View commit details
    Browse the repository at this point in the history
  5. ghcb: remove DerefMut impl

    This impl is unused. It is also unsound because we can never have
    unique ownership over the GHCB as long as it is shared with the host.
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 26, 2024
    Configuration menu
    Copy the full SHA
    48b9563 View commit details
    Browse the repository at this point in the history
  6. ghcb: move shutdown code into Drop impl

    Now that the shutdown code is only called from the Drop impl we might
    as well move it in there. This also makes it impossible to call
    shutdown more than once (or to call shutdown and the Drop the
    GhcbPage).
    
    Signed-off-by: Tom Dohrmann <[email protected]>
    Freax13 committed Sep 26, 2024
    Configuration menu
    Copy the full SHA
    f263c5f View commit details
    Browse the repository at this point in the history