From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF32EC3DA4A for ; Fri, 16 Aug 2024 21:51:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1E99A6B0110; Fri, 16 Aug 2024 17:51:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 14A678D00A2; Fri, 16 Aug 2024 17:51:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F2CEE6B0116; Fri, 16 Aug 2024 17:51:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id D304D6B0110 for ; Fri, 16 Aug 2024 17:51:17 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 347F2140366 for ; Fri, 16 Aug 2024 21:51:17 +0000 (UTC) X-FDA: 82459454994.20.9EF68AB Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by imf10.hostedemail.com (Postfix) with ESMTP id 6279DC000E for ; Fri, 16 Aug 2024 21:51:15 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=iMcT2Ejl; spf=pass (imf10.hostedemail.com: domain of dionnaglaze@google.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=dionnaglaze@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723845061; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=tSoLT3jQSFrMPTRX3f3E09PU/Z1+sySUyTnEKJFhXh4=; b=0qH7YTt6yC/iW3wJiJeREO4j/A4BjwNdrEW0/4F2PT+h399zSRKamCzh0ggc6+SULBQfDa NbWAr7UMBE8lIk/dWPHpD64H9+DIZYJ1hRTS+l1Qn6nwiOjVQeaq68a5Ytv4Vxybrimn1h Foh0AiadKxC3ITZvGKtrnjbg2gyEkJk= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=iMcT2Ejl; spf=pass (imf10.hostedemail.com: domain of dionnaglaze@google.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=dionnaglaze@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723845061; a=rsa-sha256; cv=none; b=BT7BCgJwynu78bi74JEBG/400vwL/1ygvELm92AtU86gsDeQIxDSmH7m478AbuAbTd+rcr XT6Rw5I2/Ui8tCnjudCpoFwGz0RezNkEGYWOCsJqgpBofYnhtF6044A57CMB0yr8SPwt1o XsqzE4Fuo2Hw+cIq9k0d90SUn3DGENE= Received: by mail-ej1-f47.google.com with SMTP id a640c23a62f3a-a83869373b6so222881766b.2 for ; Fri, 16 Aug 2024 14:51:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1723845074; x=1724449874; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=tSoLT3jQSFrMPTRX3f3E09PU/Z1+sySUyTnEKJFhXh4=; b=iMcT2EjlignCv/O6h/1KYM3P6ICfrU17LDYhmnhmlnpoJXSZIESRCVkKM2DAlTI6c7 hKVilYWZkFh0sOWzrGKLisnaJGT5EpUUbw0694mcwIOpz7WBhutlX+GL8KQ/U3TgNBkU 50vcLtnP9RlWtRg0afaodYxjyP6YV/8KIM2VyHVPAPujg3EgG5uLQhfvT3Ub/W48KZoW me7viu+Ilg9aVi1DirfvwxrseJqwCHjtxMVVtha65LZleZRcZTOn4Wt+Dunjn4PfijT3 cz9XOymj3lrGC1ACQXEPmh5T/qcAbNKLuFCpmtt6jPDpI2g21JOVPbnvBEguRntCzypM U1+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723845074; x=1724449874; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=tSoLT3jQSFrMPTRX3f3E09PU/Z1+sySUyTnEKJFhXh4=; b=e6tKJzi0t+w2fhemHRVHWCxlmP493JuAq17yPtCDlz0PXyFTWj0wVy/hke+w893Q3V qo+3YYXral6aVuouk1Y2McnSSc0saMedDCBr37XGOtrkeStTbDy9Vw8fJ5FPioZu9FwG xf+zsYTx4bKwG/b7TCtBMGZL/WV8Xfk82Yo1fmLg5/1pgsV0vwNbycna8MS2OZICZnSW cqICWEHBk/PMOY16RrtfN6CdNiU5vmBeCubElAb82cVTLLdu8E1JCVTqYHbAoNVthL8L xx2xglPwE4RbNMQ9Xalu0DO44j+qTs6vL0KDcbi2ufkUfHQTiRGqvdwd1DVoAYQU/5fx 6JQw== X-Forwarded-Encrypted: i=1; AJvYcCV+V+JZ1JInjIKq9PizvGDjH9IUoJCRw1tZ5uPCOlO5yfzlixFNWdUsEuOKmaoLTyu8vM/lCgghpWhpeSAHs36aQV8= X-Gm-Message-State: AOJu0YwyYu7VQb1qxevkOrPmhP4nRZBVhScL+rJFCOIBnowRm+/NETJI pI5/xj7B5MkaW/eTZsWnuaj/FeZrkzH8NIikcqZIP/nc8giapbXN43xFYhBHxq5jpsSa4wdcjUb 5Js0zHvQxzHN3YDxR2mSlWbSFoLP9P10kOryj X-Google-Smtp-Source: AGHT+IEmc4kaoUmyndexzsa7RrjVkpBN/O6EN0guPJoo2EGRzlWVfpPIGDGKEfKmK4pOF9srqCEqaC5Eq+aT59GDm8Y= X-Received: by 2002:a17:907:e2c3:b0:a7a:b1a8:6a2e with SMTP id a640c23a62f3a-a839292ff2bmr297152766b.28.1723845073435; Fri, 16 Aug 2024 14:51:13 -0700 (PDT) MIME-Version: 1.0 References: <20240515012552.801134-1-michael.roth@amd.com> In-Reply-To: <20240515012552.801134-1-michael.roth@amd.com> From: Dionna Amalie Glaze Date: Fri, 16 Aug 2024 14:50:59 -0700 Message-ID: Subject: Re: [PATCH] KVM: SEV: Replace KVM_EXIT_VMGEXIT with KVM_EXIT_SNP_REQ_CERTS To: Michael Roth Cc: seanjc@google.com, kvm@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, jroedel@suse.de, thomas.lendacky@amd.com, hpa@zytor.com, ardb@kernel.org, pbonzini@redhat.com, vkuznets@redhat.com, jmattson@google.com, luto@kernel.org, dave.hansen@linux.intel.com, slp@redhat.com, pgonda@google.com, peterz@infradead.org, srinivas.pandruvada@linux.intel.com, rientjes@google.com, dovmurik@linux.ibm.com, tobin@ibm.com, bp@alien8.de, vbabka@suse.cz, kirill@shutemov.name, ak@linux.intel.com, tony.luck@intel.com, sathyanarayanan.kuppuswamy@linux.intel.com, alpergun@google.com, jarkko@kernel.org, ashish.kalra@amd.com, nikunj.dadhania@amd.com, pankaj.gupta@amd.com, liam.merwick@oracle.com, papaluri@amd.com Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Stat-Signature: segugxhzfp4hj7q7o48ufrrjcrkj1ybs X-Rspamd-Queue-Id: 6279DC000E X-Rspamd-Server: rspam11 X-HE-Tag: 1723845075-383126 X-HE-Meta: U2FsdGVkX19jzQLsPMs22MzpxnGbJ+bXf6GQstupTKflK6naRs6AeRZre/fL/kSBHUDfN+E2+eJfYtAVbRNTkw0BkwGH/fRF1TOQ1IRZtM9plQSNs13pIRudgCzvOgVT/v0cLAsdaNzxZCbcvFuM7SyEAbx/7x6JiqMocfvhIRE0V5iuO3D18+FfctdDEFOYYD+knD+MG2GPDnVpsbqD5jW6JG3QTltfsIOeNArASvWfDUn13pt4JiexTGLZiWdIKKNZveTDVsv8x5+q6qJdWLCEElD9uvOQndBJkvM7H5WT9uqbiUieglldTrkCFA/fB/Iw/HwmRfhMBH0YfH0Hx2nkc7+hRUQheZnCKvadNsvwVDau9yQ0pXB6ECrZrXGJwv4jcmS4m6nwaZbCTE5NBAWwJPsCYn2Nbju9xIjkfLfxUog6R1g8O9dPmb0R41zhkIney4S8ts5Rqhpu+6qJ1nEMURXhroE9HW6Xx/mnJFUQ+ckNLCWTEkSnl3sd3W8k1fmkXf6X9wa3tSldxsRb0UYxSP3bHfBR62TM4QuEllapwV39sUJGkhbgNbujvmhq1c7sx+MaJiVWcemGs8sBUmEcRu8oRJpEIAVAAAHIEWvEAkDWKeDMUGmVqaOsuGWlJwp0WNMHVZ5ZJiaGC0AoejllOHB2dBWz8ESogfNmi/UvpuzEcGs06z1hxVbLvoGoGb2/49vJhI/j5XRc/ADYteYOGo4qhsJLxZiu0Wy1RHFrIOYGVDAwmRLoJj4PV8RKJ1HjjYeHjsfhFZNQmfeHzpvmjORCmw22ew8knhvv7EdmPAkK8D/JTlCLYN2NGMMjaBOiXjeqigx4MXhFzn4bSuG8mgzUnxLlABYROAqORq+jDqutrHvBOYPYN9ygy6i+vznWlqiJFxem2Wt/LCWny6BLpvlJ6W5dj9iH9XP1yY56DvmCCL+cfSxoMuG7IUMCnpnAy/Wb7CrsOVJKDFc /ekl/uR3 vo0Lk0lmV5MuLkheF26ULdYTDkQjzxwveTWWIikiIRgnpzKMKjpEyapAhFjMK+raxI2q1FG/j2wq/IEHc8nvQsLf6ts1dBcfK7cEjGwyP6XXDC53fE1jpyVj0BPQjN1zn+4mVGgCqCEPxiVJ1SfvR4Jqf1W+JNdy4tN0RvxIUUq33rl3qkRutihya7OfH2UJ3gjcY/B197FomsgdJf3H8Ur5I/Q== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000256, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -4006,21 +4006,14 @@ static int snp_complete_ext_guest_req(struct kvm_vcpu *vcpu) > sev_ret_code fw_err = 0; > int vmm_ret; > > - vmm_ret = vcpu->run->vmgexit.req_certs.ret; > + vmm_ret = vcpu->run->snp_req_certs.ret; > if (vmm_ret) { > if (vmm_ret == SNP_GUEST_VMM_ERR_INVALID_LEN) > vcpu->arch.regs[VCPU_REGS_RBX] = > - vcpu->run->vmgexit.req_certs.data_npages; > + vcpu->run->snp_req_certs.data_npages; > goto out; > } Finally getting around to this patch. Thanks for your patience. Whether the exit to guest for certs is first or second when getting the attestation report, the certificates need to be consistent. Since we don't have any locks held before exiting, and no checks happening on the result, there's a chance that a well-intentioned host can still provide the wrong certificate to the guest when VMs are running and requesting attestations during a firmware hotload. Thread 1: DOWNLOAD_FIRMWARE_EX please CURRENT_TCB > REPORTED_TCB (notify service to get a new VCEK cert) Interrupted Thread 2: VM extended guest request in. Exit to user space Call SNP_PLATFORM_STATUS to get REPORTED_TCB. Get certs for REPORTED_TCB for the blob. It's at /x/y/z-REPORTED_TCB.crt. Interrupted Thread 1: I got my VCEK cert delivered for CURRENT_TCB! I'll put it at /x/y/z-CURRENT_TCB.crt Great. SNP_COMMIT. Now both REPORTED_TCB and COMMITTED_TCB to CURRENT_TCB, because that's the spec. Different reported_tcb here. than in thread 1. Interrupted Thread 2: Get the attestation report. It will be signed by the VCEK versioned to the newer REPORTED_TCB. Return to VM guest VM guest: My report's signature doesn't verify with the VCEK cert I was given. Yes, 1-88-COM-PLAIN? How do we avoid this? 1. We can advise that the guest parses the certificate and the attestation report to determine if their TCBs match expectations and retry if they're different because of a bad luck data race. 2. We can add a new global lock that KVM holds from CCP similar to sev_cmd_lock to sequentialize req_certs, attestation reports, and SNP_COMMIT. KVM releases the lock before returning to the guest. SNP_COMMIT must now hold this lock before attempting to grab the sev_cmd_lock. I think probably 2 is better. > > @@ -4060,12 +4045,9 @@ static int snp_begin_ext_guest_req(struct kvm_vcpu *vcpu) > * Grab the certificates from userspace so that can be bundled with > * attestation/guest requests. > */ > - vcpu->run->exit_reason = KVM_EXIT_VMGEXIT; > - vcpu->run->vmgexit.type = KVM_USER_VMGEXIT_REQ_CERTS; > - vcpu->run->vmgexit.req_certs.data_gpa = data_gpa; > - vcpu->run->vmgexit.req_certs.data_npages = data_npages; > - vcpu->run->vmgexit.req_certs.flags = 0; > - vcpu->run->vmgexit.req_certs.status = KVM_USER_VMGEXIT_REQ_CERTS_STATUS_PENDING; > + vcpu->run->exit_reason = KVM_EXIT_SNP_REQ_CERTS; This should be whatever exit reason #define you go with (40), not the (1) you defined for kvm_snp_exit. > + vcpu->run->snp_req_certs.data_gpa = data_gpa; > + vcpu->run->snp_req_certs.data_npages = data_npages; > vcpu->arch.complete_userspace_io = snp_complete_ext_guest_req; > > return 0; /* forward request to userspace */ > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 106367d87189..8ebfc91dc967 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -135,22 +135,17 @@ struct kvm_xen_exit { > } u; > }; > > -struct kvm_user_vmgexit { > -#define KVM_USER_VMGEXIT_REQ_CERTS 1 > - __u32 type; /* KVM_USER_VMGEXIT_* type */ > +struct kvm_exit_snp { > +#define KVM_EXIT_SNP_REQ_CERTS 1 > + __u32 type; /* KVM_EXIT_SNP_* type */ I think this whole struct should be removed since we're only doing the one exit reason. This is unused. You also double-#define the return value preprocessor directives. > }; > @@ -198,7 +193,7 @@ struct kvm_user_vmgexit { > #define KVM_EXIT_NOTIFY 37 > #define KVM_EXIT_LOONGARCH_IOCSR 38 > #define KVM_EXIT_MEMORY_FAULT 39 > -#define KVM_EXIT_VMGEXIT 40 > +#define KVM_EXIT_SNP_REQUEST_CERTS 40 Probably we should just make this KVM_EXT_SNP_REQ_CERTS so the rest of the code works. > > /* For KVM_EXIT_INTERNAL_ERROR */ > /* Emulate instruction failed. */ > @@ -454,8 +449,16 @@ struct kvm_run { > __u64 gpa; > __u64 size; > } memory_fault; > - /* KVM_EXIT_VMGEXIT */ > - struct kvm_user_vmgexit vmgexit; > + /* KVM_EXIT_SNP_REQ_CERTS */ > + struct { > + __u64 data_gpa; > + __u64 data_npages; > +#define KVM_EXIT_SNP_REQ_CERTS_ERROR_INVALID_LEN 1 > +#define KVM_EXIT_SNP_REQ_CERTS_ERROR_BUSY 2 > +#define KVM_EXIT_SNP_REQ_CERTS_ERROR_GENERIC (1 << 31) > + __u32 ret; > + } snp_req_certs; > + > /* Fix the size of the union. */ > char padding[256]; > }; > -- > 2.25.1 > > -- -Dionna Glaze, PhD, CISSP, CCSP (she/her)