[Xen-changelog] [xen stable-4.9] gnttab: correct GNTTABOP_cache_flush empty batch handling

[patchbot@xxxxxxx]

commit da8c866e20c54864f037a0101654dbae454e5a91
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Wed Dec 20 15:56:22 2017 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Dec 20 15:56:22 2017 +0100

    gnttab: correct GNTTABOP_cache_flush empty batch handling
    
    Jann validly points out that with a caller bogusly requesting a zero-
    element batch with non-zero high command bits (the ones used for
    continuation encoding), the assertion right before the call to
    hypercall_create_continuation() would trigger. A similar situation would
    arise afaict for non-empty batches with op and/or length zero in every
    element.
    
    While we want the former to succeed (as we do elsewhere for similar
    no-op requests), the latter can clearly be converted to an error, as
    this is a state that can't be the result of a prior operation.
    
    Take the opportunity and also correct the order of argument checks:
    We shouldn't accept zero-length elements with unknown bits set in "op".
    Also constify cache_flush()'s first parameter.
    
    Reported-by: Jann Horn <jannh@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
    Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
    master commit: 9c22e4d67f5552c7c896ed83bd95d5d4c5837a9d
    master date: 2017-12-04 11:03:32 +0100
---
 xen/common/grant_table.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 989c6c1..7ac0234 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3007,7 +3007,7 @@ 
gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) uop,
     return 0;
 }
 
-static int __gnttab_cache_flush(gnttab_cache_flush_t *cflush,
+static int __gnttab_cache_flush(const gnttab_cache_flush_t *cflush,
                                 unsigned int *ref_count)
 {
     struct domain *d, *owner;
@@ -3018,19 +3018,17 @@ static int __gnttab_cache_flush(gnttab_cache_flush_t 
*cflush,
 
     if ( (cflush->offset >= PAGE_SIZE) ||
          (cflush->length > PAGE_SIZE) ||
-         (cflush->offset + cflush->length > PAGE_SIZE) )
+         (cflush->offset + cflush->length > PAGE_SIZE) ||
+         (cflush->op & ~(GNTTAB_CACHE_INVAL | GNTTAB_CACHE_CLEAN)) )
         return -EINVAL;
 
     if ( cflush->length == 0 || cflush->op == 0 )
-        return 0;
+        return !*ref_count ? 0 : -EILSEQ;
 
     /* currently unimplemented */
     if ( cflush->op & GNTTAB_CACHE_SOURCE_GREF )
         return -EOPNOTSUPP;
 
-    if ( cflush->op & ~(GNTTAB_CACHE_INVAL|GNTTAB_CACHE_CLEAN) )
-        return -EINVAL;
-
     d = rcu_lock_current_domain();
     mfn = cflush->a.dev_bus_addr >> PAGE_SHIFT;
 
@@ -3110,6 +3108,9 @@ 
gnttab_cache_flush(XEN_GUEST_HANDLE_PARAM(gnttab_cache_flush_t) uop,
         *ref_count = 0;
         guest_handle_add_offset(uop, 1);
     }
+
+    *ref_count = 0;
+
     return 0;
 }
 
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.9

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog