From patchwork Wed Jul 10 12:54:37 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Norbert Manthey X-Patchwork-Id: 11038451 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0BDEE912 for ; Wed, 10 Jul 2019 12:58:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F2AD628971 for ; Wed, 10 Jul 2019 12:58:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E72842897B; Wed, 10 Jul 2019 12:58:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_ADSP_ALL, DKIM_INVALID,DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id CA96E28971 for ; Wed, 10 Jul 2019 12:58:31 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hlC9Q-0002zs-LC; Wed, 10 Jul 2019 12:56:48 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hlC9P-0002ze-0e for xen-devel@lists.xenproject.org; Wed, 10 Jul 2019 12:56:47 +0000 X-Inumbo-ID: 2be88d22-a312-11e9-b798-7b7ba7ac6368 Received: from smtp-fw-9101.amazon.com (unknown [207.171.184.25]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 2be88d22-a312-11e9-b798-7b7ba7ac6368; Wed, 10 Jul 2019 12:56:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.de; i=@amazon.de; q=dns/txt; s=amazon201209; t=1562763406; x=1594299406; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=EH64tr4ZTZOlavnwkK9YoZxAynIy4c9UhRkVmZm0TXk=; b=kF7h2S6Naj+xnTJ1Kb2yQFq0wmINpdpzsFjgSVRbOyTDuFKO92kLR4zK Ep/OC7uBKTa/ogD31TN7/s7/p20JKbvRMSmb3k7Jr9/w1rqIGcfmKgG8N LNBqqDVrBxJdR6WB6t/OV11HRsiKFCmqTz91suv6hEooK0ZMAMNti0Wbt 8=; X-IronPort-AV: E=Sophos;i="5.62,474,1554768000"; d="scan'208";a="815390045" Received: from sea3-co-svc-lb6-vlan2.sea.amazon.com (HELO email-inbound-relay-1a-af6a10df.us-east-1.amazon.com) ([10.47.22.34]) by smtp-border-fw-out-9101.sea19.amazon.com with ESMTP; 10 Jul 2019 12:56:43 +0000 Received: from EX13MTAUEB001.ant.amazon.com (iad55-ws-svc-p15-lb9-vlan3.iad.amazon.com [10.40.159.166]) by email-inbound-relay-1a-af6a10df.us-east-1.amazon.com (Postfix) with ESMTPS id 6364EA2362; Wed, 10 Jul 2019 12:56:39 +0000 (UTC) Received: from EX13D08UEB001.ant.amazon.com (10.43.60.245) by EX13MTAUEB001.ant.amazon.com (10.43.60.129) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Wed, 10 Jul 2019 12:56:18 +0000 Received: from EX13MTAUEB001.ant.amazon.com (10.43.60.96) by EX13D08UEB001.ant.amazon.com (10.43.60.245) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Wed, 10 Jul 2019 12:56:17 +0000 Received: from uc1a35a69ae4659.ant.amazon.com (10.95.156.35) by mail-relay.amazon.com (10.43.60.129) with Microsoft SMTP Server id 15.0.1367.3 via Frontend Transport; Wed, 10 Jul 2019 12:56:14 +0000 From: Norbert Manthey To: Date: Wed, 10 Jul 2019 14:54:37 +0200 Message-ID: <1562763277-11674-3-git-send-email-nmanthey@amazon.de> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1562763277-11674-1-git-send-email-nmanthey@amazon.de> References: <1562763277-11674-1-git-send-email-nmanthey@amazon.de> MIME-Version: 1.0 Precedence: Bulk Subject: [Xen-devel] [PATCH L1TF MDS GT v2 2/2] common/grant_table: harden version dependent accesses X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Juergen Gross , Tim Deegan , Stefano Stabellini , Wei Liu , Konrad Rzeszutek Wilk , George Dunlap , Andrew Cooper , Ian Jackson , Dario Faggioli , Martin Pohlack , Pawel Wieczorkiewicz , Julien Grall , David Woodhouse , Jan Beulich , Martin Mazein , Bjoern Doebel , Norbert Manthey Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP Guests can issue grant table operations and provide guest controlled data to them. This data is used as index for memory loads after bound checks have been done. Depending on the grant table version, the size of elements in containers differ. As the base data structure is a page, the number of elements per page also differs. Consequently, bound checks are version dependent, so that speculative execution can happen in several stages, the bound check as well as the version check. This commit mitigates cases where out-of-bound accesses could happen due to the version comparison. In cases, where no different memory locations are accessed on the code path that follow an if statement, no protection is required. No different memory locations are accessed in the following functions after a version check: * gnttab_setup_table: only calculated numbersi are used, and then function gnttab_grow_table is called, which is version protected * gnttab_transfer: the case that depends on the version check just gets into copying a page or not * acquire_grant_for_copy: the not fixed comparison is on the abort path and does not access other structures, and on the else branch accesses only structures that have been validated before * gnttab_set_version: all accessible data is allocated for both versions Furthermore, the functions gnttab_populate_status_frames and gnttab_unpopulate_status_frames received a block_speculation macro. Hence, this code will only be executed once the correct version is visible in the architectural state. * gnttab_release_mappings: this function is called only during domain destruction and control is not returned to the guest * mem_sharing_gref_to_gfn: speculation will be stoped by the second if statement, as that places a barrier on any path to be executed. * gnttab_get_status_frame_mfn: no version dependent check, because all accesses, except the gt->status[idx], do not perform index-based accesses, or speculative out-of-bound accesses in the gnttab_grow_table function call. * gnttab_usage_print: cannot be triggered by the guest This is part of the speculative hardening effort. Signed-off-by: Norbert Manthey Reviewed-by: Jan Beulich --- Notes: v2: Add block_speculation to gnttab_populate_status_frames and gnttab_unpopulate_status_frames Add block_speculation in gnttab_get_status_frame_mfn, to make sure we actually work with a version 2 grant table xen/common/grant_table.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -828,7 +828,7 @@ static int _set_status(const grant_entry_header_t *shah, domid_t ldomid) { - if ( rgt_version == 1 ) + if ( evaluate_nospec(rgt_version == 1) ) return _set_status_v1(shah, rd, act, readonly, mapflag, ldomid); else return _set_status_v2(shah, status, rd, act, readonly, mapflag, ldomid); @@ -983,9 +983,12 @@ map_grant_ref( /* This call also ensures the above check cannot be passed speculatively */ shah = shared_entry_header(rgt, ref); - status = rgt->gt_version == 1 ? &shah->flags : &status_entry(rgt, ref); act = active_entry_acquire(rgt, ref); + /* Make sure we do not access memory speculatively */ + status = evaluate_nospec(rgt->gt_version == 1) ? &shah->flags + : &status_entry(rgt, ref); + /* If already pinned, check the active domid and avoid refcnt overflow. */ if ( act->pin && ((act->domid != ld->domain_id) || @@ -1006,7 +1009,7 @@ map_grant_ref( if ( !act->pin ) { - unsigned long gfn = rgt->gt_version == 1 ? + unsigned long gfn = evaluate_nospec(rgt->gt_version == 1) ? shared_entry_v1(rgt, ref).frame : shared_entry_v2(rgt, ref).full_page.frame; @@ -1458,7 +1461,7 @@ unmap_common_complete(struct gnttab_unmap_common *op) act = active_entry_acquire(rgt, op->ref); sha = shared_entry_header(rgt, op->ref); - if ( rgt->gt_version == 1 ) + if ( evaluate_nospec(rgt->gt_version == 1) ) status = &sha->flags; else status = &status_entry(rgt, op->ref); @@ -1651,6 +1654,10 @@ gnttab_populate_status_frames(struct domain *d, struct grant_table *gt, unsigned req_status_frames; req_status_frames = grant_to_status_frames(req_nr_frames); + + /* Make sure, prior version checks are architectural visible */ + block_speculation(); + for ( i = nr_status_frames(gt); i < req_status_frames; i++ ) { if ( (gt->status[i] = alloc_xenheap_page()) == NULL ) @@ -1679,6 +1686,9 @@ gnttab_unpopulate_status_frames(struct domain *d, struct grant_table *gt) { unsigned int i; + /* Make sure, prior version checks are architectural visible */ + block_speculation(); + for ( i = 0; i < nr_status_frames(gt); i++ ) { struct page_info *pg = virt_to_page(gt->status[i]); @@ -1790,7 +1800,7 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames) } /* Status pages - version 2 */ - if ( gt->gt_version > 1 ) + if ( evaluate_nospec(gt->gt_version > 1) ) { if ( gnttab_populate_status_frames(d, gt, req_nr_frames) ) goto shared_alloc_failed; @@ -2286,7 +2296,7 @@ gnttab_transfer( grant_read_lock(e->grant_table); act = active_entry_acquire(e->grant_table, ref); - if ( e->grant_table->gt_version == 1 ) + if ( evaluate_nospec(e->grant_table->gt_version == 1) ) { grant_entry_v1_t *sha = &shared_entry_v1(e->grant_table, ref); @@ -2347,7 +2357,7 @@ release_grant_for_copy( sha = shared_entry_header(rgt, gref); mfn = act->mfn; - if ( rgt->gt_version == 1 ) + if ( evaluate_nospec(rgt->gt_version == 1) ) { status = &sha->flags; td = rd; @@ -2446,7 +2456,7 @@ acquire_grant_for_copy( shah = shared_entry_header(rgt, gref); act = active_entry_acquire(rgt, gref); - if ( rgt->gt_version == 1 ) + if ( evaluate_nospec(rgt->gt_version == 1) ) { sha2 = NULL; status = &shah->flags; @@ -3265,7 +3275,7 @@ swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b) if ( act_b->pin ) PIN_FAIL(out, GNTST_eagain, "ref b %#x busy\n", ref_b); - if ( gt->gt_version == 1 ) + if ( evaluate_nospec(gt->gt_version == 1) ) { grant_entry_v1_t shared; @@ -3814,7 +3824,7 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref, rc = -EINVAL; else if ( ref >= nr_grant_entries(gt) ) rc = -ENOENT; - else if ( gt->gt_version == 1 ) + else if ( evaluate_nospec(gt->gt_version == 1) ) { const grant_entry_v1_t *sha1 = &shared_entry_v1(gt, ref); @@ -3836,7 +3846,7 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref, rc = -ENXIO; else if ( !rc && status ) { - if ( gt->gt_version == 1 ) + if ( evaluate_nospec(gt->gt_version == 1) ) *status = flags; else *status = status_entry(gt, ref); @@ -3856,6 +3866,9 @@ static int gnttab_get_status_frame_mfn(struct domain *d, ASSERT(gt->gt_version == 2); + /* Make sure we have version equal to 2 even under speculation */ + block_speculation(); + if ( idx >= nr_status_frames(gt) ) { unsigned long nr_status; @@ -3922,7 +3935,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn, mfn_t *mfn) grant_write_lock(gt); - if ( gt->gt_version == 2 && (idx & XENMAPIDX_grant_table_status) ) + if ( evaluate_nospec(gt->gt_version == 2) && (idx & XENMAPIDX_grant_table_status) ) { idx &= ~XENMAPIDX_grant_table_status; status = true;