[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/grants: fix hypercall continuation for GNTTABOP_cache_flush



Hi Juergen,

On 22/04/2020 14:07, Juergen Gross wrote:
The GNTTABOP_cache_flush hypercall has a wrong test for hypercall
continuation, the test today is:

     if ( rc > 0 || opaque_out != 0 )

Unfortunately this will be true even in case of an error (rc < 0),
possibly leading to very long lasting hypercalls (times of more
than an hour have been observed in a test case).

Correct the test condition to result in false with rc < 0 and set
opaque_out only if no error occurred, to be on the safe side.

Partially-suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
  xen/common/grant_table.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 96080b3dec..5ef7ff940d 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3626,12 +3626,12 @@ do_grant_table_op(
          if ( unlikely(!guest_handle_okay(cflush, count)) )
              goto out;
          rc = gnttab_cache_flush(cflush, &opaque_in, count);
-        if ( rc > 0 )
+        if ( rc >= 0 )
          {
              guest_handle_add_offset(cflush, rc);
              uop = guest_handle_cast(cflush, void);
+            opaque_out = opaque_in;
          }
-        opaque_out = opaque_in;
          break;
      }
@@ -3641,7 +3641,7 @@ do_grant_table_op(
      }
out:
-    if ( rc > 0 || opaque_out != 0 )
+    if ( rc > 0 || (opaque_out != 0 && rc == 0) )

I don't understand this change. If you look at the implementation of gnttab_flush() it is not possible to have opaque_out non-zero with rc = 0.

So what's the point of the second condition?

      {
          /* Adjust rc, see gnttab_copy() for why this is needed. */
          if ( cmd == GNTTABOP_copy )


Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.