Re: [PATCH v2 00/17] live update and gnttab patches

[Edwin Torok <edvin.torok@citrix.com>]

On Wed, 2021-05-12 at 13:51 +0100, Andrew Cooper wrote:
> On 12/05/2021 11:10, Edwin Torok wrote:
> > On Tue, 2021-05-11 at 21:05 +0100, Andrew Cooper wrote:
> > > 
> > diff --git a/tools/ocaml/xenstored/disk.ml
> > b/tools/ocaml/xenstored/disk.ml
> > index 59794324e1..b7678af87f 100644
> > --- a/tools/ocaml/xenstored/disk.ml
> > +++ b/tools/ocaml/xenstored/disk.ml
> > @@ -176,7 +176,7 @@ let write store =
> >             output_byte ch i
> >  
> >         let w32 ch v =
> > -           assert (v >= 0 && v <= 0xFFFF_FFFF);
> > +           assert (v >= 0 && Int64.of_int v <= 0xFFFF_FFFFL);
> 
> In the case that v is 32 bits wide, it will underflow and fail the v
> >=
> 0 check, before the upcast to Int64.

I'll have to review the callers of this, I think my intention was to
forbid dumping negative values because it is ambigous what it means.
In case you are running on 64-bit that is most likely a bug because I
think most 32-bit values were defined as unsigned in the migration spec
or in the original xen public headers (I'll have to double check).

However in case of a 32-bit system we can have negative values where an
otherwise unsigned 32-bit quantity in xen is represented as an ocaml
int, or even silently truncated (if the xen value actually uses all 32-
bits, because OCaml ints are only 31-bits on 32-bit systems, one would
have to use the int32 type to get true 32-bit quantities in ocaml but
that comes with additional boxing and a more complicated syntax,
so in most places in Xen I see the difference just being ignored).

Perhaps this should forbid negative values only on 64-bit systems
(where that would be a bug), and allow it on 32-bit systems (where a
negative value might be legitimate or a bug, we can't tell).
Checking Sys.word_size should tell us what system we are on.

Best regards,
--Edwin