Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Simon Richter <Simon.Richter@hogyros.de> writes:
>
>> Hi,
>>
>>> +		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
>>> +		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
>>
>> That feels like it could overflow.
>
> Isn't it checking if it would overflow (and dying if so)?
>
> Ah, wait.  The addition "(iov[i].iov_len + sum)" can indeed wrap
> around, and comparing it with the maximum value of ssize_t wouldn't
> catch that.  Is that what you mean?
>
> Would something like this:
>
>     if (maximum_signed_value_of_type(ssize_t) < iov[i].iov_len ||
> 	iov[i].iov_len + sum < iov[i].iov_len ||
> 	maximum_signed_value_of_type(ssize_t) < iov[i].iov_len + sum)
>
> work better to catch the three cases independently?
>
>  (1) The value is already too large on its own.
>  (2) Adding them together would cause an unsigned wrap-around.
>  (3) The sum does not wrap around, but it exceeds the maximum
>      representable value of ssize_t anyway.

Actually, looking at it again, I think the original code is safe
after all, because:

 * "sum", even though it is a size_t, is checked inside the loop to
   ensure it stays below the maximum value of ssize_t each time it
   gets a new value.
 * iov[i].iov_len is checked to ensure it does not exceed the
   maximum value of ssize_t by the first part of the condition.

If both values are less than or equal to the maximum value of
ssize_t, their sum is at most twice that limit.  For an N-bit
size_t, this sum is at most (2^N - 2), which can be computed safely
without any unsigned wrap-around.

So...?