[v2,7/7] gnttab: properly handle transfer slots when changing version

Commit Message

Jan Beulich Aug. 16, 2017, 10:51 a.m. UTC

Reported-by: Andrew Cooper <[e-mail]>
Signed-off-by: Jan Beulich <[e-mail]>
---
v2: New.
---
Or should we put ourselves on the position that reserved entries aren#t
meant to be used for transfers?

Comments

Andrew Cooper Aug. 16, 2017, 11:41 a.m. UTC | #1

On 16/08/17 11:51, Jan Beulich wrote:
> Reported-by: Andrew Cooper <[e-mail]>
> Signed-off-by: Jan Beulich <[e-mail]>
> ---
> v2: New.
> ---
> Or should we put ourselves on the position that reserved entries aren#t
> meant to be used for transfers?

Transfers are only usable by x86 PV (now, following XSA-214), and I
can't see any plausible situation for the toolstack constructing one.

I'd err on the side of fully disallowing them (also because it avoids
adding more conversion code to this function).

Also, it looks like the resulting code would be rather cleaner by
pulling shared_entry_v2(gt, i) out into a variable first.

~Andrew

Patch

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2755,6 +2755,11 @@  gnttab_set_version(XEN_GUEST_HANDLE_PARA
         {
             switch ( shared_entry_v2(gt, i).hdr.flags & GTF_type_mask )
             {
+            case GTF_accept_transfer:
+                 if ( !(shared_entry_v2(gt, i).hdr.flags &
+                        GTF_transfer_completed) )
+                     break;
+                 /* fall through */
             case GTF_permit_access:
                  if ( !(shared_entry_v2(gt, i).full_page.frame >> 32) )
                      break;
@@ -2784,13 +2789,17 @@  gnttab_set_version(XEN_GUEST_HANDLE_PARA
             switch ( flags & GTF_type_mask )
             {
             case GTF_permit_access:
-                reserved_entries[i].flags = flags | status_entry(gt, i);
+                flags |= status_entry(gt, i);
+                /* fall through */
+            case GTF_accept_transfer:
+                reserved_entries[i].flags = flags;
                 reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
                 reserved_entries[i].frame = shared_entry_v2(gt, i).full_page.frame;
                 break;
+
             default:
                 gdprintk(XENLOG_INFO,
-                         "bad flags %#x in grant %#x when switching version\n",
+                         "unstable flags %#x in grant %#x when switching version\n",
                          flags, i);
                 /* fall through */
             case GTF_invalid:
@@ -2819,10 +2828,20 @@  gnttab_set_version(XEN_GUEST_HANDLE_PARA
         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);
+                if ( (reserved_entries[i].flags & GTF_type_mask) !=
+                     GTF_accept_transfer )
+                {
+                    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);
+                }
+                else
+                {
+                    status_entry(gt, i) = 0;
+                    shared_entry_v2(gt, i).hdr.flags =
+                        reserved_entries[i].flags;
+                }
                 shared_entry_v2(gt, i).hdr.domid =
                     reserved_entries[i].domid;
                 shared_entry_v2(gt, i).full_page.frame =