Re: [Xen-devel] [PATCH v3 8/8] libxl, hvmloader: Don't relocate memory for MMIO hole

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Stefano Stabellini writes ("Re: [PATCH v3 8/8] libxl,hvmloader: Don't relocate 
memory for MMIO hole"):
> On Fri, 21 Jun 2013, George Dunlap wrote:
> > The only valid values here are "0" and "1"; everything else is undefined.
> 
> This code doesn't do what you say: "0" means false and everything else
> means true. The undefined values are treated as true. Is that what we
> want?

I think it's acceptable.  Many other kernel-level and embedded
consumers of xenstore do similar things.  This is not a
security-relevant boundary and anyway the "wrong" behaviours are all
tolerable; furthermore hvmloader doesn't have a particularly good way
to report errors.

> In order to do what you say you would need:
> 
> bool allow_memory_relocate = 1;
> int i;
> i = strtoll(s, NULL, 0);

No, you need to also
  - pass a non-NULL 2nd argument and check that on return it
    points to null (in case the string had nondigits in it)
  - check that the input string is not empty (eg by checking
    that the end pointer returned via the 2nd argument
    is not equal to the start of the string)
  - set errno to 0 beforehand and check it afterwards
    (in case of ERANGE) (and anyway I bet hvmloader's strtoll
    gets this wrong so it's probably pointless)

This is far too much faff and is ultimately pointless.

I think we should say "0" and "1" are currently defined and other
integer values are reserved and should be treated the same way as "1".

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel