[gnutls-devel] GnuTLS | gnutls 3.6 pseudo-randomly breaks VLC HTTP/2 (#713)
Development of GNU's TLS library
gnutls-devel at lists.gnutls.org
Fri Feb 22 15:39:48 CET 2019
```
WARNING: ThreadSanitizer: data race (pid=636)
Read of size 4 at 0x7b8c00012014 by thread T11:
#0 gnutls_record_send2 /home/chouquette/dev/vlc/contrib/native/gnutls/lib/record.c:1983 (libgnutls_plugin.so+0x20c52)
#1 gnutls_record_send /home/chouquette/dev/vlc/contrib/native/gnutls/lib/record.c:1944 (libgnutls_plugin.so+0x21115)
#2 gnutls_Send ../../modules/misc/gnutls.c:175 (libgnutls_plugin.so+0x197f3)
#3 vlc_https_send ../../modules/access/http/h2output.c:204 (libhttps_plugin.so+0xfddd)
#4 vlc_h2_frame_send ../../modules/access/http/h2output.c:250 (libhttps_plugin.so+0xffdc)
#5 vlc_h2_output_thread ../../modules/access/http/h2output.c:265 (libhttps_plugin.so+0x1006d)
#6 vlc_h2_client_output_thread ../../modules/access/http/h2output.c:299 (libhttps_plugin.so+0x101fb)
#7 <null> <null> (libtsan.so.0+0x29b4d)
Previous write of size 4 at 0x7b8c00012014 by thread T12:
#0 check_session_status /home/chouquette/dev/vlc/contrib/native/gnutls/lib/record.c:1690 (libgnutls_plugin.so+0x20204)
#1 _gnutls_recv_int /home/chouquette/dev/vlc/contrib/native/gnutls/lib/record.c:1753 (libgnutls_plugin.so+0x2055c)
#2 gnutls_record_recv /home/chouquette/dev/vlc/contrib/native/gnutls/lib/record.c:2261 (libgnutls_plugin.so+0x215ca)
#3 gnutls_Recv ../../modules/misc/gnutls.c:146 (libgnutls_plugin.so+0x196d0)
#4 vlc_https_recv ../../modules/access/http/h2conn.c:563 (libhttps_plugin.so+0x122c4)
#5 vlc_h2_frame_recv ../../modules/access/http/h2conn.c:606 (libhttps_plugin.so+0x12453)
#6 vlc_h2_recv_thread ../../modules/access/http/h2conn.c:656 (libhttps_plugin.so+0x1270d)
#7 <null> <null> (libtsan.so.0+0x29b4d)
Location is heap block of size 6504 at 0x7b8c00011800 allocated by thread T5:
#0 calloc <null> (libtsan.so.0+0x2b333)
#1 gnutls_init /home/chouquette/dev/vlc/contrib/native/gnutls/lib/state.c:450 (libgnutls_plugin.so+0x63a18)
#2 gnutls_SessionOpen ../../modules/misc/gnutls.c:241 (libgnutls_plugin.so+0x199fe)
#3 gnutls_ClientSessionOpen ../../modules/misc/gnutls.c:386 (libgnutls_plugin.so+0x1a22c)
#4 vlc_tls_ClientSessionCreate ../../src/network/tls.c:151 (libvlccore.so.9+0x165aac)
#5 vlc_tls_SocketOpenTLS ../../src/network/tls.c:242 (libvlccore.so.9+0x16607d)
#6 vlc_https_connect ../../modules/access/http/connmgr.c:65 (libhttps_plugin.so+0x15662)
#7 vlc_https_request ../../modules/access/http/connmgr.c:175 (libhttps_plugin.so+0x15b3d)
#8 vlc_http_mgr_request ../../modules/access/http/connmgr.c:258 (libhttps_plugin.so+0x15f2a)
#9 vlc_http_res_open ../../modules/access/http/resource.c:93 (libhttps_plugin.so+0x90c0)
#10 vlc_http_res_get_status ../../modules/access/http/resource.c:138 (libhttps_plugin.so+0x92aa)
#11 Open ../../modules/access/http/access.c:206 (libhttps_plugin.so+0x47df)
#12 generic_start ../../src/modules/modules.c:320 (libvlccore.so.9+0x8dcde)
#13 module_load ../../src/modules/modules.c:167 (libvlccore.so.9+0x8d4a0)
#14 vlc_module_load ../../src/modules/modules.c:244 (libvlccore.so.9+0x8d84f)
#15 module_need ../../src/modules/modules.c:335 (libvlccore.so.9+0x8de38)
#16 access_New ../../src/input/access.c:149 (libvlccore.so.9+0xc098e)
#17 stream_AccessNew ../../src/input/access.c:294 (libvlccore.so.9+0xc12ed)
#18 InputDemuxNew ../../src/input/input.c:2518 (libvlccore.so.9+0xfe90c)
#19 InputSourceNew ../../src/input/input.c:2659 (libvlccore.so.9+0xff26b)
#20 Init ../../src/input/input.c:1362 (libvlccore.so.9+0xf9b74)
#21 Run ../../src/input/input.c:532 (libvlccore.so.9+0xf62fe)
#22 <null> <null> (libtsan.so.0+0x29b4d)
Thread T11 (tid=649, running) created by thread T5 at:
#0 pthread_create <null> (libtsan.so.0+0x2be2b)
#1 vlc_clone_attr ../../src/posix/thread.c:421 (libvlccore.so.9+0x1a2936)
#2 vlc_clone ../../src/posix/thread.c:433 (libvlccore.so.9+0x1a29c3)
#3 vlc_h2_output_create ../../modules/access/http/h2output.c:323 (libhttps_plugin.so+0x10388)
#4 vlc_h2_conn_create ../../modules/access/http/h2conn.c:730 (libhttps_plugin.so+0x12ad3)
#5 vlc_https_request ../../modules/access/http/connmgr.c:190 (libhttps_plugin.so+0x15b88)
#6 vlc_http_mgr_request ../../modules/access/http/connmgr.c:258 (libhttps_plugin.so+0x15f2a)
#7 vlc_http_res_open ../../modules/access/http/resource.c:93 (libhttps_plugin.so+0x90c0)
#8 vlc_http_res_get_status ../../modules/access/http/resource.c:138 (libhttps_plugin.so+0x92aa)
#9 Open ../../modules/access/http/access.c:206 (libhttps_plugin.so+0x47df)
#10 generic_start ../../src/modules/modules.c:320 (libvlccore.so.9+0x8dcde)
#11 module_load ../../src/modules/modules.c:167 (libvlccore.so.9+0x8d4a0)
#12 vlc_module_load ../../src/modules/modules.c:244 (libvlccore.so.9+0x8d84f)
#13 module_need ../../src/modules/modules.c:335 (libvlccore.so.9+0x8de38)
#14 access_New ../../src/input/access.c:149 (libvlccore.so.9+0xc098e)
#15 stream_AccessNew ../../src/input/access.c:294 (libvlccore.so.9+0xc12ed)
#16 InputDemuxNew ../../src/input/input.c:2518 (libvlccore.so.9+0xfe90c)
#17 InputSourceNew ../../src/input/input.c:2659 (libvlccore.so.9+0xff26b)
#18 Init ../../src/input/input.c:1362 (libvlccore.so.9+0xf9b74)
#19 Run ../../src/input/input.c:532 (libvlccore.so.9+0xf62fe)
#20 <null> <null> (libtsan.so.0+0x29b4d)
Thread T12 (tid=650, running) created by thread T5 at:
#0 pthread_create <null> (libtsan.so.0+0x2be2b)
#1 vlc_clone_attr ../../src/posix/thread.c:421 (libvlccore.so.9+0x1a2936)
#2 vlc_clone ../../src/posix/thread.c:433 (libvlccore.so.9+0x1a29c3)
#3 vlc_h2_conn_create ../../modules/access/http/h2conn.c:742 (libhttps_plugin.so+0x12bce)
#4 vlc_https_request ../../modules/access/http/connmgr.c:190 (libhttps_plugin.so+0x15b88)
#5 vlc_http_mgr_request ../../modules/access/http/connmgr.c:258 (libhttps_plugin.so+0x15f2a)
#6 vlc_http_res_open ../../modules/access/http/resource.c:93 (libhttps_plugin.so+0x90c0)
#7 vlc_http_res_get_status ../../modules/access/http/resource.c:138 (libhttps_plugin.so+0x92aa)
#8 Open ../../modules/access/http/access.c:206 (libhttps_plugin.so+0x47df)
#9 generic_start ../../src/modules/modules.c:320 (libvlccore.so.9+0x8dcde)
#10 module_load ../../src/modules/modules.c:167 (libvlccore.so.9+0x8d4a0)
#11 vlc_module_load ../../src/modules/modules.c:244 (libvlccore.so.9+0x8d84f)
#12 module_need ../../src/modules/modules.c:335 (libvlccore.so.9+0x8de38)
#13 access_New ../../src/input/access.c:149 (libvlccore.so.9+0xc098e)
#14 stream_AccessNew ../../src/input/access.c:294 (libvlccore.so.9+0xc12ed)
#15 InputDemuxNew ../../src/input/input.c:2518 (libvlccore.so.9+0xfe90c)
#16 InputSourceNew ../../src/input/input.c:2659 (libvlccore.so.9+0xff26b)
#17 Init ../../src/input/input.c:1362 (libvlccore.so.9+0xf9b74)
#18 Run ../../src/input/input.c:532 (libvlccore.so.9+0xf62fe)
#19 <null> <null> (libtsan.so.0+0x29b4d)
Thread T5 (tid=642, running) created by thread T2 at:
#0 pthread_create <null> (libtsan.so.0+0x2be2b)
#1 vlc_clone_attr ../../src/posix/thread.c:421 (libvlccore.so.9+0x1a2936)
#2 vlc_clone ../../src/posix/thread.c:433 (libvlccore.so.9+0x1a29c3)
#3 input_Start ../../src/input/input.c:192 (libvlccore.so.9+0xf4eff)
#4 PlayItem ../../src/playlist_legacy/thread.c:242 (libvlccore.so.9+0x9bb36)
#5 Next ../../src/playlist_legacy/thread.c:482 (libvlccore.so.9+0x9cc51)
#6 Thread ../../src/playlist_legacy/thread.c:507 (libvlccore.so.9+0x9cd68)
#7 <null> <null> (libtsan.so.0+0x29b4d)
```
Since the lines may not be accurate when taken from our contrib build (at least they weren't, compared to my local gnutls repo), here are the read/writes incriminated in the race/bug:
read:
```
if (unlikely(!session->internals.initial_negotiation_completed)) {
/* this is to protect buggy applications from sending unencrypted
* data. We allow sending however, if we are in false start handshake
* state. */
if (session->internals.recv_state != RECV_STATE_FALSE_START && // <---------------
session->internals.recv_state != RECV_STATE_EARLY_START &&
!(session->internals.hsk_flags & HSK_EARLY_DATA_IN_FLIGHT))
return gnutls_assert_val(GNUTLS_E_UNAVAILABLE_DURING_HANDSHAKE);
}
```
Which is the code added by https://gitlab.com/gnutls/gnutls/commit/6a62ddfc416a4ec2118704f93c97fdd448d66566
Write:
record.c
```
case RECV_STATE_FALSE_START:
/* if false start is not complete we always expect for handshake packets
* prior to anything else. */
if (session->security_parameters.entity != GNUTLS_CLIENT ||
!(session->internals.flags & GNUTLS_ENABLE_FALSE_START))
return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
/* Attempt to complete handshake */
session->internals.recv_state = RECV_STATE_FALSE_START_HANDLING; // <----------------
ret = gnutls_handshake(session);
if (ret < 0) {
/* a temp or fatal error, make sure we reset the state
* so we can resume on temp errors */
session->internals.recv_state = RECV_STATE_FALSE_START;
return gnutls_assert_val(ret);
}
```
The 2 threads racing are our http2 stack receiver thread (`T11`), and sender thread (`T12`)
If VLC's usage of the API is correct, then accesses to the session's internals must be made atomic, or we need to add some synchronization on our end if it's our job to ensure the handshake ran to completion after a fast start during recv/send
But in any case, as things stand, the current code/use of the API is not thread safe.
I'll happily provide a patch, but should the fix be in GnuTLS, I could use some guidance :)
--
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/issues/713#note_144089452
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20190222/09b7c29f/attachment-0001.html>
More information about the Gnutls-devel
mailing list