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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 93B68C433F5 for ; Tue, 11 Jan 2022 04:21:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347730AbiAKEUk (ORCPT ); Mon, 10 Jan 2022 23:20:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59256 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229952AbiAKEUj (ORCPT ); Mon, 10 Jan 2022 23:20:39 -0500 Received: from mail-pl1-x62a.google.com (mail-pl1-x62a.google.com [IPv6:2607:f8b0:4864:20::62a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 20E25C06173F; Mon, 10 Jan 2022 20:20:39 -0800 (PST) Received: by mail-pl1-x62a.google.com with SMTP id x15so15272700plg.1; Mon, 10 Jan 2022 20:20:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:content-language:to:cc :references:from:organization:subject:in-reply-to :content-transfer-encoding; bh=Tto62C+VWnwHMjIZEt/IkjdeF/hUNskHa7hE32WUZWo=; b=nGTZvLPVZhT+r0RT2Y/D+I8RG8YKSzE/wpDhxsSXL6ToEwD5+XL24LyirAjpM9Udi8 M0+Td9ZDbJ1+oHivltWchrBBw2jO++RQz2s52zf4kUd5YJHWJxUKAu/o5J9xjIAuyryW +0hb2hMmoG3/uUR6T8yAwSptn6mFucCIGrQaqg2VJXlruJLi/e/FT+PTPouezh2VM7C+ sDNMmHgwIqW35Cgda6qPlN7ntOKaCY+XkpkQB44SIfjIo+KjNjcd2c7bIOhhyR4GVS+u uQynCn/bUcfM9q5nIkrNqkOUcj2yafqb2PC3TkFs53nNAJQR+86jy6S7njJPOlk3+HcR aSQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:organization:subject :in-reply-to:content-transfer-encoding; bh=Tto62C+VWnwHMjIZEt/IkjdeF/hUNskHa7hE32WUZWo=; b=JYOlp+PYfZNqw/P2P7SthPyOFzvoFV5ka7wKy44oIPsL8RmmbbbQKEqezCiNit7rP4 7sZVGtVHzbJlamueyBoBO+vd7Ir0jHc8+YFtzYfzlvWY2/MSLW/UJLyGmEQU6mGjUOif Q6Ybgw+v2PHxzg65mKOE4CtSX64rfCpauWe8ysaj0ux9+mQzXANvrBU9RnHolvqbG43J jp4fSqXxVAZJS+jfuTGlWGqehDgGmaN8n5WuLEnwS32n+1lAK1+rD+Wu2FsVTIRd8AkB 498ONCBbAUA6xpwwKscajZOfwKkuc41OB4QiaydGybiUOHouIywdnkx8J3RYkLh9nkWN AjQQ== X-Gm-Message-State: AOAM5333N4bQlnKWfc1/+mq+DTLvOtG2r14uSj3qfisx19Af/isTAmZu Bg/dDo3Ca80kU858YQptm9w= X-Google-Smtp-Source: ABdhPJyCeAzKwRCAJCdxXI9tSlZrQtzB5EZyg4yJJ/9hoSTjTGV3tnt7nvk8rmjjXPCEDgSuyAt0qw== X-Received: by 2002:a17:90a:5893:: with SMTP id j19mr1198018pji.30.1641874838514; Mon, 10 Jan 2022 20:20:38 -0800 (PST) Received: from [192.168.255.10] ([103.7.29.32]) by smtp.gmail.com with ESMTPSA id w5sm8460312pfu.214.2022.01.10.20.20.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 10 Jan 2022 20:20:38 -0800 (PST) Message-ID: <80b40829-0d25-eb84-7bd7-f21685daeb20@gmail.com> Date: Tue, 11 Jan 2022 12:20:29 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Content-Language: en-US To: Sean Christopherson Cc: Paolo Bonzini , Jim Mattson , Wanpeng Li , Vitaly Kuznetsov , Joerg Roedel , x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20220110034747.30498-1-likexu@tencent.com> From: Like Xu Organization: Tencent Subject: Re: [PATCH v2] KVM: x86/pt: Ignore all unknown Intel PT capabilities In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/1/2022 8:57 am, Sean Christopherson wrote: > On Mon, Jan 10, 2022, Like Xu wrote: >> From: Like Xu >> >> Some of the new Intel PT capabilities (e.g. SDM Vol3, 32.2.4 Event >> Tracing, it exposes details about the asynchronous events, when they are >> generated, and when their corresponding software event handler completes >> execution) cannot be safely and fully emulated by the KVM, especially >> emulating the simultaneous writing of guest PT packets generated by >> the KVM to the guest PT buffer. >> >> For KVM, it's better to advertise currently supported features based on >> the "static struct pt_cap_desc" implemented in the host PT driver and >> ignore _all_ unknown features before they have been investigated one by >> one and supported in a safe manner, leaving the rest as system-wide-only >> tracing capabilities. >> >> Suggested-by: Paolo Bonzini >> Signed-off-by: Like Xu >> --- >> v1 -> v2 Changelog: >> - Be safe and ignore _all_ unknown capabilities. (Paolo) >> >> Previous: >> https://lore.kernel.org/kvm/20220106085533.84356-1-likexu@tencent.com/ >> >> arch/x86/kvm/cpuid.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >> index 0b920e12bb6d..439b93359848 100644 >> --- a/arch/x86/kvm/cpuid.c >> +++ b/arch/x86/kvm/cpuid.c >> @@ -901,6 +901,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) >> break; >> } >> >> + /* It's better to be safe and ignore _all_ unknown capabilities. */ > > No need to justify why unknown capabilities are hidden as that's very much (supposed > to be) standard KVM behavior. > >> + entry->ebx &= GENMASK(5, 0); > > Please add a #define somewhere so that this is self-documenting, e.g. see > KVM_SUPPORTED_XCR0. How about we define this macro in the so that the next PT capability enabler can update the mask with minimal effort, considering that many pure kernel developers don't care about KVM code ? > > And why just EBX? ECX appears to enumerate features too, and EDX is presumably > reserved to enumerate yet more features when EBX/ECX run out of bits. Yes, how about this version: diff --git a/arch/x86/include/asm/intel_pt.h b/arch/x86/include/asm/intel_pt.h index ebe8d2ea44fe..da94d0eeb9df 100644 --- a/arch/x86/include/asm/intel_pt.h +++ b/arch/x86/include/asm/intel_pt.h @@ -24,6 +24,12 @@ enum pt_capabilities { PT_CAP_psb_periods, }; +#define GUEST_SUPPORTED_CPUID_14_EBX \ + (BIT(0) | BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5)) + +#define GUEST_SUPPORTED_CPUID_14_ECX \ + (BIT(0) | BIT(1) | BIT(2) | BIT(3) | BIT(31)) + #if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_CPU_SUP_INTEL) void cpu_emergency_stop_pt(void); extern u32 intel_pt_validate_hw_cap(enum pt_capabilities cap); diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 0b920e12bb6d..be8c9170f98e 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -19,6 +19,7 @@ #include #include #include +#include #include "cpuid.h" #include "lapic.h" #include "mmu.h" @@ -900,7 +901,10 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) entry->eax = entry->ebx = entry->ecx = entry->edx = 0; break; } - + entry->eax = min(entry->eax, 1u); + entry->ebx &= GUEST_SUPPORTED_CPUID_14_EBX; + entry->ecx &= GUEST_SUPPORTED_CPUID_14_ECX; + entry->edx = 0; for (i = 1, max_idx = entry->eax; i <= max_idx; ++i) { if (!do_host_cpuid(array, function, i)) goto out; > > And is there any possibility of a malicious user/guest using features to cause > problems in the host? I.e. does KVM need to enforce that the guest can't enable > any unsupported features? If a user space is set up with features not supported by KVM, it owns the risk itself. AFAI, the guest Intel PT introduces a great attack interface for the host and we only use the guest supported PT features in a highly trusted environment. I agree that more uncertainty and fixes can be triggered in the security motive, not expecting too much from this patch. :D > >> for (i = 1, max_idx = entry->eax; i <= max_idx; ++i) { >> if (!do_host_cpuid(array, function, i)) >> goto out; >> -- >> 2.33.1 >>