From afae6906552fa5852ab295920e25da35392c8443 Mon Sep 17 00:00:00 2001 From: Allan Boll Date: Fri, 18 Aug 2017 17:45:44 -0700 Subject: [PATCH] Preallocate memory when SecStreamInBodyInspection is on. 20x speed improvement for 10mb upload. Also simplified modsecurity_request_body_to_stream. --- apache2/apache2_io.c | 1 - apache2/modsecurity.h | 1 + apache2/msc_reqbody.c | 99 ++++++++++++++++++++++-------------------- apache2/re_operators.c | 4 +- 4 files changed, 57 insertions(+), 48 deletions(-) diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c index 9ab59a53..74a7a7cd 100644 --- a/apache2/apache2_io.c +++ b/apache2/apache2_io.c @@ -283,7 +283,6 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) { } if (msr->txcfg->stream_inbody_inspection == 1) { - msr->stream_input_length+=buflen; modsecurity_request_body_to_stream(msr, buf, buflen, error_msg); } diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index f170034c..601b5f24 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -287,6 +287,7 @@ struct modsec_rec { unsigned int resbody_contains_html; apr_size_t stream_input_length; + apr_size_t stream_input_allocated_length; char *stream_input_data; apr_size_t stream_output_length; char *stream_output_data; diff --git a/apache2/msc_reqbody.c b/apache2/msc_reqbody.c index dc840472..8c8b4add 100644 --- a/apache2/msc_reqbody.c +++ b/apache2/msc_reqbody.c @@ -428,55 +428,62 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr, } apr_status_t modsecurity_request_body_to_stream(modsec_rec *msr, const char *buffer, int buflen, char **error_msg) { - char *stream_input_body = NULL; - char *data = NULL; - int first_pkt = 0; + apr_size_t allocate; + char* allocated; - if(msr->stream_input_data == NULL) { - msr->stream_input_data = (char *)calloc(sizeof(char), msr->stream_input_length + 1); - first_pkt = 1; - } - else { - - data = (char *)malloc(msr->stream_input_length + 1 - buflen); - - if(data == NULL) - return -1; - - memset(data, 0, msr->stream_input_length + 1 - buflen); - memcpy(data, msr->stream_input_data, msr->stream_input_length - buflen); - - stream_input_body = (char *)realloc(msr->stream_input_data, msr->stream_input_length + 1); - - msr->stream_input_data = (char *)stream_input_body; - } - - if (msr->stream_input_data == NULL) { - if(data) { - free(data); - data = NULL; + if (msr->stream_input_data == NULL) { + // Is the request body length is known beforehand? (requests that are not Transfer-Encoding: chunked) + if (msr->request_content_length > 0) { + allocate = msr->request_content_length; + } + else { + // We don't know how this request is going to be, so hope for just buflen to begin with (requests that are Transfer-Encoding: chunked) + allocate = buflen; + } + + allocated = (char*) calloc(allocate, sizeof(char)); + if (allocated) { + msr->stream_input_data = allocated; + msr->stream_input_allocated_length = allocate; + } + else { + *error_msg = apr_psprintf( + msr->mp, + "Unable to allocate memory to hold request body on stream. Asked for %" APR_SIZE_T_FMT " bytes.", + allocate); + return -1; + } + } + else { + // Do we need to expand the space we have previously allocated? + if ((msr->stream_input_length + buflen) > msr->stream_input_allocated_length) { + + // If this becomes a hotspot again, consider increasing by some percent extra each time, for fewer reallocs + allocate = msr->stream_input_length + buflen; + + allocated = (char*) realloc(msr->stream_input_data, allocate); + if (allocated) { + msr->stream_input_data = allocated; + msr->stream_input_allocated_length = allocate; + } + else { + *error_msg = apr_psprintf( + msr->mp, + "Unable to reallocate memory to hold request body on stream. Asked for %" APR_SIZE_T_FMT " bytes.", + allocate); + free(msr->stream_input_data); + return -1; + } } - *error_msg = apr_psprintf(msr->mp, "Unable to allocate memory to hold request body on stream. Asked for %" APR_SIZE_T_FMT " bytes.", - msr->stream_input_length + 1); - return -1; } - memset(msr->stream_input_data, 0, msr->stream_input_length+1); - - if(first_pkt) { - memcpy(msr->stream_input_data, buffer, msr->stream_input_length); - } else { - memcpy(msr->stream_input_data, data, msr->stream_input_length - buflen); - memcpy(msr->stream_input_data+(msr->stream_input_length - buflen), buffer, buflen); - } - - if(data) { - free(data); - data = NULL; - } + // Append buffer to msr->stream_input_data + memcpy(msr->stream_input_data + msr->stream_input_length, buffer, buflen); + msr->stream_input_length += buflen; return 1; } + /** * Replace a bunch of chunks holding a request body with a single large chunk. */ @@ -884,15 +891,15 @@ apr_status_t modsecurity_request_body_clear(modsec_rec *msr, char **error_msg) { if (msr->msc_reqbody_filename != NULL) { if (keep_body) { - /* Move request body (which is a file) to the storage area. */ - const char *put_filename = NULL; - const char *put_basename = NULL; - if (strcmp(msr->txcfg->upload_dir, msr->txcfg->tmp_dir) == 0) { msr_log(msr, 4, "Not moving file to identical location."); goto nullify; } + /* Move request body (which is a file) to the storage area. */ + const char *put_filename = NULL; + const char *put_basename = NULL; + /* Construct the new filename. */ put_basename = file_basename(msr->msc_reqbody_mp, msr->msc_reqbody_filename); if (put_basename == NULL) { diff --git a/apache2/re_operators.c b/apache2/re_operators.c index f444d3c5..dbade1f4 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -634,6 +634,7 @@ nextround: free(msr->stream_input_data); msr->stream_input_data = NULL; msr->stream_input_length = 0; + msr->stream_input_allocated_length = 0; msr->stream_input_data = (char *)malloc(size+1); @@ -642,7 +643,8 @@ nextround: } msr->stream_input_length = size; - memset(msr->stream_input_data, 0x0, size+1); + msr->stream_input_allocated_length = size + 1; + memset(msr->stream_input_data, 0x0, size + 1); msr->if_stream_changed = 1;