From: Paul Meyer <katexochen0@gmail.com>
Date: Wed, 17 Jun 2026 14:13:35 +0000 (+0200)
Subject: confidential-virt: treat an unreadable SEV MSR as confidential
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d76ce200a61591069f47710e8b50383776968946;p=thirdparty%2Fsystemd.git

confidential-virt: treat an unreadable SEV MSR as confidential

msr() returned 0 on failure, indistinguishable from a real MSR value of
0. With /dev/cpu/0/msr unavailable (e.g. the msr module not loaded in the
initrd), detect_sev() read 0 and reported a genuine SEV/-ES/-SNP guest as
CONFIDENTIAL_VIRTUALIZATION_NONE.

That inverts the firmware-credential trust gate: import_credentials_*()
skip fw_cfg/SMBIOS credentials only when detect_confidential_virtualization()
is > 0 ("don't trust firmware in confidential VMs"). A false NONE makes a
confidential guest trust and import credentials injected by the untrusted
hypervisor.

msr() now returns a negative errno, and detect_sev() assumes plain SEV when
the MSR is unreadable but CPUID already advertised SEV under a hypervisor,
so the gate still trips.

The conservative branch only fires when CPUID already advertised SEV, i.e.
for a guest the hypervisor marked SEV-capable. QEMU gates that CPUID leaf on
the SEV launch object and does not expose it to ordinary guests even under
-cpu host, so it does not misfire for non-confidential guests. Were a
hypervisor to expose the bit anyway the outcome is fail-safe (we only
decline to trust firmware-supplied data); nothing in-tree branches on the
specific SEV tier.

Signed-off-by: Paul Meyer <katexochen0@gmail.com>
---

diff --git a/src/basic/confidential-virt.c b/src/basic/confidential-virt.c
index 29d598a5988..3f8cb2a3cd8 100644
--- a/src/basic/confidential-virt.c
+++ b/src/basic/confidential-virt.c
@@ -43,37 +43,32 @@ static uint32_t cpuid_leaf(uint32_t eax, char ret_sig[static 13], bool swapped)
 
 #define MSR_DEVICE "/dev/cpu/0/msr"
 
-static uint64_t msr(uint64_t index) {
-        uint64_t ret;
-        ssize_t rv;
+static int msr(uint64_t index, uint64_t *ret) {
         _cleanup_close_ int fd = -EBADF;
+        uint64_t v;
+        ssize_t n;
 
-        fd = open(MSR_DEVICE, O_RDONLY|O_CLOEXEC);
-        if (fd < 0) {
-                log_debug_errno(errno,
-                                "Cannot open MSR device %s (index %" PRIu64 "), ignoring: %m",
-                                MSR_DEVICE,
-                                index);
-                return 0;
-        }
+        assert(ret);
 
-        rv = pread(fd, &ret, sizeof(ret), index);
-        if (rv < 0) {
-                log_debug_errno(errno,
-                                "Cannot read MSR device %s (index %" PRIu64 "), ignoring: %m",
-                                MSR_DEVICE,
-                                index);
-                return 0;
-        } else if (rv != sizeof(ret)) {
-                log_debug("Short read %zd bytes from MSR device %s (index %" PRIu64 "), ignoring",
-                          rv,
-                          MSR_DEVICE,
-                          index);
-                return 0;
-        }
-
-        log_debug("MSR %" PRIu64 " result %" PRIu64 "", index, ret);
-        return ret;
+        fd = open(MSR_DEVICE, O_RDONLY|O_CLOEXEC);
+        if (fd < 0)
+                return log_debug_errno(errno,
+                                       "Cannot open MSR device %s (index %" PRIu64 "): %m",
+                                       MSR_DEVICE, index);
+
+        n = pread(fd, &v, sizeof(v), index);
+        if (n < 0)
+                return log_debug_errno(errno,
+                                       "Cannot read MSR device %s (index %" PRIu64 "): %m",
+                                       MSR_DEVICE, index);
+        if (n != sizeof(v))
+                return log_debug_errno(SYNTHETIC_ERRNO(EIO),
+                                       "Short read %zd bytes from MSR device %s (index %" PRIu64 ")",
+                                       n, MSR_DEVICE, index);
+
+        log_debug("MSR %" PRIu64 " result %" PRIu64, index, v);
+        *ret = v;
+        return 0;
 }
 
 static bool detect_hyperv_cvm(uint32_t isoltype) {
@@ -110,6 +105,7 @@ static bool detect_hyperv_cvm(uint32_t isoltype) {
 static ConfidentialVirtualization detect_sev(void) {
         uint32_t eax, ebx, ecx, edx;
         uint64_t msrval;
+        int r;
 
         eax = CPUID_GET_HIGHEST_FUNCTION;
         ebx = ecx = edx = 0;
@@ -140,7 +136,15 @@ static ConfidentialVirtualization detect_sev(void) {
                 return CONFIDENTIAL_VIRTUALIZATION_NONE;
         }
 
-        msrval = msr(MSR_AMD64_SEV);
+        r = msr(MSR_AMD64_SEV, &msrval);
+        if (r < 0) {
+                /* The CPU advertises SEV support and we're running under a hypervisor, but we couldn't read
+                 * the SEV MSR to determine the exact mode (e.g. /dev/cpu/0/msr is unavailable because the
+                 * msr module isn't loaded). Assume plain SEV. Misreporting a genuine confidential guest as
+                 * non-confidential would wrongly make us trust hypervisor-provided data such as firmware credentials. */
+                log_debug_errno(r, "Failed to read SEV MSR, assuming SEV: %m");
+                return CONFIDENTIAL_VIRTUALIZATION_SEV;
+        }
 
         /* Test reverse order, since the SEV-SNP bit implies
          * the SEV-ES bit, which implies the SEV bit */