From patchwork Mon Nov 14 10:34:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Beulich X-Patchwork-Id: 9427173 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8FD4460471 for ; Mon, 14 Nov 2016 10:36:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7A941287A2 for ; Mon, 14 Nov 2016 10:36:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6F64C287A8; Mon, 14 Nov 2016 10:36:53 +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, 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 AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7C7EB287A2 for ; Mon, 14 Nov 2016 10:36:52 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c6Eay-0007hH-2Q; Mon, 14 Nov 2016 10:34:36 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c6Eaw-0007h8-KG for xen-devel@lists.xenproject.org; Mon, 14 Nov 2016 10:34:34 +0000 Received: from [85.158.143.35] by server-8.bemta-6.messagelabs.com id 87/89-19686-93399285; Mon, 14 Nov 2016 10:34:33 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrEIsWRWlGSWpSXmKPExsXS6fjDS9disma EQedxPYvvWyYzOTB6HP5whSWAMYo1My8pvyKBNWPC+cWsBcc6GCs+HW1kaWBcl9DFyMkhJJAn MX/xfmYQm1fATmLb24OsILaEgKHE0/fX2UBsFgFViZM3v7KD2GwC6hJtz7YD1XBwiAgYSJw7m tTFyMXBLHCYSWJJ+36wemEBG4nr6w6xgNTwCghK/N0hDBJmBhq/6nYX+wRGrlkImVlIMhC2ls TDX7dYIGxtiWULXzODlDMLSEss/8cBYdpITJ0vhKoCxHaX6Jjyin0BI8cqRo3i1KKy1CJdQwu 9pKLM9IyS3MTMHF1DAzO93NTi4sT01JzEpGK95PzcTYzA8GMAgh2MNzcGHGKU5GBSEuWNj9SM EOJLyk+pzEgszogvKs1JLT7EqMHBIdC3ZvUFRimWvPy8VCUJXtdJQHWCRanpqRVpmTnACIEpl eDgURLhtQJJ8xYXJOYWZ6ZDpE4xKkqJ894ASQiAJDJK8+DaYFF5iVFWSpiXEegoIZ6C1KLczB JU+VeM4hyMSsK8KyYCTeHJzCuBm/4KaDET0OJd5hogi0sSEVJSDYxdN9dMUJdQ46reUrB0UZn pl3JOxYM1KfmCslqppef2Tk2ZGrX7OY8m44bpC/JWXXr96VHzmdUu0zl/+lusNLjz/1Ox0AIX q4cG2yx5m39xnkpaZ+PRxO8yOfnihX2vf3RPCwkK2FFmcs3w0975R+aVq8b2Lp+1W2TLYeu0+ Lb8l+d8hGVbRfcpsRRnJBpqMRcVJwIAFOyo1MUCAAA= X-Env-Sender: JBeulich@suse.com X-Msg-Ref: server-11.tower-21.messagelabs.com!1479119670!43061472!1 X-Originating-IP: [137.65.248.74] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.0.16; banners=-,-,- X-VirusChecked: Checked Received: (qmail 16724 invoked from network); 14 Nov 2016 10:34:32 -0000 Received: from prv-mh.provo.novell.com (HELO prv-mh.provo.novell.com) (137.65.248.74) by server-11.tower-21.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 14 Nov 2016 10:34:32 -0000 Received: from INET-PRV-MTA by prv-mh.provo.novell.com with Novell_GroupWise; Mon, 14 Nov 2016 03:34:30 -0700 Message-Id: <5829A143020000780011E59D@prv-mh.provo.novell.com> X-Mailer: Novell GroupWise Internet Agent 14.2.1 Date: Mon, 14 Nov 2016 03:34:27 -0700 From: "Jan Beulich" To: "xen-devel" Mime-Version: 1.0 Cc: Stefano Stabellini , Wei Liu , George Dunlap , Andrew Cooper , Ian Jackson , Tim Deegan , tamas@tklengyel.com Subject: [Xen-devel] [PATCH] x86/memshr: properly check grant references X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP They need to be range checked against the current table limit in any event. Reported-by: Huawei PSIRT Move the code to where it belongs, eliminating a number of duplicate definitions. Add locking. Produce proper error codes, and consume them instead of making one up. Check grant type. Convert parameter types at once. Signed-off-by: Jan Beulich Acked-by: Tamas K Lengyel --- Note that likely there's more work needed subsequently: The grant isn't being marked in use for the duration of the use of the GFN. But I'll leave that to someone actually knowing how to properly to test this. x86/memshr: properly check grant references They need to be range checked against the current table limit in any event. Reported-by: Huawei PSIRT Move the code to where it belongs, eliminating a number of duplicate definitions. Add locking. Produce proper error codes, and consume them instead of making one up. Check grant type. Convert parameter types at once. Signed-off-by: Jan Beulich --- Note that likely there's more work needed subsequently: The grant isn't being marked in use for the duration of the use of the GFN. But I'll leave that to someone actually knowing how to properly to test this. --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -753,75 +753,25 @@ int mem_sharing_debug_gfn(struct domain return num_refs; } -#define SHGNT_PER_PAGE_V1 (PAGE_SIZE / sizeof(grant_entry_v1_t)) -#define shared_entry_v1(t, e) \ - ((t)->shared_v1[(e)/SHGNT_PER_PAGE_V1][(e)%SHGNT_PER_PAGE_V1]) -#define SHGNT_PER_PAGE_V2 (PAGE_SIZE / sizeof(grant_entry_v2_t)) -#define shared_entry_v2(t, e) \ - ((t)->shared_v2[(e)/SHGNT_PER_PAGE_V2][(e)%SHGNT_PER_PAGE_V2]) -#define STGNT_PER_PAGE (PAGE_SIZE / sizeof(grant_status_t)) -#define status_entry(t, e) \ - ((t)->status[(e)/STGNT_PER_PAGE][(e)%STGNT_PER_PAGE]) - -static grant_entry_header_t * -shared_entry_header(struct grant_table *t, grant_ref_t ref) -{ - ASSERT (t->gt_version != 0); - if ( t->gt_version == 1 ) - return (grant_entry_header_t*)&shared_entry_v1(t, ref); - else - return &shared_entry_v2(t, ref).hdr; -} - -static int mem_sharing_gref_to_gfn(struct domain *d, - grant_ref_t ref, - unsigned long *gfn) -{ - if ( d->grant_table->gt_version < 1 ) - return -1; - - if ( d->grant_table->gt_version == 1 ) - { - grant_entry_v1_t *sha1; - sha1 = &shared_entry_v1(d->grant_table, ref); - *gfn = sha1->frame; - } - else - { - grant_entry_v2_t *sha2; - sha2 = &shared_entry_v2(d->grant_table, ref); - *gfn = sha2->full_page.frame; - } - - return 0; -} - - int mem_sharing_debug_gref(struct domain *d, grant_ref_t ref) { - grant_entry_header_t *shah; + int rc; uint16_t status; - unsigned long gfn; + gfn_t gfn; - if ( d->grant_table->gt_version < 1 ) + rc = mem_sharing_gref_to_gfn(d->grant_table, ref, &gfn, &status); + if ( rc ) { - MEM_SHARING_DEBUG( - "Asked to debug [dom=%d,gref=%d], but not yet inited.\n", - d->domain_id, ref); - return -EINVAL; - } - (void)mem_sharing_gref_to_gfn(d, ref, &gfn); - shah = shared_entry_header(d->grant_table, ref); - if ( d->grant_table->gt_version == 1 ) - status = shah->flags; - else - status = status_entry(d->grant_table, ref); + MEM_SHARING_DEBUG("Asked to debug [dom=%d,gref=%u]: error %d.\n", + d->domain_id, ref, rc); + return rc; + } MEM_SHARING_DEBUG( "==> Grant [dom=%d,ref=%d], status=%x. ", d->domain_id, ref, status); - return mem_sharing_debug_gfn(d, gfn); + return mem_sharing_debug_gfn(d, gfn_x(gfn)); } int mem_sharing_nominate_page(struct domain *d, @@ -1422,23 +1372,24 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_P case XENMEM_sharing_op_nominate_gref: { grant_ref_t gref = mso.u.nominate.u.grant_ref; - unsigned long gfn; + gfn_t gfn; shr_handle_t handle; rc = -EINVAL; if ( !mem_sharing_enabled(d) ) goto out; - if ( mem_sharing_gref_to_gfn(d, gref, &gfn) < 0 ) + rc = mem_sharing_gref_to_gfn(d->grant_table, gref, &gfn, NULL); + if ( rc < 0 ) goto out; - rc = mem_sharing_nominate_page(d, gfn, 3, &handle); + rc = mem_sharing_nominate_page(d, gfn_x(gfn), 3, &handle); mso.u.nominate.handle = handle; } break; case XENMEM_sharing_op_share: { - unsigned long sgfn, cgfn; + gfn_t sgfn, cgfn; struct domain *cd; shr_handle_t sh, ch; @@ -1470,35 +1421,38 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_P grant_ref_t gref = (grant_ref_t) (XENMEM_SHARING_OP_FIELD_GET_GREF( mso.u.share.source_gfn)); - if ( mem_sharing_gref_to_gfn(d, gref, &sgfn) < 0 ) + rc = mem_sharing_gref_to_gfn(d->grant_table, gref, &sgfn, + NULL); + if ( rc < 0 ) { rcu_unlock_domain(cd); - rc = -EINVAL; goto out; } - } else { - sgfn = mso.u.share.source_gfn; } + else + sgfn = _gfn(mso.u.share.source_gfn); if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mso.u.share.client_gfn) ) { grant_ref_t gref = (grant_ref_t) (XENMEM_SHARING_OP_FIELD_GET_GREF( mso.u.share.client_gfn)); - if ( mem_sharing_gref_to_gfn(cd, gref, &cgfn) < 0 ) + rc = mem_sharing_gref_to_gfn(cd->grant_table, gref, &cgfn, + NULL); + if ( rc < 0 ) { rcu_unlock_domain(cd); - rc = -EINVAL; goto out; } - } else { - cgfn = mso.u.share.client_gfn; } + else + cgfn = _gfn(mso.u.share.client_gfn); sh = mso.u.share.source_handle; ch = mso.u.share.client_handle; - rc = mem_sharing_share_pages(d, sgfn, sh, cd, cgfn, ch); + rc = mem_sharing_share_pages(d, gfn_x(sgfn), sh, + cd, gfn_x(cgfn), ch); rcu_unlock_domain(cd); } --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -3438,6 +3438,53 @@ void grant_table_init_vcpu(struct vcpu * v->maptrack_tail = MAPTRACK_TAIL; } +#ifdef CONFIG_HAS_MEM_SHARING +int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref, + gfn_t *gfn, uint16_t *status) +{ + int rc = 0; + uint16_t flags = 0; + + grant_read_lock(gt); + + if ( gt->gt_version < 1 ) + rc = -EINVAL; + else if ( ref >= nr_grant_entries(gt) ) + rc = -ENOENT; + else if ( gt->gt_version == 1 ) + { + const grant_entry_v1_t *sha1 = &shared_entry_v1(gt, ref); + + flags = sha1->flags; + *gfn = _gfn(sha1->frame); + } + else + { + const grant_entry_v2_t *sha2 = &shared_entry_v2(gt, ref); + + flags = sha2->hdr.flags; + if ( flags & GTF_sub_page ) + *gfn = _gfn(sha2->sub_page.frame); + else + *gfn = _gfn(sha2->full_page.frame); + } + + if ( (flags & GTF_type_mask) != GTF_permit_access ) + rc = -ENXIO; + else if ( !rc && status ) + { + if ( gt->gt_version == 1 ) + *status = flags; + else + *status = status_entry(gt, ref); + } + + grant_read_unlock(gt); + + return rc; +} +#endif + static void gnttab_usage_print(struct domain *rd) { int first = 1; --- a/xen/include/xen/grant_table.h +++ b/xen/include/xen/grant_table.h @@ -149,4 +149,7 @@ static inline unsigned int grant_to_stat GRANT_STATUS_PER_PAGE; } +int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref, + gfn_t *gfn, uint16_t *status); + #endif /* __XEN_GRANT_TABLE_H__ */ --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -753,75 +753,25 @@ int mem_sharing_debug_gfn(struct domain return num_refs; } -#define SHGNT_PER_PAGE_V1 (PAGE_SIZE / sizeof(grant_entry_v1_t)) -#define shared_entry_v1(t, e) \ - ((t)->shared_v1[(e)/SHGNT_PER_PAGE_V1][(e)%SHGNT_PER_PAGE_V1]) -#define SHGNT_PER_PAGE_V2 (PAGE_SIZE / sizeof(grant_entry_v2_t)) -#define shared_entry_v2(t, e) \ - ((t)->shared_v2[(e)/SHGNT_PER_PAGE_V2][(e)%SHGNT_PER_PAGE_V2]) -#define STGNT_PER_PAGE (PAGE_SIZE / sizeof(grant_status_t)) -#define status_entry(t, e) \ - ((t)->status[(e)/STGNT_PER_PAGE][(e)%STGNT_PER_PAGE]) - -static grant_entry_header_t * -shared_entry_header(struct grant_table *t, grant_ref_t ref) -{ - ASSERT (t->gt_version != 0); - if ( t->gt_version == 1 ) - return (grant_entry_header_t*)&shared_entry_v1(t, ref); - else - return &shared_entry_v2(t, ref).hdr; -} - -static int mem_sharing_gref_to_gfn(struct domain *d, - grant_ref_t ref, - unsigned long *gfn) -{ - if ( d->grant_table->gt_version < 1 ) - return -1; - - if ( d->grant_table->gt_version == 1 ) - { - grant_entry_v1_t *sha1; - sha1 = &shared_entry_v1(d->grant_table, ref); - *gfn = sha1->frame; - } - else - { - grant_entry_v2_t *sha2; - sha2 = &shared_entry_v2(d->grant_table, ref); - *gfn = sha2->full_page.frame; - } - - return 0; -} - - int mem_sharing_debug_gref(struct domain *d, grant_ref_t ref) { - grant_entry_header_t *shah; + int rc; uint16_t status; - unsigned long gfn; + gfn_t gfn; - if ( d->grant_table->gt_version < 1 ) + rc = mem_sharing_gref_to_gfn(d->grant_table, ref, &gfn, &status); + if ( rc ) { - MEM_SHARING_DEBUG( - "Asked to debug [dom=%d,gref=%d], but not yet inited.\n", - d->domain_id, ref); - return -EINVAL; - } - (void)mem_sharing_gref_to_gfn(d, ref, &gfn); - shah = shared_entry_header(d->grant_table, ref); - if ( d->grant_table->gt_version == 1 ) - status = shah->flags; - else - status = status_entry(d->grant_table, ref); + MEM_SHARING_DEBUG("Asked to debug [dom=%d,gref=%u]: error %d.\n", + d->domain_id, ref, rc); + return rc; + } MEM_SHARING_DEBUG( "==> Grant [dom=%d,ref=%d], status=%x. ", d->domain_id, ref, status); - return mem_sharing_debug_gfn(d, gfn); + return mem_sharing_debug_gfn(d, gfn_x(gfn)); } int mem_sharing_nominate_page(struct domain *d, @@ -1422,23 +1372,24 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_P case XENMEM_sharing_op_nominate_gref: { grant_ref_t gref = mso.u.nominate.u.grant_ref; - unsigned long gfn; + gfn_t gfn; shr_handle_t handle; rc = -EINVAL; if ( !mem_sharing_enabled(d) ) goto out; - if ( mem_sharing_gref_to_gfn(d, gref, &gfn) < 0 ) + rc = mem_sharing_gref_to_gfn(d->grant_table, gref, &gfn, NULL); + if ( rc < 0 ) goto out; - rc = mem_sharing_nominate_page(d, gfn, 3, &handle); + rc = mem_sharing_nominate_page(d, gfn_x(gfn), 3, &handle); mso.u.nominate.handle = handle; } break; case XENMEM_sharing_op_share: { - unsigned long sgfn, cgfn; + gfn_t sgfn, cgfn; struct domain *cd; shr_handle_t sh, ch; @@ -1470,35 +1421,38 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_P grant_ref_t gref = (grant_ref_t) (XENMEM_SHARING_OP_FIELD_GET_GREF( mso.u.share.source_gfn)); - if ( mem_sharing_gref_to_gfn(d, gref, &sgfn) < 0 ) + rc = mem_sharing_gref_to_gfn(d->grant_table, gref, &sgfn, + NULL); + if ( rc < 0 ) { rcu_unlock_domain(cd); - rc = -EINVAL; goto out; } - } else { - sgfn = mso.u.share.source_gfn; } + else + sgfn = _gfn(mso.u.share.source_gfn); if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mso.u.share.client_gfn) ) { grant_ref_t gref = (grant_ref_t) (XENMEM_SHARING_OP_FIELD_GET_GREF( mso.u.share.client_gfn)); - if ( mem_sharing_gref_to_gfn(cd, gref, &cgfn) < 0 ) + rc = mem_sharing_gref_to_gfn(cd->grant_table, gref, &cgfn, + NULL); + if ( rc < 0 ) { rcu_unlock_domain(cd); - rc = -EINVAL; goto out; } - } else { - cgfn = mso.u.share.client_gfn; } + else + cgfn = _gfn(mso.u.share.client_gfn); sh = mso.u.share.source_handle; ch = mso.u.share.client_handle; - rc = mem_sharing_share_pages(d, sgfn, sh, cd, cgfn, ch); + rc = mem_sharing_share_pages(d, gfn_x(sgfn), sh, + cd, gfn_x(cgfn), ch); rcu_unlock_domain(cd); } --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -3438,6 +3438,53 @@ void grant_table_init_vcpu(struct vcpu * v->maptrack_tail = MAPTRACK_TAIL; } +#ifdef CONFIG_HAS_MEM_SHARING +int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref, + gfn_t *gfn, uint16_t *status) +{ + int rc = 0; + uint16_t flags = 0; + + grant_read_lock(gt); + + if ( gt->gt_version < 1 ) + rc = -EINVAL; + else if ( ref >= nr_grant_entries(gt) ) + rc = -ENOENT; + else if ( gt->gt_version == 1 ) + { + const grant_entry_v1_t *sha1 = &shared_entry_v1(gt, ref); + + flags = sha1->flags; + *gfn = _gfn(sha1->frame); + } + else + { + const grant_entry_v2_t *sha2 = &shared_entry_v2(gt, ref); + + flags = sha2->hdr.flags; + if ( flags & GTF_sub_page ) + *gfn = _gfn(sha2->sub_page.frame); + else + *gfn = _gfn(sha2->full_page.frame); + } + + if ( (flags & GTF_type_mask) != GTF_permit_access ) + rc = -ENXIO; + else if ( !rc && status ) + { + if ( gt->gt_version == 1 ) + *status = flags; + else + *status = status_entry(gt, ref); + } + + grant_read_unlock(gt); + + return rc; +} +#endif + static void gnttab_usage_print(struct domain *rd) { int first = 1; --- a/xen/include/xen/grant_table.h +++ b/xen/include/xen/grant_table.h @@ -149,4 +149,7 @@ static inline unsigned int grant_to_stat GRANT_STATUS_PER_PAGE; } +int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref, + gfn_t *gfn, uint16_t *status); + #endif /* __XEN_GRANT_TABLE_H__ */