Re: [Xen-devel] [PATCH 1/5] xen: add ssize_t

[Jean Guyader <jean.guyader@xxxxxxxxx>]

On 9 August 2012 11:59, Tim Deegan <tim@xxxxxxx> wrote:
> At 11:19 +0100 on 09 Aug (1344511142), Jean Guyader wrote:
>> On 9 August 2012 10:51, Tim Deegan <tim@xxxxxxx> wrote:
>> > At 15:47 +0100 on 06 Aug (1344268059), Jean Guyader wrote:
>> >> On 6 August 2012 09:08, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> >> >>>> On 03.08.12 at 21:50, Jean Guyader <jean.guyader@xxxxxxxxxx> wrote:
>> >> >
>> >> > Without finally explaining why you need this type in the first place,
>> >> > I'll continue to NAK this patch. (This is made even worse by the fact
>> >> > taht the two inline functions in patch 5 that make use of the type
>> >> > appear to be unused.)
>> >> >
>> >>
>> >> Understood. I'll switch to use long instead of ssize_t in my
>> >> forthcoming patch serie.
>> >
>> > Please use an explicitly 64-bit type - AFAICS you're holding the sum of
>> > some 64-bit length fields.
>> >
>>
>> Ok but ssize_t is kind of a funny one. It should accept everything
>> that size_t can accept + negative values.
>
> Given that you start with user-supplied 64-bit iov lengths, there is no
> such type. :)
>
> And now that I look at it, your handling of sizes is pretty confused.
> v4v_ringbuf_insertv() returns a ssize_t, but calculates it internally as
> an int32_t, which is _smaller_ than the size_t it starts with (which is
> the sum of some guest-supplied uint64_ts).  And then v4v_sendv() puts
> that ssize_t into an int again before returning it as a size_t, which
> d_v4v_op() casts as a long to give to the guest.  Whee!
>
> Can you please make that lot consistent, and then audit the whole path
> from user-supplied iovec lists to actual copies and make sure that there
> are no overflows?
>
> I think that explicitly limiting your sum-of-iovec-lengths to 31 bits
> would be perfectly reasonable (1 hypercall per 2GB copy), and would
> avoid a lot of pain here.  As long as it was documented in the
> interface, of course!
>

Ok fine that works for me. I'll do that for the next version.

Jean

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