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 X-Spam-Level: X-Spam-Status: No, score=-7.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 52267C64E7C for ; Wed, 2 Dec 2020 10:47:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EC53F2222C for ; Wed, 2 Dec 2020 10:47:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729527AbgLBKrO (ORCPT ); Wed, 2 Dec 2020 05:47:14 -0500 Received: from aserp2130.oracle.com ([141.146.126.79]:47074 "EHLO aserp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729514AbgLBKrN (ORCPT ); Wed, 2 Dec 2020 05:47:13 -0500 Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 0B2AixNW072970; Wed, 2 Dec 2020 10:46:13 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2020-01-29; bh=wwhr47586+/6Fpo43Hc7ftI6vWuRmnMUsfbm3tDnCB4=; b=oFxRdXh4Zvbgyzs83YZP1YFbXgu/SHQNxfa1KhL6Kmk+m+hOpCEn4J/DwTkUAw4iHUQK fC7Xln2u+2ylqsaPPG4jSOjgwrm3lvuFvG0wiekSHCFwM3EXCmBjMY+lYX6x6SVkgVtD xHQ9WBju9CO63NBMum9NuN8XvLDSLrdT/Cf1U1v8dGEyrC4jl/ZP7njMGA7Hq2px5iSA h9TYLZkXyAvYU+JLMw0UUdW4oxdUnWBH+1szuiBeaMBj2qSNl6FLzWIyO61pRPdidQy7 n9ZHsh5Hrq/gmkmzZZmX1NnRFdS0dCJbNe2y4Vhg5ihkO5fbaoicj1dvESjrq7pgQLBx /w== Received: from aserp3030.oracle.com (aserp3030.oracle.com [141.146.126.71]) by aserp2130.oracle.com with ESMTP id 353c2aypeg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 02 Dec 2020 10:46:12 +0000 Received: from pps.filterd (aserp3030.oracle.com [127.0.0.1]) by aserp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 0B2AaXOY130396; Wed, 2 Dec 2020 10:44:12 GMT Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserp3030.oracle.com with ESMTP id 35404p4qj9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 02 Dec 2020 10:44:12 +0000 Received: from abhmp0016.oracle.com (abhmp0016.oracle.com [141.146.116.22]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 0B2Ai7ko032261; Wed, 2 Dec 2020 10:44:08 GMT Received: from [10.175.181.158] (/10.175.181.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 02 Dec 2020 02:44:07 -0800 Subject: Re: [PATCH RFC 03/39] KVM: x86/xen: register shared_info page To: Ankur Arora , David Woodhouse Cc: Boris Ostrovsky , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org References: <20190220201609.28290-1-joao.m.martins@oracle.com> <20190220201609.28290-4-joao.m.martins@oracle.com> <2d4df59d-f945-32dc-6999-a6f711e972ea@oracle.com> From: Joao Martins Message-ID: <896dc984-fa71-8f2f-d12b-458294f5f706@oracle.com> Date: Wed, 2 Dec 2020 10:44:03 +0000 MIME-Version: 1.0 In-Reply-To: <2d4df59d-f945-32dc-6999-a6f711e972ea@oracle.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9822 signatures=668682 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=1 bulkscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999 phishscore=0 spamscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2012020062 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9822 signatures=668682 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=1 lowpriorityscore=0 clxscore=1015 bulkscore=0 mlxlogscore=999 phishscore=0 malwarescore=0 spamscore=0 adultscore=0 mlxscore=0 priorityscore=1501 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2012020063 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [late response - was on holiday yesterday] On 12/2/20 12:40 AM, Ankur Arora wrote: > On 2020-12-01 5:07 a.m., David Woodhouse wrote: >> On Wed, 2019-02-20 at 20:15 +0000, Joao Martins wrote: >>> +static int kvm_xen_shared_info_init(struct kvm *kvm, gfn_t gfn) >>> +{ >>> + struct shared_info *shared_info; >>> + struct page *page; >>> + >>> + page = gfn_to_page(kvm, gfn); >>> + if (is_error_page(page)) >>> + return -EINVAL; >>> + >>> + kvm->arch.xen.shinfo_addr = gfn; >>> + >>> + shared_info = page_to_virt(page); >>> + memset(shared_info, 0, sizeof(struct shared_info)); >>> + kvm->arch.xen.shinfo = shared_info; >>> + return 0; >>> +} >>> + >> >> Hm. >> >> How come we get to pin the page and directly dereference it every time, >> while kvm_setup_pvclock_page() has to use kvm_write_guest_cached() >> instead? > > So looking at my WIP trees from the time, this is something that > we went back and forth on as well with using just a pinned page or a > persistent kvm_vcpu_map(). > > I remember distinguishing shared_info/vcpu_info from kvm_setup_pvclock_page() > as shared_info is created early and is not expected to change during the > lifetime of the guest which didn't seem true for MSR_KVM_SYSTEM_TIME (or > MSR_KVM_STEAL_TIME) so that would either need to do a kvm_vcpu_map() > kvm_vcpu_unmap() dance or do some kind of synchronization. > > That said, I don't think this code explicitly disallows any updates > to shared_info. > >> >> If that was allowed, wouldn't it have been a much simpler fix for >> CVE-2019-3016? What am I missing? > > Agreed. > > Perhaps, Paolo can chime in with why KVM never uses pinned page > and always prefers to do cached mappings instead? > Part of the CVE fix to not use cached versions. It's not a longterm pin of the page unlike we try to do here (partly due to the nature of the pages we are mapping) but we still we map the gpa, RMW the steal time struct, and then unmap the page. See record_steal_time() -- but more specifically commit b043138246 ("x86/KVM: Make sure KVM_VCPU_FLUSH_TLB flag is not missed"). But I am not sure it's a good idea to follow the same as record_steal_time() given that this is a fairly sensitive code path for event channels. >> >> Should I rework these to use kvm_write_guest_cached()? > > kvm_vcpu_map() would be better. The event channel logic does RMW operations > on shared_info->vcpu_info. > Indeed, yes. Ankur IIRC, we saw missed event channels notifications when we were using the {write,read}_cached() version of the patch. But I can't remember the reason it was due to, either the evtchn_pending or the mask word -- which would make it not inject an upcall. Joao