[PATCH v2 0/3] gnttab: misc adjustments

[PATCH v2 0/3] gnttab: misc adjustments

[Jan Beulich]

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>

[PATCH v2 1/3 resend] gnttab: fix out of range shift count

[Jan Beulich]

[-- 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

[PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version()

[Jan Beulich]

[-- 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

[PATCH v2 3/3] gnttab: clean up gnttab_set_version()

[Jan Beulich]

[-- 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(&gt->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(&gt->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(&gt->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(&gt->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

Re: [PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version()

[Andrew Cooper]

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>

Re: [PATCH v2 3/3] gnttab: clean up gnttab_set_version()

[Andrew Cooper]

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.

Re: [PATCH v2 0/3] gnttab: misc adjustments

[Ian Campbell]

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>