From: mhkelley58@gmail.com
To: rick.p.edgecombe@intel.com, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
gregkh@linuxfoundation.org, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
kirill.shutemov@linux.intel.com, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-coco@lists.linux.dev
Cc: sathyanarayanan.kuppuswamy@linux.intel.com, elena.reshetova@intel.com
Subject: [PATCH 0/5] Handle set_memory_XXcrypted() errors in Hyper-V
Date: Mon, 11 Mar 2024 09:15:53 -0700 [thread overview]
Message-ID: <20240311161558.1310-1-mhklinux@outlook.com> (raw)
From: Michael Kelley <mhklinux@outlook.com>
Shared (decrypted) pages should never be returned to the page allocator,
lest future usage of the pages store data that should not be exposed to
the host. They may also cause the guest to crash if the page is used in
a way disallowed by HW (i.e. for executable code or as a page table).
Normally set_memory() call failures are rare. But in CoCo VMs
set_memory_XXcrypted() may involve calls to the untrusted host, 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 host's help to do this and the host 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[1] to warn if the guest detects strange host
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 Hyper-V code.
This is the non-RFC/RFT version of Rick Edgecombe's previous series.[2]
Rick asked me to do this version based on my comments and the testing
I did. I've tested most of the error paths by hacking
set_memory_encrypted() to fail, and observing /proc/vmallocinfo and
/proc/buddyinfo to confirm that the memory is leaked as expected
instead of freed.
Changes in this version:
* Expanded commit message references to "TDX" to be "CoCo VMs" since
set_memory_encrypted() could fail in other configurations, such as
Hyper-V CoCo guests running with a paravisor on SEV-SNP processors.
* Changed "Subject:" prefixes to match historical practice in Hyper-V
related source files
* Patch 1: Added handling of set_memory_decrypted() failure
* Patch 2: Changed where the "decrypted" flag is set so that
error cases not related to set_memory_encrypted() are handled
correctly
* Patch 2: Fixed the polarity of the test for set_memory_encrypted()
failing
* Added Patch 5 to the series to properly handle free'ing of
ring buffer memory
* Fixed a few typos throughout
[1] https://lore.kernel.org/lkml/20240122184003.129104-1-rick.p.edgecombe@intel.com/
[2] https://lore.kernel.org/linux-hyperv/20240222021006.2279329-1-rick.p.edgecombe@intel.com/
Michael Kelley (1):
Drivers: hv: vmbus: Don't free ring buffers that couldn't be
re-encrypted
Rick Edgecombe (4):
Drivers: hv: vmbus: Leak pages if set_memory_encrypted() fails
Drivers: hv: vmbus: Track decrypted status in vmbus_gpadl
hv_netvsc: Don't free decrypted memory
uio_hv_generic: Don't free decrypted memory
drivers/hv/channel.c | 16 ++++++++++++----
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, 33 insertions(+), 14 deletions(-)
--
2.25.1
next reply other threads:[~2024-03-11 16:16 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-11 16:15 mhkelley58 [this message]
2024-03-11 16:15 ` [PATCH v2 1/5] Drivers: hv: vmbus: Leak pages if set_memory_encrypted() fails mhkelley58
2024-03-12 2:56 ` Kuppuswamy Sathyanarayanan
2024-03-11 16:15 ` [PATCH v2 2/5] Drivers: hv: vmbus: Track decrypted status in vmbus_gpadl mhkelley58
2024-03-12 5:02 ` Kuppuswamy Sathyanarayanan
2024-03-12 5:45 ` Kuppuswamy Sathyanarayanan
2024-03-12 6:07 ` Michael Kelley
2024-03-12 15:22 ` Kuppuswamy Sathyanarayanan
2024-03-14 13:56 ` Michael Kelley
2024-03-11 16:15 ` [PATCH v2 3/5] hv_netvsc: Don't free decrypted memory mhkelley58
2024-03-12 5:03 ` Kuppuswamy Sathyanarayanan
2024-03-11 16:15 ` [PATCH v2 4/5] uio_hv_generic: " mhkelley58
2024-03-12 5:04 ` Kuppuswamy Sathyanarayanan
2024-03-11 16:15 ` [PATCH v2 5/5] Drivers: hv: vmbus: Don't free ring buffers that couldn't be re-encrypted mhkelley58
2024-03-12 15:16 ` Kuppuswamy Sathyanarayanan
2024-03-12 14:52 ` [PATCH 0/5] Handle set_memory_XXcrypted() errors in Hyper-V Kirill A. Shutemov
2024-04-10 21:34 ` Wei Liu
2024-04-11 21:07 ` Edgecombe, Rick P
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240311161558.1310-1-mhklinux@outlook.com \
--to=mhkelley58@gmail.com \
--cc=dave.hansen@linux.intel.com \
--cc=davem@davemloft.net \
--cc=decui@microsoft.com \
--cc=edumazet@google.com \
--cc=elena.reshetova@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=haiyangz@microsoft.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=kuba@kernel.org \
--cc=kys@microsoft.com \
--cc=linux-coco@lists.linux.dev \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhklinux@outlook.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=rick.p.edgecombe@intel.com \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=wei.liu@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).