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__ */