[gnutls-devel] GnuTLS 3.2.15 SIGSEGV in _gnutls_buffer_append_data

[Jaak Ristioja]

Hello!

Yesterday I found myself debugging a SIGSEGV in an application which
calls gnutls_record_send() on 7-byte and 16384-byte buffers during a
short amount of time (the total size of data the application must pass
to gnutls_record_send() exceeds 2,5 GiB). I think that GnuTLS is mostly
corked during these operations. I got the following backtrace in GDB:

(gdb) thread 12
[Switching to thread 12 (Thread 0x7ffff22eb700 (LWP 3461))]
#0  __memmove_ssse3_back () at
../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S:93
93      ../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S: No such file or
directory.
(gdb) bt
#0  __memmove_ssse3_back () at
../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S:93
#1  0x00007ffff6c17c55 in memmove (__len=<optimized out>,
__src=<optimized out>, __dest=<optimized out>) at
/usr/include/bits/string3.h:57
#2  _gnutls_buffer_append_data (dest=dest at entry=0x7fff9c0012d0,
data=0x7ffd158e9008, data_size=data_size at entry=16384) at gnutls_str.c:147
#3  0x00007ffff6bf5727 in gnutls_record_send (session=0x7fff9c0009c0,
data=<optimized out>, data_size=16384) at gnutls_record.c:1456
...
(gdb) f 2
#2  _gnutls_buffer_append_data (dest=dest at entry=0x7fff9c0012d0,
data=0x7ffd158e9008, data_size=data_size at entry=16384) at gnutls_str.c:147
147                             memmove(dest->allocd, dest->data,
dest->length);
(gdb) print *dest
$1 = {allocd = 0x555555974380 "\020@\227UUU", data = 0xd55340522390
<error: Cannot access memory at address 0xd55340522390>, max_length =
851513336, length = 851496223}
(gdb) print data
$2 = (const void *) 0x7ffd158e9008
(gdb) print unused
$3 = 140728541569040
(gdb) print new_len
$4 = 851513336
(gdb) print tot_len
$5 = 851512607

Can anybody help me understand how a gnutls_buffer_st,
_gnutls_buffer_append_data and related functions should work, or why
this crash happens? This code in lib/gnutls_str.[ch] could use a few
more comments.

Thanks!


Best regards,

Jaak Ristioja
Cybernetica AS
http://cyber.ee/

PS: A few small ideas for optimizing the current
_gnutls_buffer_append_data function:

1) The line
     size_t unused = MEMSUB(dest->data, dest->allocd);
can be deduplicated by moving it out of the if-block.

2) The variables declared in this function may be marked const.

3) In the else branch, why check for dest->data (in the if-condition for
memmove) if it has previously been initialized to (dest->allocd + unused)?

PPS: Why is MEMSUB in lib/gnutls_int.h defined as
  #define MEMSUB(x,y) ((ssize_t)((ptrdiff_t)x-(ptrdiff_t)y))
instead of
  #define MEMSUB(x,y) ((ssize_t)((char const *)x-(char const *)y))
The size of the result of subtracting two pointers should be of type
ptrdiff_t (a signed integer type) anyway. Are we sure no errors are
introduced when converting the pointers x and y to ptrdiff_t before the
subtraction? In addition, using parenthesis around x and y in the macro
body might help macro safety.