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.5 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 1C0EFC433DB for ; Tue, 9 Feb 2021 14:57:36 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 99AA364DB2 for ; Tue, 9 Feb 2021 14:57:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 99AA364DB2 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.83273.154530 (Exim 4.92) (envelope-from ) id 1l9USD-0003vi-GN; Tue, 09 Feb 2021 14:57:25 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 83273.154530; Tue, 09 Feb 2021 14:57:25 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l9USD-0003vb-D4; Tue, 09 Feb 2021 14:57:25 +0000 Received: by outflank-mailman (input) for mailman id 83273; Tue, 09 Feb 2021 14:57:23 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l9USB-0003vW-Oi for xen-devel@lists.xenproject.org; Tue, 09 Feb 2021 14:57:23 +0000 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 367524fb-4ee8-4538-8a9c-996a97ee57b9; Tue, 09 Feb 2021 14:57:22 +0000 (UTC) Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id DD8CCB1D0; Tue, 9 Feb 2021 14:57:21 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 367524fb-4ee8-4538-8a9c-996a97ee57b9 X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1612882642; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6hER2UZhFNmNNsYtEHAf4viM09RvDVnXS8darKIfCYQ=; b=BIMuKyWhfT1kRkKwBe/Yz+QFs9U0ajdjMsI61FRBQqusvGOSv6IUL2YxxYSnW0FlgnggFg J/n186WxslJUA87GHFdz0CvXcs7T6OgFq2qTbu/rEcVv3dvgbfwSoJQi8knmfom0arv347 gdY1oEFeeix7xgVp5m/OO6COPp2Gs8M= Subject: Re: [PATCH 02/17] x86: split __{get,put}_user() into "guest" and "unsafe" variants To: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= Cc: "xen-devel@lists.xenproject.org" , Andrew Cooper , Wei Liu , Tim Deegan , George Dunlap References: <4f1975a9-bdd9-f556-9db5-eb6c428f258f@suse.com> <13d1d621-21db-0e59-6603-2b22b6a9d180@suse.com> <199d2681-9704-8804-d3c3-d8ad24fca137@suse.com> <1cf476b9-4ac1-9a12-7fdb-c898f02532f7@suse.com> From: Jan Beulich Message-ID: Date: Tue, 9 Feb 2021 15:57:22 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 09.02.2021 15:46, Roger Pau Monné wrote: > On Tue, Feb 09, 2021 at 02:15:18PM +0100, Jan Beulich wrote: >> On 09.02.2021 14:07, Roger Pau Monné wrote: >>> On Fri, Feb 05, 2021 at 05:26:33PM +0100, Jan Beulich wrote: >>>> On 05.02.2021 17:18, Roger Pau Monné wrote: >>>>> On Fri, Feb 05, 2021 at 05:13:22PM +0100, Jan Beulich wrote: >>>>>> On 05.02.2021 16:43, Roger Pau Monné wrote: >>>>>>> On Thu, Jan 14, 2021 at 04:04:11PM +0100, Jan Beulich wrote: >>>>>>>> The "guest" variants are intended to work with (potentially) fully guest >>>>>>>> controlled addresses, while the "unsafe" variants are not. >>>>>>> >>>>>>> Just to clarify, both work against user addresses, but guest variants >>>>>>> need to be more careful because the guest provided address can also be >>>>>>> modified? >>>>>>> >>>>>>> I'm trying to understand the difference between "fully guest >>>>>>> controlled" and "guest controlled". >>>>>> >>>>>> Not exactly, not. "unsafe" means access to anything which may >>>>>> fault, guest controlled or not. do_invalid_op()'s reading of >>>>>> the insn stream is a good example - the faulting insn there >>>>>> isn't guest controlled at all, but we still want to be careful >>>>>> when trying to read these bytes, as we don't want to fully >>>>>> trust %rip there. >>> >>> Oh, I see. It's possible that %rip points to an unmapped address >>> there, and we need to be careful when reading, even if the value of >>> %rip cannot be controlled by the guest and can legitimacy point to >>> Xen's address space. >>> >>>>> Would it make sense to threat everything as 'guest' accesses for the >>>>> sake of not having this difference? >>>> >>>> That's what we've been doing until now. It is the purpose of >>>> this change to allow the two to behave differently. >>>> >>>>> I think having two accessors it's likely to cause confusion and could >>>>> possibly lead to the wrong one being used in unexpected contexts. Does >>>>> it add a too big performance penalty to always use the most >>>>> restrictive one? >>>> >>>> The problem is the most restrictive one is going to be too >>>> restrictive - we wouldn't be able to access Xen space anymore >>>> e.g. from the place pointed at above as example. This is >>>> because for guest accesses (but not for "unsafe" ones) we're >>>> going to divert them into non-canonical space (and hence make >>>> speculation impossible, as such an access would fault) if it >>>> would touch Xen space. >>> >>> Yes, I understand now. I think it would have been helpful (for me) to >>> have the first sentence as: >>> >>> The "guest" variants are intended to work with (potentially) fully guest >>> controlled addresses, while the "unsafe" variants are expected to be >>> used in order to access addresses not under the guest control, but >>> that could trigger faults anyway (like accessing the instruction >>> stream in do_invalid_op). >> >> I can use some of this, but in particular "access addresses not >> under the guest control" isn't entirely correct. The question isn't >> whether there's a guest control aspect, but which part of the >> address space the addresses are in. See specifically x86/PV: use >> get_unsafe() instead of copy_from_unsafe()" for two pretty good >> examples. The address within the linear page tables are - in a >> way at least - still somewhat guest controlled, but we're >> deliberately accessing Xen virtual addresses there. > > OK, could this be somehow added to the commit message then? > > Maybe it would be better to have something like: > > The "guest" variants are intended to work with addresses belonging to > the guest address space, while the "unsafe" variants should be used > for addresses that fall into the Xen address space. > > I think it's important to list exactly how the distinction between the > guest/unsafe accessors is made, or else it's impossible to review that > the changes done here are correct. I did change the paragraph to read The "guest" variants are intended to work with (potentially) fully guest controlled addresses, while the "unsafe" variants are intended to be used in order to access addresses not (directly) under guest control, within Xen's part of virtual address space. (For linear page table and descriptor table accesses the low bits of the addresses may still be guest controlled, but this still won't allow speculation to "escape" into unwanted areas.) Subsequently we will want them to have distinct behavior, so as first step identify which one is which. For now, both groups of constructs alias one another. earlier today. Is this clear enough? Relevant parts I've also mirrored into patch 3's description? Jan