[Xen-devel] [PATCH] xen/grant: Fix signed/unsigned comparisons issues

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Each function takes an unsigned count, and loops based on a signed i.  This
causes problems when between 2 and 4 billion operations are requested.

In practice, signed-ness issues aren't possible because count exceeding
INT_MAX is excluded earlier in do_grant_op(), but the code reads as if it is
buggy, and GCC obviously can't spot this either.

Bloat-o-meter reports:
  add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-95 (-95)
  Function                                     old     new   delta
  do_grant_table_op                           7155    7140     -15
  gnttab_transfer                             2732    2716     -16
  gnttab_unmap_grant_ref                       771     739     -32
  gnttab_unmap_and_replace                     771     739     -32
  Total: Before=2996364, After=2996269, chg -0.00%

and inspection of gnttab_unmap_grant_ref() at least reveals one fewer local
variables on the stack.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Julien Grall <julien@xxxxxxx>

gnttab_unmap_grant_ref()'s stack frame size is 0x740 (dropping to 0x738) which
is alarmingly close to 2k.
---
 xen/common/grant_table.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index bc37acae0e..0f81875bee 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1270,7 +1270,7 @@ static long
 gnttab_map_grant_ref(
     XEN_GUEST_HANDLE_PARAM(gnttab_map_grant_ref_t) uop, unsigned int count)
 {
-    int i;
+    unsigned int i;
     struct gnttab_map_grant_ref op;
 
     for ( i = 0; i < count; i++ )
@@ -1568,13 +1568,14 @@ static long
 gnttab_unmap_grant_ref(
     XEN_GUEST_HANDLE_PARAM(gnttab_unmap_grant_ref_t) uop, unsigned int count)
 {
-    int i, c, partial_done, done = 0;
+    unsigned int i, partial_done, done = 0;
     struct gnttab_unmap_grant_ref op;
     struct gnttab_unmap_common common[GNTTAB_UNMAP_BATCH_SIZE];
 
     while ( count != 0 )
     {
-        c = min(count, (unsigned int)GNTTAB_UNMAP_BATCH_SIZE);
+        unsigned int c = min(count, (unsigned int)GNTTAB_UNMAP_BATCH_SIZE);
+
         partial_done = 0;
 
         for ( i = 0; i < c; i++ )
@@ -1633,13 +1634,14 @@ static long
 gnttab_unmap_and_replace(
     XEN_GUEST_HANDLE_PARAM(gnttab_unmap_and_replace_t) uop, unsigned int count)
 {
-    int i, c, partial_done, done = 0;
+    unsigned int i, partial_done, done = 0;
     struct gnttab_unmap_and_replace op;
     struct gnttab_unmap_common common[GNTTAB_UNMAP_BATCH_SIZE];
 
     while ( count != 0 )
     {
-        c = min(count, (unsigned int)GNTTAB_UNMAP_BATCH_SIZE);
+        unsigned int c = min(count, (unsigned int)GNTTAB_UNMAP_BATCH_SIZE);
+
         partial_done = 0;
 
         for ( i = 0; i < c; i++ )
@@ -2142,7 +2144,7 @@ gnttab_transfer(
     struct domain *d = current->domain;
     struct domain *e;
     struct page_info *page;
-    int i;
+    unsigned int i;
     struct gnttab_transfer gop;
     mfn_t mfn;
     unsigned int max_bitsize;
@@ -3359,7 +3361,7 @@ static long
 gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) uop,
                       unsigned int count)
 {
-    int i;
+    unsigned int i;
     gnttab_swap_grant_ref_t op;
 
     for ( i = 0; i < count; i++ )
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel