Bug 45444 - overlapping memcpy in ssl_io_input_read
Summary: overlapping memcpy in ssl_io_input_read
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_ssl (show other bugs)
Version: 2.2.9
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-21 02:51 UTC by Frederic Heem
Modified: 2014-02-17 13:59 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Heem 2008-07-21 02:51:01 UTC
Hi,
Valgrind has found a problem related to an overlapping memcpy in mod_ssl (Apache/2.2.9 (Unix)), here is the output:

==18546== Thread 5:
==18546== Source and destination overlap in memcpy(0x425E0E8, 0x425E10E, 141)
==18546==    at 0x4007A42: memcpy (mc_replace_strmem.c:402)
==18546==    by 0x446C464: ssl_io_input_read (in /usr/local/apache2/modules/mod_ssl.so)
==18546==    by 0x446C781: ssl_io_filter_input (in /usr/local/apache2/modules/mod_ssl.so)
==18546==    by 0x8068DB5: ap_rgetline_core (in /usr/local/apache2/bin/httpd)
==18546==    by 0x80690CE: ap_get_mime_headers_core (in /usr/local/apache2/bin/httpd)
==18546==    by 0x80696FC: ap_read_request (in /usr/local/apache2/bin/httpd)
==18546==    by 0x80799DA: ap_process_http_connection (in /usr/local/apache2/bin/httpd)
==18546==    by 0x8076CEC: ap_run_process_connection (in /usr/local/apache2/bin/httpd)
==18546==    by 0x807FFD3: worker_thread (in /usr/local/apache2/bin/httpd)
==18546==    by 0x4057603: dummy_worker (in /usr/local/apache2/lib/libapr-1.so.0.3.0)
==18546==    by 0x8E145A: start_thread (in /lib/libpthread-2.5.so)
==18546==    by 0x71323D: clone (in /lib/libc-2.5.so) 


This happens when a client sends a https request, regardless on the type of request.
Let me know if you need more information.
Comment 1 Joe Orton 2008-07-23 07:03:51 UTC
Is it possible that you can get it to dump core, and get a backtrace with line numbers from the core dump?
Comment 2 Frederic Heem 2008-07-23 08:13:46 UTC
Here you are:
==23377== Thread 5:
==23377== Source and destination overlap in memcpy(0x427DE58, 0x427DE7E, 142)
==23377==    at 0x4007A42: memcpy (mc_replace_strmem.c:402)
==23377==    by 0x44795FE: ssl_io_input_read (ssl_engine_io.c:353)
==23377==    by 0x44798DC: ssl_io_filter_input (ssl_engine_io.c:727)
==23377==    by 0x806ACA2: ap_rgetline_core (protocol.c:231)
==23377==    by 0x806B08B: ap_get_mime_headers_core (protocol.c:690)
==23377==    by 0x806B8C2: ap_read_request (protocol.c:918)
==23377==    by 0x8081864: ap_process_http_connection (http_core.c:183)
==23377==    by 0x807DA48: ap_run_process_connection (connection.c:43)
==23377==    by 0x80898C2: worker_thread (worker.c:544)
==23377==    by 0x4060D05: dummy_worker (thread.c:142)
==23377==    by 0x8E145A: start_thread (in /lib/libpthread-2.5.so)
==23377==    by 0x71323D: clone (in /lib/libc-2.5.so)

Comment 3 Joe Orton 2008-08-06 05:57:40 UTC
It seems like the code is indeed bogus, for the input getline case:

-> getline the first time
 -> inctx->cbuf is empty
 -> read block from SSL layer into inctx->buffer
 -> pass back first line, stash remainder of inctx->buffer at inctx->cbuf

-> getline the second time
 -> inctx->cbuf non-empty! copy entire inctx->cbuf into inctx->buffer

which results in the overlapping memcpy, and is pretty inefficient to boot; for N GETLINE calls to read the entire HTTP request, the buffer gets copied over itself N times.

Changing it to a memmove seems safe enough.
Comment 4 Joe Orton 2008-08-06 07:58:25 UTC
Fixed on trunk - thanks for the report and getting the backtrace.

http://svn.apache.org/viewvc?rev=683280&view=rev

Code made more efficient in following commit:

http://svn.apache.org/viewvc?rev=683283&view=rev