[Xen-devel] [PATCH] memory: don't depend on guest_handle_subrange_okay() implementation details

[Xen-devel] [PATCH] memory: don't depend on guest_handle_subrange_okay() implementation details

[Jan Beulich]

guest_handle_subrange_okay() takes inclusive first and last parameters,
i.e. checks that [first, last] is valid. Many callers, however, actually
need to see whether [first, limit) is valid (i.e., limit is non-
inclusive), and to do this they subtract 1 from the size. This is
normally correct, except in cases where first == limit, in which case
guest_handle_subrange_okay() will be passed a second parameter less than
its first.

As it happens, due to the way the math is implemented in x86's
guest_handle_subrange_okay(), the return value turns out to be correct;
but we shouldn’t rely on this behavior.

Make sure all callers handle first == limit explicitly before calling
guest_handle_subrange_okay().

Note that the other uses (increase-reservation, populate-physmap, and
decrease-reservation) are already fine due to a suitable check in
do_memory_op().

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -541,6 +541,9 @@ static long memory_exchange(XEN_GUEST_HA
         goto fail_early;
     }
 
+    if ( exch.nr_exchanged == exch.in.nr_extents )
+        return 0;
+
     if ( !guest_handle_subrange_okay(exch.in.extent_start, exch.nr_exchanged,
                                      exch.in.nr_extents - 1) )
     {
@@ -866,9 +869,12 @@ static int xenmem_add_to_physmap_batch(s
                                        struct xen_add_to_physmap_batch *xatpb,
                                        unsigned int extent)
 {
-    if ( xatpb->size < extent )
+    if ( unlikely(xatpb->size < extent) )
         return -EILSEQ;
 
+    if ( unlikely(xatpb->size == extent) )
+        return extent ? -EILSEQ : 0;
+
     if ( !guest_handle_subrange_okay(xatpb->idxs, extent, xatpb->size - 1) ||
          !guest_handle_subrange_okay(xatpb->gpfns, extent, xatpb->size - 1) ||
          !guest_handle_subrange_okay(xatpb->errs, extent, xatpb->size - 1) )




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

Re: [Xen-devel] [PATCH] memory: don't depend on guest_handle_subrange_okay() implementation details

[George Dunlap]

On Wed, Jun 5, 2019 at 3:30 PM Jan Beulich <JBeulich@suse.com> wrote:
>
> guest_handle_subrange_okay() takes inclusive first and last parameters,
> i.e. checks that [first, last] is valid. Many callers, however, actually
> need to see whether [first, limit) is valid (i.e., limit is non-
> inclusive), and to do this they subtract 1 from the size. This is
> normally correct, except in cases where first == limit, in which case
> guest_handle_subrange_okay() will be passed a second parameter less than
> its first.
>
> As it happens, due to the way the math is implemented in x86's
> guest_handle_subrange_okay(), the return value turns out to be correct;
> but we shouldn’t rely on this behavior.
>
> Make sure all callers handle first == limit explicitly before calling
> guest_handle_subrange_okay().
>
> Note that the other uses (increase-reservation, populate-physmap, and
> decrease-reservation) are already fine due to a suitable check in
> do_memory_op().
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

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