* [PATCH] x86/memshr: properly check grant references
@ 2016-11-14 10:34 Jan Beulich
2016-11-14 11:56 ` Andrew Cooper
2016-11-22 10:12 ` Ping: " Jan Beulich
0 siblings, 2 replies; 7+ messages in thread
From: Jan Beulich @ 2016-11-14 10:34 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, tamas
[-- Attachment #1: Type: text/plain, Size: 8157 bytes --]
They need to be range checked against the current table limit in any
event.
Reported-by: Huawei PSIRT <psirt@huawei.com>
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 <jbeulich@suse.com>
---
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__ */
[-- Attachment #2: x86-memshr-gref-check.patch --]
[-- Type: text/plain, Size: 8200 bytes --]
x86/memshr: properly check grant references
They need to be range checked against the current table limit in any
event.
Reported-by: Huawei PSIRT <psirt@huawei.com>
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 <jbeulich@suse.com>
---
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__ */
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/memshr: properly check grant references
2016-11-14 10:34 [PATCH] x86/memshr: properly check grant references Jan Beulich
@ 2016-11-14 11:56 ` Andrew Cooper
2016-11-14 12:56 ` Jan Beulich
2016-11-22 10:12 ` Ping: " Jan Beulich
1 sibling, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2016-11-14 11:56 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, tamas
On 14/11/16 10:34, Jan Beulich wrote:
> --- 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;
This will clobber the EINVAL/ENOENT cases. It wants to be pared with an
!rc &&.
With this fixed, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> + 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;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/memshr: properly check grant references
2016-11-14 11:56 ` Andrew Cooper
@ 2016-11-14 12:56 ` Jan Beulich
0 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2016-11-14 12:56 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, tamas, xen-devel
>>> On 14.11.16 at 12:56, <andrew.cooper3@citrix.com> wrote:
> On 14/11/16 10:34, Jan Beulich wrote:
>> --- 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;
>
> This will clobber the EINVAL/ENOENT cases. It wants to be pared with an
> !rc &&.
Oh, indeed - thanks for noticing.
> With this fixed, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Thanks, Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Ping: [PATCH] x86/memshr: properly check grant references
2016-11-14 10:34 [PATCH] x86/memshr: properly check grant references Jan Beulich
2016-11-14 11:56 ` Andrew Cooper
@ 2016-11-22 10:12 ` Jan Beulich
2016-11-22 16:13 ` Tamas K Lengyel
1 sibling, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2016-11-22 10:12 UTC (permalink / raw)
To: tamas
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, xen-devel
>>> On 14.11.16 at 11:34, <JBeulich@suse.com> wrote:
> They need to be range checked against the current table limit in any
> event.
>
> Reported-by: Huawei PSIRT <psirt@huawei.com>
>
> 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 <jbeulich@suse.com>
Tamas? (The minor fix needed to address Andrew's reply doesn't seem
to warrant sending out a v2.)
> ---
> 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__ */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Ping: [PATCH] x86/memshr: properly check grant references
2016-11-22 10:12 ` Ping: " Jan Beulich
@ 2016-11-22 16:13 ` Tamas K Lengyel
2016-11-22 16:17 ` Jan Beulich
0 siblings, 1 reply; 7+ messages in thread
From: Tamas K Lengyel @ 2016-11-22 16:13 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 9602 bytes --]
On Tue, Nov 22, 2016 at 3:12 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 14.11.16 at 11:34, <JBeulich@suse.com> wrote:
> > They need to be range checked against the current table limit in any
> > event.
> >
> > Reported-by: Huawei PSIRT <psirt@huawei.com>
> >
> > 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 <jbeulich@suse.com>
>
> Tamas? (The minor fix needed to address Andrew's reply doesn't seem
> to warrant sending out a v2.)
> > ---
> > 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.
>
>
Hi Jan,
unfortunately I don't have a good way to test this either as I never used
memsharing with grefs before. The above comment about the grant not being
marked for in-use makes me wonder whether this is a regression from this
patch or whether that just was never the case. Either way, I can see this
being an issue only if memory is being removed by hot-plugging, which AFAIK
is not a supported scenario anyway. The rest of the patch is fairly
mechanical, so:
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> > --- 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__ */
>
>
[-- Attachment #1.2: Type: text/html, Size: 13286 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Ping: [PATCH] x86/memshr: properly check grant references
2016-11-22 16:13 ` Tamas K Lengyel
@ 2016-11-22 16:17 ` Jan Beulich
2016-11-22 16:22 ` Wei Liu
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2016-11-22 16:17 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, xen-devel
>>> On 22.11.16 at 17:13, <tamas@tklengyel.com> wrote:
> On Tue, Nov 22, 2016 at 3:12 AM, Jan Beulich <JBeulich@suse.com> wrote:
>
>> >>> On 14.11.16 at 11:34, <JBeulich@suse.com> wrote:
>> > They need to be range checked against the current table limit in any
>> > event.
>> >
>> > Reported-by: Huawei PSIRT <psirt@huawei.com>
>> >
>> > 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 <jbeulich@suse.com>
>>
>> Tamas? (The minor fix needed to address Andrew's reply doesn't seem
>> to warrant sending out a v2.)
>
>
>> > ---
>> > 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.
>>
>>
> Hi Jan,
> unfortunately I don't have a good way to test this either as I never used
> memsharing with grefs before. The above comment about the grant not being
> marked for in-use makes me wonder whether this is a regression from this
> patch or whether that just was never the case.
This was never the case. I wouldn't dare to submit a patch
knowingly breaking something.
> Either way, I can see this
> being an issue only if memory is being removed by hot-plugging, which AFAIK
> is not a supported scenario anyway. The rest of the patch is fairly
> mechanical, so:
>
> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Thanks.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Ping: [PATCH] x86/memshr: properly check grant references
2016-11-22 16:17 ` Jan Beulich
@ 2016-11-22 16:22 ` Wei Liu
0 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2016-11-22 16:22 UTC (permalink / raw)
To: Jan Beulich
Cc: Tamas K Lengyel, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Stefano Stabellini, xen-devel
On Tue, Nov 22, 2016 at 09:17:43AM -0700, Jan Beulich wrote:
> >>> On 22.11.16 at 17:13, <tamas@tklengyel.com> wrote:
> > On Tue, Nov 22, 2016 at 3:12 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >
> >> >>> On 14.11.16 at 11:34, <JBeulich@suse.com> wrote:
> >> > They need to be range checked against the current table limit in any
> >> > event.
> >> >
> >> > Reported-by: Huawei PSIRT <psirt@huawei.com>
> >> >
> >> > 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 <jbeulich@suse.com>
> >>
> >> Tamas? (The minor fix needed to address Andrew's reply doesn't seem
> >> to warrant sending out a v2.)
> >
> >
> >> > ---
> >> > 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.
> >>
> >>
> > Hi Jan,
> > unfortunately I don't have a good way to test this either as I never used
> > memsharing with grefs before. The above comment about the grant not being
> > marked for in-use makes me wonder whether this is a regression from this
> > patch or whether that just was never the case.
>
> This was never the case. I wouldn't dare to submit a patch
> knowingly breaking something.
>
> > Either way, I can see this
> > being an issue only if memory is being removed by hot-plugging, which AFAIK
> > is not a supported scenario anyway. The rest of the patch is fairly
> > mechanical, so:
> >
> > Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-11-22 16:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-14 10:34 [PATCH] x86/memshr: properly check grant references Jan Beulich
2016-11-14 11:56 ` Andrew Cooper
2016-11-14 12:56 ` Jan Beulich
2016-11-22 10:12 ` Ping: " Jan Beulich
2016-11-22 16:13 ` Tamas K Lengyel
2016-11-22 16:17 ` Jan Beulich
2016-11-22 16:22 ` Wei Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.