Re: [Xen-devel] XENMEM_maximum_gpfn return value

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.02.13 at 16:27, Tim Deegan <tim@xxxxxxx> wrote:
> At 14:33 +0000 on 25 Feb (1361802812), Jan Beulich wrote:
>> This coming from a shared info field, it has to be assumed to possibly
>> have any value. In particular, our kernels don't set up this field at all
>> if running as Dom0 and kexec isn't enabled (along with not setting up
>> the P2M frame lists, as that's simply wasted memory in that case).
>> 
>> So, with this being a guest provided value, we apparently have two
>> problems: do_memory_op()'s "rc" variable is only of type "int", thus
>> potentially truncating the result of domain_get_maximum_gpfn()
>> (considering that we now support 16Tb, an 8Tb+ guest ought to
>> be possible, and such would have a max GPFN with 32 significant
>> bits). And zero or a very large number being returned by the latter
>> will generally be mistaken as an error code by the caller of the
>> hypercall.
>> 
>> So, along with promoting "rc" to long, I'm considering enforcing a
>> sane lower bound on domain_get_maximum_gpfn()'s return value,
>> using d->tot_pages (under the assumption that each page would
>> have a representation in the physical map, and hence the
>> physical map can't reasonably be smaller than this). That would
>> overcome the subtraction of 1 done there for PV guests to
>> convert 0 to -1 (i.e. -EPERM). Similarly, a sane upper bound
>> ought to be enforced (to avoid collisions with the -E range).
>> 
>> Other thoughts? Is such a behavioral change acceptable for an
>> existing interface?
> 
> The new behaviour seems sensible but I'm a little worried about changing
> the behaviour of an existing call.  I'd be inclined to add a new
> hypercall that DTRT and leave the old one, deprecated.

So I think this should be two steps then - fix the current call (so
that it doesn't return -1 (== -EPERM) anymore when the field is
zero, and so it doesn't truncate big values anymore) and, should
it ever turn out necessary, add a new one returning a sanitized
value.

I'll send a patch for the first part in a minute, but I'll leave the
second step open as I think the context in which the problem
was observed (xen-mceinj) is actually in need of fixing itself (i.e.
should not be using this call).

Jan


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