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
