Re: [Xen-devel] [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-08-08 at 13:12 +0100, Stefano Stabellini wrote:
> On Tue, 7 Aug 2012, Jan Beulich wrote:
> > > Considering that each field of a multicall_entry is usually passed as an
> > > hypercall parameter, they should all remain unsigned long.
> > 
> > That'll give you subtle bugs I'm afraid: do_memory_op()'s
> > encoding of a continuation start extent (into the 'cmd' value),
> > for example, depends on being able to store the full value into
> > the command field of the multicall structure. The limit checking
> > of the permitted number of extents therefore is different
> > between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
> > compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would
> > neither find it very appealing to have do_memory_op() adjusted
> > for dealing with this new special case, nor am I sure that's the
> > only place your approach would cause problems if you excluded
> > the multicall structure from the model change.
> 
> Given the way the continuation is implemented, the same problem can
> also happen on x86.
> In fact, considering that we don't use any compat code, and that
> do_memory_op has the following check:
> 
>     /* Is size too large for us to encode a continuation? */
>     if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) )
>         return start_extent;
> 
> it would work as-is for ARM too.

For 32-on-32 ARM it'll work. It'll need changing for 32-on-64 ARM
though. We might even consider making the limit the smaller number even
for 64-on-64 ARM.

We should make the limit MEMOP_MAX_EXTENTS and allow it to be per-arch,
on x86 it'll remain ULONG_MAX >> MEMOP_EXTENT_SHIFT.

Ian.


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