[Home]

Summary:ASTERISK-24566: Uninit buf in WS write
Reporter:Badalian Vyacheslav (slavon)Labels:
Date Opened:2014-11-28 17:37:01.000-0600Date Closed:2014-12-30 04:52:38.000-0600
Priority:CriticalRegression?
Status:Closed/CompleteComponents:Resources/res_http_websocket
Versions:11.14.1 11.15.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) unbuff.txt
Description:Then WS drop connection, asterisk try write uninit buf.., in WS close and WS write

WS Close valgrind:
{code}
==50066== Thread 35:
==50066== Conditional jump or move depends on uninitialised value(s)
==50066==    at 0x37C9E722CB: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:872)
==50066==    by 0x37C9E74638: _IO_default_xsputn (genops.c:485)
==50066==    by 0x37C9E71791: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1372)
==50066==    by 0x37C9E67A4C: fwrite (iofwrite.c:45)
==50066==    by 0x59A62E: ast_careful_fwrite (utils.c:1403)
==50066==    by 0x9F86F96: ast_websocket_close (res_http_websocket.c:211)
==50066==    by 0x9F86B3B: session_destroy_fn (res_http_websocket.c:125)
==50066==    by 0x44D123: internal_ao2_ref (astobj2.c:466)
==50066==    by 0x44D444: __ao2_ref (astobj2.c:548)
==50066==    by 0x9F872D9: ast_websocket_unref (res_http_websocket.c:288)
==50066==    by 0x11245ACD: __sip_destroy (chan_sip.c:6487)
==50066==    by 0x11246A92: sip_destroy (chan_sip.c:6688)
==50066==  Uninitialised value was created by a stack allocation
==50066==    at 0x9F86EA9: ast_websocket_close (res_http_websocket.c:193)
==50066==
==50066== Syscall param write(buf) points to uninitialised byte(s)
==50066==    at 0x37C9EDB61D: ??? (syscall-template.S:82)
==50066==    by 0x5891EC: tcptls_stream_write (tcptls.c:335)
==50066==    by 0x37C9E66FD8: _IO_cookie_write (iofopncook.c:72)
==50066==    by 0x37C9E73084: _IO_do_write@@GLIBC_2.2.5 (fileops.c:522)
==50066==    by 0x37C9E723DE: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:881)
==50066==    by 0x37C9E74638: _IO_default_xsputn (genops.c:485)
==50066==    by 0x37C9E71791: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1372)
==50066==    by 0x37C9E67A4C: fwrite (iofwrite.c:45)
==50066==    by 0x59A62E: ast_careful_fwrite (utils.c:1403)
==50066==    by 0x9F86F96: ast_websocket_close (res_http_websocket.c:211)
==50066==    by 0x9F86B3B: session_destroy_fn (res_http_websocket.c:125)
==50066==    by 0x44D123: internal_ao2_ref (astobj2.c:466)
==50066==  Address 0x6eb0e73 is 131 bytes inside a block of size 280 alloc'd
==50066==    at 0x4A06A2E: malloc (vg_replace_malloc.c:270)
==50066==    by 0x37C9E6718D: fopencookie@@GLIBC_2.2.5 (iofopncook.c:200)
==50066==    by 0x5895DB: tcptls_stream_fopen (tcptls.c:506)
==50066==    by 0x589895: handle_tcptls_connection (tcptls.c:592)
==50066==    by 0x599DDB: dummy_start (utils.c:1192)
==50066==    by 0x37CA2079D0: start_thread (pthread_create.c:301)
==50066==    by 0x37C9EE89DC: clone (clone.S:115)
==50066==  Uninitialised value was created by a stack allocation
==50066==    at 0x9F86EA9: ast_websocket_close (res_http_websocket.c:193)
==50066==
{code}

WS Write valgrind
{code}
==22077== Thread 29:
==22077== Conditional jump or move depends on uninitialised value(s)
==22077==    at 0x37C9E722CB: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:872)
==22077==    by 0x37C9E74638: _IO_default_xsputn (genops.c:485)
==22077==    by 0x37C9E71791: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1372)
==22077==    by 0x37C9E67A4C: fwrite (iofwrite.c:45)
==22077==    by 0x599927: ast_careful_fwrite (utils.c:1434)
==22077==    by 0xA20B161: ast_websocket_write (res_http_websocket.c:256)
==22077==    by 0x10FB0FB5: __sip_xmit (chan_sip.c:3731)
==22077==    by 0x10FB4C30: send_response (chan_sip.c:4609)
==22077==    by 0x10FD6D0A: __transmit_response (chan_sip.c:11957)
==22077==    by 0x10FD740D: transmit_response (chan_sip.c:12047)
==22077==    by 0x11025736: handle_request_bye (chan_sip.c:26884)
==22077==    by 0x1102C46F: handle_incoming (chan_sip.c:28368)
==22077==    by 0x1102CEC9: handle_request_do (chan_sip.c:28548)
==22077==    by 0x10FAD4CD: sip_websocket_callback (chan_sip.c:2617)
==22077==    by 0xA20CAA1: websocket_callback (res_http_websocket.c:681)
==22077==  Uninitialised value was created by a stack allocation
==22077==    at 0xA20AFB9: ast_websocket_write (res_http_websocket.c:220)
==22077==
==22077== Syscall param write(buf) points to uninitialised byte(s)
==22077==    at 0x37C9EDB61D: ??? (syscall-template.S:82)
==22077==    by 0x588414: tcptls_stream_write (tcptls.c:335)
==22077==    by 0x37C9E66FD8: _IO_cookie_write (iofopncook.c:72)
==22077==    by 0x37C9E73084: new_do_write (fileops.c:522)
==22077==    by 0x37C9E73084: _IO_do_write@@GLIBC_2.2.5 (fileops.c:495)
==22077==    by 0x37C9E723DE: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:881)
==22077==    by 0x37C9E74638: _IO_default_xsputn (genops.c:485)
==22077==    by 0x37C9E71791: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1372)
==22077==    by 0x37C9E67A4C: fwrite (iofwrite.c:45)
==22077==    by 0x599927: ast_careful_fwrite (utils.c:1434)
==22077==    by 0xA20B161: ast_websocket_write (res_http_websocket.c:256)
==22077==    by 0x10FB0FB5: __sip_xmit (chan_sip.c:3731)
==22077==    by 0x10FB4C30: send_response (chan_sip.c:4609)
==22077==    by 0x10FD6D0A: __transmit_response (chan_sip.c:11957)
==22077==    by 0x10FD740D: transmit_response (chan_sip.c:12047)
==22077==    by 0x11025736: handle_request_bye (chan_sip.c:26884)
==22077==  Address 0x6187ca3 is 131 bytes inside a block of size 280 alloc'd
==22077==    at 0x4A0720A: malloc (vg_replace_malloc.c:296)
==22077==    by 0x37C9E6718D: fopencookie@@GLIBC_2.2.5 (iofopncook.c:200)
==22077==    by 0x588803: tcptls_stream_fopen (tcptls.c:506)
==22077==    by 0x588ABD: handle_tcptls_connection (tcptls.c:592)
==22077==    by 0x5990D4: dummy_start (utils.c:1223)
==22077==    by 0x37CA2079D0: start_thread (pthread_create.c:301)
==22077==    by 0x37C9EE89DC: clone (clone.S:115)
==22077==  Uninitialised value was created by a stack allocation
==22077==    at 0xA20AFB9: ast_websocket_write (res_http_websocket.c:220)

{code}

VGDB backtrace caused by  WS write attached.

Also look to bug in init {{frame}} buf in ws write
{code}
-        memset(frame, 0, sizeof(*frame));
+       memset(frame, 0, header_size);
{code}

Comments:By: Badalian Vyacheslav (slavon) 2014-11-28 17:48:37.988-0600

Its becouse you use {{ast_websocket_close(session, 0)}} in {{session_destroy_fn}} and reason 0 is have 1 byte in reason header.

By: Badalian Vyacheslav (slavon) 2014-11-28 19:58:49.320-0600

also is a bad idea... reason codes must be stared from 1000
{code}
-ast_websocket_close(session, 0);
+ast_websocket_close(session, 1000);
{code}
By: Badalian Vyacheslav (slavon) 2014-11-29 07:27:23.982-0600

Hmmm... patch does not help... i see more for code...
By: Badalian Vyacheslav (slavon) 2014-12-01 06:26:17.838-0600

I'm catch it. Go to test. After 20 000 calls i post patch if it help us
By: Badalian Vyacheslav (slavon) 2014-12-02 20:18:37.555-0600

I need to help. I try many things but valgrind say it again and again... may be not {{frame}} is broken? Maybe {{FILE *f}} in {{socket}}?
By: Rusty Newton (rnewton) 2014-12-05 08:25:33.540-0600

Your description doesn't contain a description.

Please add a description into the description field. That is, describe how the issue is reproduced and the effects on the system.

I understand valgrind is showing what is happening, but users who are not developers may be searching for a similar issue and there needs to be some English description of the issue's symptoms and reproduction to allow them to find it.
By: Matt Jordan (mjordan) 2014-12-08 10:21:35.817-0600

I don't really understand why your patch would make a difference here, or how the uninitialized buffer trip is occurring.

The initialization of {{frame}} occurring in the existing code _must_ be initialized to 0 per the C99 standard:

{quote}
C99 Standard 6.7.8.21

   If there are fewer initializers in a brace-enclosed list than there are elements or members of an aggregate, or fewer characters in a string literal used to initialize an array of known size than there are elements in the array, the remainder of the aggregate shall be initialized implicitly the same as objects that have static storage duration.
{quote}

Thus, there's no reason to change the declaration of frame from {{char frame[4]}} to a {{char *}}. I don't think you've gotten to the root of what was causing the valgrind issue.
By: Badalian Vyacheslav (slavon) 2014-12-09 21:38:28.739-0600

{quote}
I try many things but valgrind say it again and again
{quote}
I was wrong as i say upper. I don't understand why valgrind say about cause...  I need you help
By: Matt Jordan (mjordan) 2014-12-11 16:25:50.843-0600

[~slavon]: I think it would be good to know what is attempting to be written into that buffer when valgrind complains about it. You may want to try some additional debug that shows that - if what goes into the buffer is not expected, then we can better understand what the solution should be.
By: Badalian Vyacheslav (slavon) 2014-12-18 16:09:00.120-0600

asterisk 11.15.0
ast_websocket_write: line 237
{code}
-        memset(frame, 0, sizeof(*frame));
+       memset(frame, 0, header_size);
{code}
sizeof(*frame) = 1
sizeof(frame) = 8
but we need zero all {{frame}}. it's size is {{header_size}}

founded by valgrind:
{code}
==22077== Thread 29:
==22077== Conditional jump or move depends on uninitialised value(s)
==22077==    at 0x37C9E722CB: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:872)
==22077==    by 0x37C9E74638: _IO_default_xsputn (genops.c:485)
==22077==    by 0x37C9E71791: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1372)
==22077==    by 0x37C9E67A4C: fwrite (iofwrite.c:45)
==22077==    by 0x599927: ast_careful_fwrite (utils.c:1434)
==22077==    by 0xA20B161: ast_websocket_write (res_http_websocket.c:256)
==22077==    by 0x10FB0FB5: __sip_xmit (chan_sip.c:3731)
==22077==    by 0x10FB4C30: send_response (chan_sip.c:4609)
==22077==    by 0x10FD6D0A: __transmit_response (chan_sip.c:11957)
==22077==    by 0x10FD740D: transmit_response (chan_sip.c:12047)
==22077==    by 0x11025736: handle_request_bye (chan_sip.c:26884)
==22077==    by 0x1102C46F: handle_incoming (chan_sip.c:28368)
==22077==    by 0x1102CEC9: handle_request_do (chan_sip.c:28548)
==22077==    by 0x10FAD4CD: sip_websocket_callback (chan_sip.c:2617)
==22077==    by 0xA20CAA1: websocket_callback (res_http_websocket.c:681)
==22077==  Uninitialised value was created by a stack allocation
==22077==    at 0xA20AFB9: ast_websocket_write (res_http_websocket.c:220)
==22077==
==22077== Syscall param write(buf) points to uninitialised byte(s)
==22077==    at 0x37C9EDB61D: ??? (syscall-template.S:82)
==22077==    by 0x588414: tcptls_stream_write (tcptls.c:335)
==22077==    by 0x37C9E66FD8: _IO_cookie_write (iofopncook.c:72)
==22077==    by 0x37C9E73084: new_do_write (fileops.c:522)
==22077==    by 0x37C9E73084: _IO_do_write@@GLIBC_2.2.5 (fileops.c:495)
==22077==    by 0x37C9E723DE: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:881)
==22077==    by 0x37C9E74638: _IO_default_xsputn (genops.c:485)
==22077==    by 0x37C9E71791: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1372)
==22077==    by 0x37C9E67A4C: fwrite (iofwrite.c:45)
==22077==    by 0x599927: ast_careful_fwrite (utils.c:1434)
==22077==    by 0xA20B161: ast_websocket_write (res_http_websocket.c:256)
==22077==    by 0x10FB0FB5: __sip_xmit (chan_sip.c:3731)
==22077==    by 0x10FB4C30: send_response (chan_sip.c:4609)
==22077==    by 0x10FD6D0A: __transmit_response (chan_sip.c:11957)
==22077==    by 0x10FD740D: transmit_response (chan_sip.c:12047)
==22077==    by 0x11025736: handle_request_bye (chan_sip.c:26884)
==22077==  Address 0x6187ca3 is 131 bytes inside a block of size 280 alloc'd
==22077==    at 0x4A0720A: malloc (vg_replace_malloc.c:296)
==22077==    by 0x37C9E6718D: fopencookie@@GLIBC_2.2.5 (iofopncook.c:200)
==22077==    by 0x588803: tcptls_stream_fopen (tcptls.c:506)
==22077==    by 0x588ABD: handle_tcptls_connection (tcptls.c:592)
==22077==    by 0x5990D4: dummy_start (utils.c:1223)
==22077==    by 0x37CA2079D0: start_thread (pthread_create.c:301)
==22077==    by 0x37C9EE89DC: clone (clone.S:115)
==22077==  Uninitialised value was created by a stack allocation
==22077==    at 0xA20AFB9: ast_websocket_write (res_http_websocket.c:220)

{code}
By: Badalian Vyacheslav (slavon) 2014-12-18 20:02:14.556-0600

I get backtrace on VGDB...I attach it...
Strange... look as bug in GLIBC... From WS {{frame}} is set but in #f0 its empty...

Very simple to reproduce....
# get Opera 20.0.1387.91 (its work without DTLS). We use HTTPS and newer browser request DTLS and WSS...
# use transport=ws in sip.conf
# run asterisk in valgrind
# Create HTML page that try call when loading
# Do F5 after call start. You must get time then asterisk try replay to WS. In my log it asterisk try send ACK message do dead channel.
# Get Valgrind error...

As i see asterisk try write uninit buf with size = 1 to stream but i can't understand why...
By: Matt Jordan (mjordan) 2014-12-19 10:31:04.057-0600

That sounds like a different problem than this one. Fixing the incorrect usage of {{sizeof}} should address the original intent of this issue.

If, after fixing that, there are additional problems, let's fix that on a separate issue.
By: Richard Mudgett (rmudgett) 2014-12-19 14:24:00.432-0600

While your finding of the incorrect use of {{sizeof}} in {{ast_websocket_write()}} is correct, it will have no effect since the efforts of {{memset()}} are overwritten by the following code which sets all of the allocated bytes to the needed values.
{code}
frame[0] = opcode | 0x80;
frame[1] = length;

/* Use the additional available bytes to store the length */
if (length == 126) {
put_unaligned_uint16(&frame[2], htons(actual_length));
} else if (length == 127) {
put_unaligned_uint64(&frame[2], htonl(actual_length));
}
{code}
Even if the above code did not set all of the bytes and the {{memset()}} was fixed, valgrind would have the same finding because valgrind was also triggering on {{ast_websocket_close()}}.

Something that should be checked is which implementation of {{put_unaligned_uint16()}} and {{htons()}} are used by your system.  Otherwise, the valgrind findings are looking like red-herrings or the result of some off nominal code path in the GLIBC IO library.

By: Badalian Vyacheslav (slavon) 2014-12-30 04:52:02.020-0600

Close this bug. I found race conditions in websocket. I open new bugreport when test our patch.