[gnutls-devel] [PATCH 1/6] Explicitly marked some variables const in _gnutls_buffer_append_data().

[Nikos Mavrogiannopoulos]

On Fri, 2014-12-19 at 11:03 +0200, Jaak Ristioja wrote:
> On 17.12.2014 14:55, Nikos Mavrogiannopoulos wrote:
> > Thanks. I've applied everything except the 2/4. While allowed in C99, I
> > prefer to define variables in the beginning of the function. I've also
> > committed an overflow check in 32-bit systems.
> 
> In my opinion, commit 8b592bc ("Added 32-bit overflow protection in
> _gnutls_buffer_append_data()") only fixes some overflows present in
> _gnutls_buffer_append_data(). These statements may still overflow:
> 
>     size_t const tot_len = data_size + dest->length;
> 
>     size_t const new_len =
>         MAX(data_size, MIN_CHUNK) + MAX(dest->max_length,
>                                         MIN_CHUNK);

They will, but the check below will detect it and return an error.

> Although unlikely on x86_64, they are still an issue.

I don't believe it makes sense to even try to detect such long buffers.
gnutls_record_send() is not supposed to be used with gigabytes of data,
nor petabytes or more.

> I'm too lazy to lookup the x86_64 ABI, but on my 64-bit Linux machine
> sizeof(int) is 4 and sizeof(size_t) is 8. Hence we still have a problem
> with _gnutls_buffer_append_data, because its return type is signed int.
> This means that the statement
>   return tot_len;

I've switched it to return 0 (success), instead of the sent data. Its
return value was not interpreted by any of its callers.

> Therefore we had to drop support for gnutls_record_cork() and
> gnutls_record_uncork() because it is still unreliable when buffering
> more than 2^31-1 bytes of data on 64-bit platforms.

Why would you need to buffer such large amount of data? I don't believe
it would speed up much given the small packet size of TLS.

regards,
Nikos