* [PATCH v2 0/3] gnttab: misc adjustments
@ 2015-06-22 11:39 Jan Beulich
2015-06-22 11:44 ` [PATCH v2 1/3 resend] gnttab: fix out of range shift count Jan Beulich
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Jan Beulich @ 2015-06-22 11:39 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell, Keir Fraser, Ian Jackson, Tim Deegan
Patch 1 is a plain resend. Patches 2 and 3 are the split result of a
previously sent patch.
1: fix out of range shift count
2: don't silently truncate frame numbers in gnttab_set_version()
3: clean up gnttab_set_version()
Signed-off-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3 resend] gnttab: fix out of range shift count
2015-06-22 11:39 [PATCH v2 0/3] gnttab: misc adjustments Jan Beulich
@ 2015-06-22 11:44 ` Jan Beulich
2015-06-22 11:45 ` [PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version() Jan Beulich
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2015-06-22 11:44 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell, Keir Fraser, Ian Jackson, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 762 bytes --]
Commit 213f145114 ("gnttab: fix/adjust gnttab_transfer()") wasn't
careful enough in this regard.
Coverity ID: 1306859
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1829,7 +1829,8 @@ gnttab_transfer(
max_bitsize = domain_clamp_alloc_bitsize(
e, e->grant_table->gt_version > 1 || paging_mode_translate(e)
? BITS_PER_LONG + PAGE_SHIFT : 32 + PAGE_SHIFT);
- if ( (1UL << (max_bitsize - PAGE_SHIFT)) <= mfn )
+ if ( max_bitsize < BITS_PER_LONG + PAGE_SHIFT &&
+ (mfn >> (max_bitsize - PAGE_SHIFT)) )
{
struct page_info *new_page;
[-- Attachment #2: gnttab-transfer-shift-count.patch --]
[-- Type: text/plain, Size: 796 bytes --]
gnttab: fix out of range shift count
Commit 213f145114 ("gnttab: fix/adjust gnttab_transfer()") wasn't
careful enough in this regard.
Coverity ID: 1306859
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1829,7 +1829,8 @@ gnttab_transfer(
max_bitsize = domain_clamp_alloc_bitsize(
e, e->grant_table->gt_version > 1 || paging_mode_translate(e)
? BITS_PER_LONG + PAGE_SHIFT : 32 + PAGE_SHIFT);
- if ( (1UL << (max_bitsize - PAGE_SHIFT)) <= mfn )
+ if ( max_bitsize < BITS_PER_LONG + PAGE_SHIFT &&
+ (mfn >> (max_bitsize - PAGE_SHIFT)) )
{
struct page_info *new_page;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version()
2015-06-22 11:39 [PATCH v2 0/3] gnttab: misc adjustments Jan Beulich
2015-06-22 11:44 ` [PATCH v2 1/3 resend] gnttab: fix out of range shift count Jan Beulich
@ 2015-06-22 11:45 ` Jan Beulich
2015-06-22 11:51 ` Andrew Cooper
2015-06-22 11:46 ` [PATCH v2 3/3] gnttab: clean up gnttab_set_version() Jan Beulich
2015-07-06 15:59 ` [PATCH v2 0/3] gnttab: misc adjustments Ian Campbell
3 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2015-06-22 11:45 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell, Keir Fraser, Ian Jackson, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]
On a v2 -> v1 transition frame numbers previously stored in a 64-bit
field have to fit into a 32-bit one.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2597,14 +2597,32 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
}
}
- /* XXX: If we're going to version 2, we could maybe shrink the
- active grant table here. */
-
- if ( op.version == 2 && gt->gt_version < 2 )
+ switch ( gt->gt_version )
{
- res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
- if ( res < 0)
- goto out_unlock;
+ case 0:
+ if ( op.version == 2 )
+ {
+ case 1:
+ /* XXX: We could maybe shrink the active grant table here. */
+ res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
+ if ( res < 0)
+ goto out_unlock;
+ }
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ {
+ if ( ((shared_entry_v2(gt, i).hdr.flags & GTF_type_mask) ==
+ GTF_permit_access) &&
+ (shared_entry_v2(gt, i).full_page.frame >> 32) )
+ {
+ gdprintk(XENLOG_WARNING,
+ "tried to change grant table version to 1 with non-representable entries\n");
+ res = -ERANGE;
+ goto out_unlock;
+ }
+ }
+ break;
}
/* Preserve the first 8 entries (toolstack reserved grants) */
[-- Attachment #2: gnttab-set-version-no-truncate.patch --]
[-- Type: text/plain, Size: 1664 bytes --]
gnttab: don't silently truncate frame numbers in gnttab_set_version()
On a v2 -> v1 transition frame numbers previously stored in a 64-bit
field have to fit into a 32-bit one.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2597,14 +2597,32 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
}
}
- /* XXX: If we're going to version 2, we could maybe shrink the
- active grant table here. */
-
- if ( op.version == 2 && gt->gt_version < 2 )
+ switch ( gt->gt_version )
{
- res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
- if ( res < 0)
- goto out_unlock;
+ case 0:
+ if ( op.version == 2 )
+ {
+ case 1:
+ /* XXX: We could maybe shrink the active grant table here. */
+ res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
+ if ( res < 0)
+ goto out_unlock;
+ }
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ {
+ if ( ((shared_entry_v2(gt, i).hdr.flags & GTF_type_mask) ==
+ GTF_permit_access) &&
+ (shared_entry_v2(gt, i).full_page.frame >> 32) )
+ {
+ gdprintk(XENLOG_WARNING,
+ "tried to change grant table version to 1 with non-representable entries\n");
+ res = -ERANGE;
+ goto out_unlock;
+ }
+ }
+ break;
}
/* Preserve the first 8 entries (toolstack reserved grants) */
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] gnttab: clean up gnttab_set_version()
2015-06-22 11:39 [PATCH v2 0/3] gnttab: misc adjustments Jan Beulich
2015-06-22 11:44 ` [PATCH v2 1/3 resend] gnttab: fix out of range shift count Jan Beulich
2015-06-22 11:45 ` [PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version() Jan Beulich
@ 2015-06-22 11:46 ` Jan Beulich
2015-06-22 12:01 ` Andrew Cooper
2015-07-06 15:59 ` [PATCH v2 0/3] gnttab: misc adjustments Ian Campbell
3 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2015-06-22 11:46 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell, Keir Fraser, Ian Jackson, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 7409 bytes --]
- drop pointless nr_grant_entries() check from loop over reserved
entries (adding suitable BUILD_BUG_ON()s to validate that)
- adjust types
- rename d to currd
- formatting
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -465,10 +465,16 @@ static unsigned int nr_grant_entries(str
{
switch ( gt->gt_version )
{
+#define f2e(nr, ver) (((nr) << PAGE_SHIFT) / sizeof(grant_entry_v##ver##_t))
case 1:
- return (nr_grant_frames(gt) << PAGE_SHIFT) / sizeof(grant_entry_v1_t);
+ BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 1) <
+ GNTTAB_NR_RESERVED_ENTRIES);
+ return f2e(nr_grant_frames(gt), 1);
case 2:
- return (nr_grant_frames(gt) << PAGE_SHIFT) / sizeof(grant_entry_v2_t);
+ BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 2) <
+ GNTTAB_NR_RESERVED_ENTRIES);
+ return f2e(nr_grant_frames(gt), 2);
+#undef f2e
}
return 0;
@@ -2563,17 +2569,17 @@ static long
gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
{
gnttab_set_version_t op;
- struct domain *d = current->domain;
- struct grant_table *gt = d->grant_table;
+ struct domain *currd = current->domain;
+ struct grant_table *gt = currd->grant_table;
grant_entry_v1_t reserved_entries[GNTTAB_NR_RESERVED_ENTRIES];
- long res;
- int i;
+ int res;
+ unsigned int i;
- if (copy_from_guest(&op, uop, 1))
+ if ( copy_from_guest(&op, uop, 1) )
return -EFAULT;
res = -EINVAL;
- if (op.version != 1 && op.version != 2)
+ if ( op.version != 1 && op.version != 2 )
goto out;
res = 0;
@@ -2581,10 +2587,12 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
goto out;
write_lock(>->lock);
- /* Make sure that the grant table isn't currently in use when we
- change the version number, except for the first 8 entries which
- are allowed to be in use (xenstore/xenconsole keeps them mapped).
- (You need to change the version number for e.g. kexec.) */
+ /*
+ * Make sure that the grant table isn't currently in use when we
+ * change the version number, except for the first 8 entries which
+ * are allowed to be in use (xenstore/xenconsole keeps them mapped).
+ * (You need to change the version number for e.g. kexec.)
+ */
for ( i = GNTTAB_NR_RESERVED_ENTRIES; i < nr_grant_entries(gt); i++ )
{
if ( read_atomic(&_active_entry(gt, i).pin) != 0 )
@@ -2604,7 +2612,7 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
{
case 1:
/* XXX: We could maybe shrink the active grant table here. */
- res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
+ res = gnttab_populate_status_frames(currd, gt, nr_grant_frames(gt));
if ( res < 0)
goto out_unlock;
}
@@ -2625,66 +2633,78 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
break;
}
- /* Preserve the first 8 entries (toolstack reserved grants) */
- if ( gt->gt_version == 1 )
- {
- memcpy(reserved_entries, &shared_entry_v1(gt, 0), sizeof(reserved_entries));
- }
- else if ( gt->gt_version == 2 )
+ /* Preserve the first 8 entries (toolstack reserved grants). */
+ switch ( gt->gt_version )
{
- for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES && i < nr_grant_entries(gt); i++ )
+ case 1:
+ memcpy(reserved_entries, &shared_entry_v1(gt, 0),
+ sizeof(reserved_entries));
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
{
- int flags = status_entry(gt, i);
- flags |= shared_entry_v2(gt, i).hdr.flags;
- if ((flags & GTF_type_mask) == GTF_permit_access)
+ unsigned int flags = shared_entry_v2(gt, i).hdr.flags;
+
+ switch ( flags & GTF_type_mask )
{
- reserved_entries[i].flags = flags;
+ case GTF_permit_access:
+ reserved_entries[i].flags = flags | status_entry(gt, i);
reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
reserved_entries[i].frame = shared_entry_v2(gt, i).full_page.frame;
- }
- else
- {
- if ((flags & GTF_type_mask) != GTF_invalid)
- gdprintk(XENLOG_INFO, "d%d: bad flags %x in grant %d when switching grant version\n",
- d->domain_id, flags, i);
+ break;
+ default:
+ gdprintk(XENLOG_INFO,
+ "bad flags %x in grant %u when switching version\n",
+ flags, i);
+ /* fall through */
+ case GTF_invalid:
memset(&reserved_entries[i], 0, sizeof(reserved_entries[i]));
+ break;
}
}
+ break;
}
if ( op.version < 2 && gt->gt_version == 2 )
- gnttab_unpopulate_status_frames(d, gt);
+ gnttab_unpopulate_status_frames(currd, gt);
- /* Make sure there's no crud left over in the table from the
- old version. */
+ /* Make sure there's no crud left over from the old version. */
for ( i = 0; i < nr_grant_frames(gt); i++ )
clear_page(gt->shared_raw[i]);
- /* Restore the first 8 entries (toolstack reserved grants) */
- if ( gt->gt_version != 0 && op.version == 1 )
- {
- memcpy(&shared_entry_v1(gt, 0), reserved_entries, sizeof(reserved_entries));
- }
- else if ( gt->gt_version != 0 && op.version == 2 )
+ /* Restore the first 8 entries (toolstack reserved grants). */
+ if ( gt->gt_version )
{
- for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ switch ( op.version )
{
- status_entry(gt, i) = reserved_entries[i].flags & (GTF_reading|GTF_writing);
- shared_entry_v2(gt, i).hdr.flags = reserved_entries[i].flags & ~(GTF_reading|GTF_writing);
- shared_entry_v2(gt, i).hdr.domid = reserved_entries[i].domid;
- shared_entry_v2(gt, i).full_page.frame = reserved_entries[i].frame;
+ case 1:
+ memcpy(&shared_entry_v1(gt, 0), reserved_entries, sizeof(reserved_entries));
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ {
+ status_entry(gt, i) =
+ reserved_entries[i].flags & (GTF_reading | GTF_writing);
+ shared_entry_v2(gt, i).hdr.flags =
+ reserved_entries[i].flags & ~(GTF_reading | GTF_writing);
+ shared_entry_v2(gt, i).hdr.domid =
+ reserved_entries[i].domid;
+ shared_entry_v2(gt, i).full_page.frame =
+ reserved_entries[i].frame;
+ }
+ break;
}
}
gt->gt_version = op.version;
-out_unlock:
+ out_unlock:
write_unlock(>->lock);
-out:
+ out:
op.version = gt->gt_version;
- if (__copy_to_guest(uop, &op, 1))
+ if ( __copy_to_guest(uop, &op, 1) )
res = -EFAULT;
return res;
[-- Attachment #2: gnttab-set-version-cleanup.patch --]
[-- Type: text/plain, Size: 7446 bytes --]
gnttab: clean up gnttab_set_version()
- drop pointless nr_grant_entries() check from loop over reserved
entries (adding suitable BUILD_BUG_ON()s to validate that)
- adjust types
- rename d to currd
- formatting
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -465,10 +465,16 @@ static unsigned int nr_grant_entries(str
{
switch ( gt->gt_version )
{
+#define f2e(nr, ver) (((nr) << PAGE_SHIFT) / sizeof(grant_entry_v##ver##_t))
case 1:
- return (nr_grant_frames(gt) << PAGE_SHIFT) / sizeof(grant_entry_v1_t);
+ BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 1) <
+ GNTTAB_NR_RESERVED_ENTRIES);
+ return f2e(nr_grant_frames(gt), 1);
case 2:
- return (nr_grant_frames(gt) << PAGE_SHIFT) / sizeof(grant_entry_v2_t);
+ BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 2) <
+ GNTTAB_NR_RESERVED_ENTRIES);
+ return f2e(nr_grant_frames(gt), 2);
+#undef f2e
}
return 0;
@@ -2563,17 +2569,17 @@ static long
gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
{
gnttab_set_version_t op;
- struct domain *d = current->domain;
- struct grant_table *gt = d->grant_table;
+ struct domain *currd = current->domain;
+ struct grant_table *gt = currd->grant_table;
grant_entry_v1_t reserved_entries[GNTTAB_NR_RESERVED_ENTRIES];
- long res;
- int i;
+ int res;
+ unsigned int i;
- if (copy_from_guest(&op, uop, 1))
+ if ( copy_from_guest(&op, uop, 1) )
return -EFAULT;
res = -EINVAL;
- if (op.version != 1 && op.version != 2)
+ if ( op.version != 1 && op.version != 2 )
goto out;
res = 0;
@@ -2581,10 +2587,12 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
goto out;
write_lock(>->lock);
- /* Make sure that the grant table isn't currently in use when we
- change the version number, except for the first 8 entries which
- are allowed to be in use (xenstore/xenconsole keeps them mapped).
- (You need to change the version number for e.g. kexec.) */
+ /*
+ * Make sure that the grant table isn't currently in use when we
+ * change the version number, except for the first 8 entries which
+ * are allowed to be in use (xenstore/xenconsole keeps them mapped).
+ * (You need to change the version number for e.g. kexec.)
+ */
for ( i = GNTTAB_NR_RESERVED_ENTRIES; i < nr_grant_entries(gt); i++ )
{
if ( read_atomic(&_active_entry(gt, i).pin) != 0 )
@@ -2604,7 +2612,7 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
{
case 1:
/* XXX: We could maybe shrink the active grant table here. */
- res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
+ res = gnttab_populate_status_frames(currd, gt, nr_grant_frames(gt));
if ( res < 0)
goto out_unlock;
}
@@ -2625,66 +2633,78 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
break;
}
- /* Preserve the first 8 entries (toolstack reserved grants) */
- if ( gt->gt_version == 1 )
- {
- memcpy(reserved_entries, &shared_entry_v1(gt, 0), sizeof(reserved_entries));
- }
- else if ( gt->gt_version == 2 )
+ /* Preserve the first 8 entries (toolstack reserved grants). */
+ switch ( gt->gt_version )
{
- for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES && i < nr_grant_entries(gt); i++ )
+ case 1:
+ memcpy(reserved_entries, &shared_entry_v1(gt, 0),
+ sizeof(reserved_entries));
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
{
- int flags = status_entry(gt, i);
- flags |= shared_entry_v2(gt, i).hdr.flags;
- if ((flags & GTF_type_mask) == GTF_permit_access)
+ unsigned int flags = shared_entry_v2(gt, i).hdr.flags;
+
+ switch ( flags & GTF_type_mask )
{
- reserved_entries[i].flags = flags;
+ case GTF_permit_access:
+ reserved_entries[i].flags = flags | status_entry(gt, i);
reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
reserved_entries[i].frame = shared_entry_v2(gt, i).full_page.frame;
- }
- else
- {
- if ((flags & GTF_type_mask) != GTF_invalid)
- gdprintk(XENLOG_INFO, "d%d: bad flags %x in grant %d when switching grant version\n",
- d->domain_id, flags, i);
+ break;
+ default:
+ gdprintk(XENLOG_INFO,
+ "bad flags %x in grant %u when switching version\n",
+ flags, i);
+ /* fall through */
+ case GTF_invalid:
memset(&reserved_entries[i], 0, sizeof(reserved_entries[i]));
+ break;
}
}
+ break;
}
if ( op.version < 2 && gt->gt_version == 2 )
- gnttab_unpopulate_status_frames(d, gt);
+ gnttab_unpopulate_status_frames(currd, gt);
- /* Make sure there's no crud left over in the table from the
- old version. */
+ /* Make sure there's no crud left over from the old version. */
for ( i = 0; i < nr_grant_frames(gt); i++ )
clear_page(gt->shared_raw[i]);
- /* Restore the first 8 entries (toolstack reserved grants) */
- if ( gt->gt_version != 0 && op.version == 1 )
- {
- memcpy(&shared_entry_v1(gt, 0), reserved_entries, sizeof(reserved_entries));
- }
- else if ( gt->gt_version != 0 && op.version == 2 )
+ /* Restore the first 8 entries (toolstack reserved grants). */
+ if ( gt->gt_version )
{
- for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ switch ( op.version )
{
- status_entry(gt, i) = reserved_entries[i].flags & (GTF_reading|GTF_writing);
- shared_entry_v2(gt, i).hdr.flags = reserved_entries[i].flags & ~(GTF_reading|GTF_writing);
- shared_entry_v2(gt, i).hdr.domid = reserved_entries[i].domid;
- shared_entry_v2(gt, i).full_page.frame = reserved_entries[i].frame;
+ case 1:
+ memcpy(&shared_entry_v1(gt, 0), reserved_entries, sizeof(reserved_entries));
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ {
+ status_entry(gt, i) =
+ reserved_entries[i].flags & (GTF_reading | GTF_writing);
+ shared_entry_v2(gt, i).hdr.flags =
+ reserved_entries[i].flags & ~(GTF_reading | GTF_writing);
+ shared_entry_v2(gt, i).hdr.domid =
+ reserved_entries[i].domid;
+ shared_entry_v2(gt, i).full_page.frame =
+ reserved_entries[i].frame;
+ }
+ break;
}
}
gt->gt_version = op.version;
-out_unlock:
+ out_unlock:
write_unlock(>->lock);
-out:
+ out:
op.version = gt->gt_version;
- if (__copy_to_guest(uop, &op, 1))
+ if ( __copy_to_guest(uop, &op, 1) )
res = -EFAULT;
return res;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version()
2015-06-22 11:45 ` [PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version() Jan Beulich
@ 2015-06-22 11:51 ` Andrew Cooper
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Cooper @ 2015-06-22 11:51 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Ian Campbell, Ian Jackson, Keir Fraser, Tim Deegan
On 22/06/15 12:45, Jan Beulich wrote:
> On a v2 -> v1 transition frame numbers previously stored in a 64-bit
> field have to fit into a 32-bit one.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] gnttab: clean up gnttab_set_version()
2015-06-22 11:46 ` [PATCH v2 3/3] gnttab: clean up gnttab_set_version() Jan Beulich
@ 2015-06-22 12:01 ` Andrew Cooper
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Cooper @ 2015-06-22 12:01 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Ian Campbell, Ian Jackson, Keir Fraser, Tim Deegan
On 22/06/15 12:46, Jan Beulich wrote:
> - drop pointless nr_grant_entries() check from loop over reserved
> entries (adding suitable BUILD_BUG_ON()s to validate that)
> - adjust types
> - rename d to currd
> - formatting
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with one suggestion.
> @@ -2625,66 +2633,78 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
> break;
> }
>
> - /* Preserve the first 8 entries (toolstack reserved grants) */
> - if ( gt->gt_version == 1 )
> - {
> - memcpy(reserved_entries, &shared_entry_v1(gt, 0), sizeof(reserved_entries));
> - }
> - else if ( gt->gt_version == 2 )
> + /* Preserve the first 8 entries (toolstack reserved grants). */
> + switch ( gt->gt_version )
> {
> - for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES && i < nr_grant_entries(gt); i++ )
> + case 1:
> + memcpy(reserved_entries, &shared_entry_v1(gt, 0),
> + sizeof(reserved_entries));
> + break;
> + case 2:
> + for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
> {
> - int flags = status_entry(gt, i);
> - flags |= shared_entry_v2(gt, i).hdr.flags;
> - if ((flags & GTF_type_mask) == GTF_permit_access)
> + unsigned int flags = shared_entry_v2(gt, i).hdr.flags;
> +
> + switch ( flags & GTF_type_mask )
> {
> - reserved_entries[i].flags = flags;
> + case GTF_permit_access:
> + reserved_entries[i].flags = flags | status_entry(gt, i);
> reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
> reserved_entries[i].frame = shared_entry_v2(gt, i).full_page.frame;
> - }
> - else
> - {
> - if ((flags & GTF_type_mask) != GTF_invalid)
> - gdprintk(XENLOG_INFO, "d%d: bad flags %x in grant %d when switching grant version\n",
> - d->domain_id, flags, i);
> + break;
> + default:
> + gdprintk(XENLOG_INFO,
> + "bad flags %x in grant %u when switching version\n",
> + flags, i);
A 0x for flags, to avoid decimal confusion.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] gnttab: misc adjustments
2015-06-22 11:39 [PATCH v2 0/3] gnttab: misc adjustments Jan Beulich
` (2 preceding siblings ...)
2015-06-22 11:46 ` [PATCH v2 3/3] gnttab: clean up gnttab_set_version() Jan Beulich
@ 2015-07-06 15:59 ` Ian Campbell
3 siblings, 0 replies; 7+ messages in thread
From: Ian Campbell @ 2015-07-06 15:59 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Ian Jackson, Tim Deegan
On Mon, 2015-06-22 at 12:39 +0100, Jan Beulich wrote:
> Patch 1 is a plain resend. Patches 2 and 3 are the split result of a
> previously sent patch.
>
> 1: fix out of range shift count
> 2: don't silently truncate frame numbers in gnttab_set_version()
> 3: clean up gnttab_set_version()
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Given Andy has reviewed these and is happy:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-07-06 16:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-22 11:39 [PATCH v2 0/3] gnttab: misc adjustments Jan Beulich
2015-06-22 11:44 ` [PATCH v2 1/3 resend] gnttab: fix out of range shift count Jan Beulich
2015-06-22 11:45 ` [PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version() Jan Beulich
2015-06-22 11:51 ` Andrew Cooper
2015-06-22 11:46 ` [PATCH v2 3/3] gnttab: clean up gnttab_set_version() Jan Beulich
2015-06-22 12:01 ` Andrew Cooper
2015-07-06 15:59 ` [PATCH v2 0/3] gnttab: misc adjustments Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).