| Message ID | 5CF7D1F30200007800235943@prv1-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | memory: don't depend on guest_handle_subrange_okay() implementation details | expand |
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>
--- 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) )
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>