Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <ac496e47-c949-0e9d-4735-d51a7c9c0f62@oracle.com>
Date:   Thu, 20 Jan 2022 16:11:02 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 7/9] KVM: SVM: Inject #UD on attempted emulation for SEV
 guest w/o insn buffer
Content-Language: en-GB
To:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>
Cc:     Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Brijesh Singh <brijesh.singh@amd.com>,
        Liam Merwick <liam.merwick@oracle.com>
References: <20220120010719.711476-1-seanjc@google.com>
 <20220120010719.711476-8-seanjc@google.com>
From:   Liam Merwick <liam.merwick@oracle.com>
In-Reply-To: <20220120010719.711476-8-seanjc@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TDZvY1lybiswMmVXOEkxMExra1VsZ3JjdVZYNi9YOWsvOWpwVjRaT1h3eitV?=
 =?utf-8?B?U2IzR1dmcEFWVmpqV3pONXJ2TURxWVFQeHF2TWZuMGFtd3h1MzFrcFFCWG4z?=
 =?utf-8?B?ODV1cHg1RGl6aE1zZXg0UkIwOEFlcGlqOENYMjdVdzhzTHRmMExreWZEOUxO?=
 =?utf-8?B?bFZ3UUs5QjdmTlJwOThNcG9oVkd2VDErU0FDRU9JaEJQL3NibEc1SGV0aVI5?=
 =?utf-8?B?WmJZMkx0T1NRQ0p6b3FhTXRmeHVDd0JSK2IzakY0d0RWeUZxNzVNTnBSdExL?=
 =?utf-8?B?eFd5TXYwK0lWRnAzTkhyZTcxTEgyVU9tNmJRU1BmWWNLTzJlUlh5TTloT1BR?=
 =?utf-8?B?SkMxdnFKK3FOZG9VNjRVWGtpemdOSmxjQVUwZVFlS29rSjNOMDU5ak1ub1Ry?=
 =?utf-8?B?eVB4N1YwVDZoMWZRYTU1ZENtYlVEblM5RWVib3Fid1E5aXQxQnU2OUhvbDk5?=
 =?utf-8?B?WXRBVzA3ajNYeHBLUFV0dEpMaDNjTEVaYWNiRTNqRjJrYXMrY2hrR1J6dThE?=
 =?utf-8?B?bXY5ZUc0c3IvQTBtNytxeCtST3Jjd3VGQitES3g2NXBPOVhCR3pocEpwai9q?=
 =?utf-8?B?RTJHWUxUUWprU3NEOFU4ek9yZEJEZGxnQkp3QlJQY3NTcHBvOTNGVFRaaUo2?=
 =?utf-8?B?WnN4cDIzRlZBZ0sweDcxcWc0cTcxcktFV3hHRitLd09WcXRXdFZnQ0l4SjZB?=
 =?utf-8?B?a1lyYXFVd29zRDVKVDhYVGV4cUNiSEVReEtkNjVNYVlWKzkveXhhMDVpdjV4?=
 =?utf-8?B?STIzVGZmcEpMYU5jMHY3blYzaFF1WnJLYlRzREIwSy9GTTB6aFRYSUYvTngy?=
 =?utf-8?B?bmhvUnNpZVFMTkttZUtYZTY3djMxMkREVi93YVBLZWRTMjRkQWRJU0tzRWdn?=
 =?utf-8?B?RGllcVp2aStWeExQWmhFQWt1cXlUQnZOTlR4dS9TY2FzY2t3LzFsTHFTWTht?=
 =?utf-8?B?Q0JvZjI2L2I2Zkl3amJqRHkrWUVyanUrQXpQWnR1eTZCSjdmSmxQNW5iZHhV?=
 =?utf-8?B?TDNEUW1jTFhieGpJRGdodEQ0UGE0TFNOUHdXeURxWFJNNHc1S21CTzNCSVdF?=
 =?utf-8?B?MlV0bzNRVlRKbE12UmNsODl4cGJPTEVDUk5NMjVmYUtKRkphQktwdktsYWI3?=
 =?utf-8?B?aElBYVlnU1RrNzN5Ky9Dc1Y2ZTZRZVNsVG9qN1JxbHRNTlVyaGVaWVBjRllk?=
 =?utf-8?B?MENOMHd4VGhsUlluS2licnpaY2JSTXBrZ0xNOUgvekx0ekE1RGhScVRDVktY?=
 =?utf-8?B?TVNrcU16T2kzVVNwZzdhRmlUS2E0QUVYc05LNkhoejJPdTA4cng3ZzNXcUpp?=
 =?utf-8?B?elM1VEt1MXU1RUR5UGZWcjB3Q0hMbmEzYmxCSVcxL2FNZnFaT1U0UUR6N0RL?=
 =?utf-8?B?T2VJNll6cTRvZlZzYXBiSUdFVzgwTzJtSUdGeEd6TjY2L3ptMUtKWXR4aDBM?=
 =?utf-8?B?R3Z1eVcxdW5JZ3QrUzVsY2xvSVZpeC8yQlJBTER2RkQzejkxRTBKcXJCMitQ?=
 =?utf-8?B?Y2s2Ukh1UnVmNkdYbWlyRUZ1YXZwVzJoZXlPbCtiLzNTSGJIbkQzallLaVhE?=
 =?utf-8?B?b2RpWHhEVStITXNHK1FOYWx2WEJDM25WRENpdlBpaVhabWllakJyTWFuK3Q5?=
 =?utf-8?B?c09sVjdzakZDNFhzVThaaGNpcGtDUUVGblMzSXpzVnM5TFVoMGZPZVhZNWdM?=
 =?utf-8?B?Vlkxb2xpQlF4SlRMdE9mb0pYQ3VjQVYyZ1JTQmE5b2UxTzBWdnhHa2x3TUYz?=
 =?utf-8?B?MjVNMWZNYW5EWnFHa292V2JpMXFCRFNRUjVkdHZBOVI1NGZ1L1hKUDNhSWNj?=
 =?utf-8?B?WnpEK2hxMkNrd1ptK3dMUFQ3dGV0UU9RYTVESVhTTy9zR2pmNGVnMUtSYkxy?=
 =?utf-8?B?QzQ4K3dtQXVQcUNYZlVFYjh2ZWg0TFhWRGFBVDQrTjVEWGp6L3pWRE5Xd2pX?=
 =?utf-8?B?K0s2Y0ZKZkxFeVMyc3lsZ2F2K21WL0lxSHJ2d0xZckthWGh6RWtKWW8wUjhW?=
 =?utf-8?B?TUd4L2sxbllwcmdjb0U4eE5NZmdJdlZPTk0xNWttWWhqd1JmcXJLWTQ0cDlM?=
 =?utf-8?B?dkk0Q09LMy9GUTMwWFYzTE1nYmRmakNIcmZnVzdxekRPalY2b0dqeXEvN2tm?=
 =?utf-8?B?RlNkNnVoNjFzRnV2UEtzWUIzRFh2UmtpcFFZSVk3aXNKRnZROXFrbWdYZnNL?=
 =?utf-8?Q?rRtuOmZcV21atz/x8sdKbUw=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1365683a-ccd0-435b-1958-08d9dc2f782b
X-MS-Exchange-CrossTenant-AuthSource: DS7PR10MB5038.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jan 2022 16:11:08.9799
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: C+CK4N+UOaG0T9n7ym2VM4jwuPLSaYDtGn/VcHvBoFWnigbl4VwpNxQJzg1e1ddm7BA1O2aVuzRLA2uWzbgwDA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR10MB3123
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10233 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 bulkscore=0
 malwarescore=0 adultscore=0 phishscore=0 mlxscore=0 spamscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2201200083
X-Proofpoint-GUID: vXNG9d46771HpImfWZDTqw6zQpupjTo3
X-Proofpoint-ORIG-GUID: vXNG9d46771HpImfWZDTqw6zQpupjTo3
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 20/01/2022 01:07, Sean Christopherson wrote:
> Inject #UD if KVM attempts emulation for an SEV guests without an insn
> buffer and instruction decoding is required.  The previous behavior of
> allowing emulation if there is no insn buffer is undesirable as doing so
> means KVM is reading guest private memory and thus decoding cyphertext,
> i.e. is emulating garbage.  The check was previously necessary as the
> emulation type was not provided, i.e. SVM needed to allow emulation to
> handle completion of emulation after exiting to userspace to handle I/O.
> 

A few cyphertext references...

> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Liam Merwick <liam.merwick@oracle.com>

> ---
>   arch/x86/kvm/svm/svm.c | 89 ++++++++++++++++++++++++++----------------
>   1 file changed, 55 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index ed2ca875b84b..d324183fc596 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4277,49 +4277,70 @@ static bool svm_can_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type,
>   	if (sev_es_guest(vcpu->kvm))
>   		return false;
>   
> +	/*
> +	 * Emulation is possible if the instruction is already decoded, e.g.
> +	 * when completing I/O after returning from userspace.
> +	 */
> +	if (emul_type & EMULTYPE_NO_DECODE)
> +		return true;
> +
> +	/*
> +	 * Emulation is possible for SEV guests if and only if a prefilled
> +	 * buffer containing the bytes of the intercepted instruction is
> +	 * available. SEV guest memory is encrypted with a guest specific key
> +	 * and cannot be decrypted by KVM, i.e. KVM would read cyphertext and
> +	 * decode garbage.
> +	 *
> +	 * Inject #UD if KVM reached this point without an instruction buffer.
> +	 * In practice, this path should never be hit by a well-behaved guest,
> +	 * e.g. KVM doesn't intercept #UD or #GP for SEV guests, but this path
> +	 * is still theoretically reachable, e.g. via unaccelerated fault-like
> +	 * AVIC access, and needs to be handled by KVM to avoid putting the
> +	 * guest into an infinite loop.   Injecting #UD is somewhat arbitrary,
> +	 * but its the least awful option given lack of insight into the guest.
> +	 */
> +	if (unlikely(!insn)) {
> +		kvm_queue_exception(vcpu, UD_VECTOR);
> +		return false;
> +	}
> +
> +	/*
> +	 * Emulate for SEV guests if the insn buffer is not empty.  The buffer
> +	 * will be empty if the DecodeAssist microcode cannot fetch bytes for
> +	 * the faulting instruction because the code fetch itself faulted, e.g.
> +	 * the guest attempted to fetch from emulated MMIO or a guest page
> +	 * table used to translate CS:RIP resides in emulated MMIO.
> +	 */
> +	if (likely(insn_len))
> +		return true;
> +
>   	/*
>   	 * Detect and workaround Errata 1096 Fam_17h_00_0Fh.
>   	 *
>   	 * Errata:
> -	 * When CPU raise #NPF on guest data access and vCPU CR4.SMAP=1, it is
> -	 * possible that CPU microcode implementing DecodeAssist will fail
> -	 * to read bytes of instruction which caused #NPF. In this case,
> -	 * GuestIntrBytes field of the VMCB on a VMEXIT will incorrectly
> -	 * return 0 instead of the correct guest instruction bytes.
> -	 *
> -	 * This happens because CPU microcode reading instruction bytes
> -	 * uses a special opcode which attempts to read data using CPL=0
> -	 * privileges. The microcode reads CS:RIP and if it hits a SMAP
> -	 * fault, it gives up and returns no instruction bytes.
> +	 * When CPU raises #NPF on guest data access and vCPU CR4.SMAP=1, it is
> +	 * possible that CPU microcode implementing DecodeAssist will fail to
> +	 * read guest memory at CS:RIP and vmcb.GuestIntrBytes will incorrectly
> +	 * be '0'.  This happens because microcode reads CS:RIP using a _data_
> +	 * loap uop with CPL=0 privileges.  If the load hits a SMAP #PF, ucode
> +	 * gives up and does not fill the instruction bytes buffer.
>   	 *
>   	 * Detection:
> -	 * We reach here in case CPU supports DecodeAssist, raised #NPF and
> -	 * returned 0 in GuestIntrBytes field of the VMCB.
> -	 * First, errata can only be triggered in case vCPU CR4.SMAP=1.
> -	 * Second, if vCPU CR4.SMEP=1, errata could only be triggered
> -	 * in case vCPU CPL==3 (Because otherwise guest would have triggered
> -	 * a SMEP fault instead of #NPF).
> -	 * Otherwise, vCPU CR4.SMEP=0, errata could be triggered by any vCPU CPL.
> -	 * As most guests enable SMAP if they have also enabled SMEP, use above
> -	 * logic in order to attempt minimize false-positive of detecting errata
> -	 * while still preserving all cases semantic correctness.
> +	 * KVM reaches this point if the VM is an SEV guest, the CPU supports
> +	 * DecodeAssist, a #NPF was raised, KVM's page fault handler triggered
> +	 * emulation (e.g. for MMIO), and the CPU returned 0 in GuestIntrBytes
> +	 * field of the VMCB.
>   	 *
> -	 * Workaround:
> -	 * To determine what instruction the guest was executing, the hypervisor
> -	 * will have to decode the instruction at the instruction pointer.
> +	 * This does _not_ mean that the erratum has been encountered, as the
> +	 * DecodeAssist will also fail if the load for CS:RIP hits a legitimate
> +	 * #PF, e.g. if the guest attempt to execute from emulated MMIO and
> +	 * encountered a reserved/not-present #PF.
>   	 *
> -	 * In non SEV guest, hypervisor will be able to read the guest
> -	 * memory to decode the instruction pointer when insn_len is zero
> -	 * so we return true to indicate that decoding is possible.
> -	 *
> -	 * But in the SEV guest, the guest memory is encrypted with the
> -	 * guest specific key and hypervisor will not be able to decode the
> -	 * instruction pointer so we will not able to workaround it. Lets
> -	 * print the error and request to kill the guest.
> +	 * To reduce the likelihood of false positives, take action if and only
> +	 * if CR4.SMAP=1 (obviously required to hit the erratum) and CR4.SMEP=0
> +	 * or CPL=3.  If SMEP=1 and CPL!=3, the erratum cannot have been hit as
> +	 * the guest would have encountered a SMEP violation #PF, not a #NPF.
>   	 */
> -	if (likely(!insn || insn_len))
> -		return true;
> -
>   	cr4 = kvm_read_cr4(vcpu);
>   	smep = cr4 & X86_CR4_SMEP;
>   	smap = cr4 & X86_CR4_SMAP;