mbox series

[RFC,RFT,0/4] Handle set_memory_XXcrypted() errors in hyperv

Message ID 20240222021006.2279329-1-rick.p.edgecombe@intel.com
Headers
Series Handle set_memory_XXcrypted() errors in hyperv |

Message

Edgecombe, Rick P Feb. 22, 2024, 2:10 a.m. UTC 
  Shared (decrypted) pages should never return to the page allocator, or 
future usage of the pages may allow for the contents to be exposed to the 
host. They may also cause the guest to crash if the page is used in way 
disallowed by HW (i.e. for executable code or as a page table).

Normally set_memory() call failures are rare. But on TDX 
set_memory_XXcrypted() involves calls to the untrusted VMM, and an attacker
could fail these calls such that:
 1. set_memory_encrypted() returns an error and leaves the pages fully
    shared.
 2. set_memory_decrypted() returns an error, but the pages are actually
    full converted to shared.

This means that patterns like the below can cause problems:
void *addr = alloc();
int fail = set_memory_decrypted(addr, 1);
if (fail)
        free_pages(addr, 0);

And:
void *addr = alloc();
int fail = set_memory_decrypted(addr, 1);
if (fail) {
        set_memory_encrypted(addr, 1);
        free_pages(addr, 0);
}

Unfortunately these patterns appear in the kernel. And what the 
set_memory() callers should do in this situation is not clear either. They 
shouldn’t use them as shared because something clearly went wrong, but 
they also need to fully reset the pages to private to free them. But, the 
kernel needs the VMMs help to do this and the VMM is already being 
uncooperative around the needed operations. So this isn't guaranteed to 
succeed and the caller is kind of stuck with unusable pages.

The only choice is to panic or leak the pages. The kernel tries not to 
panic if at all possible, so just leak the pages at the call sites. 
Separately there is a patch[0] to warn if the guest detects strange VMM 
behavior around this. It is stalled, so in the mean time I’m proceeding 
with fixing the callers to leak the pages. No additional warnings are 
added, because the plan is to warn in a single place in x86 set_memory() 
code.

This series fixes the cases in the hyperv code.

IMPORTANT NOTE:
I don't have a setup to test tdx hyperv changes. These changes are compile
tested only. Previously Michael Kelley suggested some folks at MS might be
able to help with this.

[0] https://lore.kernel.org/lkml/20240122184003.129104-1-rick.p.edgecombe@intel.com/

Rick Edgecombe (4):
  hv: Leak pages if set_memory_encrypted() fails
  hv: Track decrypted status in vmbus_gpadl
  hv_nstvsc: Don't free decrypted memory
  uio_hv_generic: Don't free decrypted memory

 drivers/hv/channel.c         | 11 ++++++++---
 drivers/hv/connection.c      | 11 +++++++----
 drivers/net/hyperv/netvsc.c  |  7 +++++--
 drivers/uio/uio_hv_generic.c | 12 ++++++++----
 include/linux/hyperv.h       |  1 +
 5 files changed, 29 insertions(+), 13 deletions(-)