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

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

  • To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Edwin Torok <edvin.torok@xxxxxxxxxx>
  • Date: Wed, 12 May 2021 15:04:25 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=XfrM3QD/84DYsI1vmmrpIh9pwqCM+vWCmFicTrB9Mwk=; b=kOkit8Gii5bCESK2UrzBaykQ+s7hxRvYdXtSrQeDWEPFyl/lU19QTzzHrrpwl+rZvEuF/Ojb1+grf8mtb1qNuMVN2VbjSpEP273wW5MuLqwaDJlEaxiz82TFTjMwj/58NunN6iT0o5czip+gDISAvNP9iKpJa3KjRxDEl6Y9P0Ct+riTO9+Qc1ZGluPWBOF5JzltIhrR0dQVuCgnQJL5AUpGIy9ADM5N4xyKT0n8QVIyIfYQR5AeG2kOqFKpbOvXt5WXv08jXzwUZS4xF2UKlYf3nBq6ZMlDUzqpXsDxB9QFUZQJANGvwQCPANhO1EFgKyKkvgW7JdjfTwzjBRSBKg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MXdHCXaETK534/OomAp0UdL2Fay5N55ZAoKd2rSAQtBqiuC4WrTuezwEsMzK3ZF/DNZo5RtbZDUC/G4rToTGPvZtbHRSnDiMWeDx/WfxEy1eX4dgtbwKeCgZWFLvR+KyZt0VEResEJ4byYZywnWigg6nqzH/mEacJ2xqbe9SnyL00VEwRQiPuNzkTDq/kK/U+0ieROOTFdgOvnmQKsV9QrPz3vzxaHD/Ott0h9QX6X1BGapBIHLoLyYEeoxnHSD/muYpY8NIhY1Ox5t+g9dZfEQW9bcWAN8cp7yZwLos6G/f6kie3btBwTmaFE9j8i0s+v1txFeDonIC27vGp6RRCA==
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "jbeulich@xxxxxxxx" <jbeulich@xxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, "jgross@xxxxxxxx" <jgross@xxxxxxxx>, "wl@xxxxxxx" <wl@xxxxxxx>, "iwj@xxxxxxxxxxxxxx" <iwj@xxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "dave@xxxxxxxxxx" <dave@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>, Christian Lindig <christian.lindig@xxxxxxxxxx>
  • Delivery-date: Wed, 12 May 2021 15:04:42 +0000
  • Ironport-hdrordr: A9a23:8yQTGq8/M6ORRYjkHlhuk+AiI+orL9Y04lQ7vn2ZKSY5TiVXra CTdZUgpHvJYVMqMk3I9uruBEDtex3hHP1OkOws1NWZLWrbUQKTRekP0WKL+Vbd8kbFh4xgPM lbEpSXCLfLfCVHZcSR2njFLz73quP3j5xBho3lvglQpRkBUdAG0+/gYDzraXGfQmN9dPwEPa vZ3OVrjRy6d08aa8yqb0N1JdQq97Xw5evbiQdtPW9e1DWz
  • Ironport-sdr: 7buVpZ3J6iml1QF7cMyc/n7430L9oeNjIzCY8cC+XQgPLp0dh+GhxuOQgJ0fC6g5fTkd3isLaM SFrJFKtWtnzEFt8DJN5zEHGshGcXehcoHXCptao4AqpmWxqfsMDHOR+tPIzaOx0PUPwtBHY/IH +qFQsXTD8DHzh7Rz/iHihxpakD73UWGMo5ZtE0IPH5jwd4wyhZRqnnKgApq/RP+CAv3MJruqxj B3pmit51gJ9F76dpUqy9/bebzMiKxHN/4pH2E1Fsmw6rtGhTi/R6uVFYnes5cP0q1kIde9Js75 xsg=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHXRpBYbSP9+kXyy06RGaCma1VeS6retOQAgADsBACAACz3AIAAJTAA
  • Thread-topic: [PATCH v2 00/17] live update and gnttab patches

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,



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