From b1f65f1cd18a40574665c04cfff122fe819dc53b Mon Sep 17 00:00:00 2001 From: Graham Leggett Date: Sun, 17 Nov 2013 20:10:28 +0000 Subject: [PATCH] slotmem_shm: Error detection trunk patch: https://svn.apache.org/viewvc?view=revision&revision=1540161 https://svn.apache.org/viewvc?view=revision&revision=1540163 https://svn.apache.org/viewvc?view=revision&revision=1540178 https://svn.apache.org/viewvc?view=revision&revision=1540179 https://svn.apache.org/viewvc?view=revision&revision=1540220 http://svn.apache.org/viewvc?view=revision&revision=r1542413 2.4.x patch: http://people.apache.org/~jim/patches/slotmem-error2.patch Submitted by: jim Reviewed by: jorton, minfrin git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1542803 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 2 + STATUS | 9 ---- include/ap_slotmem.h | 1 + modules/slotmem/mod_slotmem_shm.c | 68 ++++++++++++++++++++++++------- 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/CHANGES b/CHANGES index 29286d236b..3dab2228ed 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,8 @@ Changes with Apache 2.4.7 + *) slotmem_shm: Error detection. [Jim Jagielski] + *) event: Use skiplist data structure (requires APR 1.5). [Jim Jagielski] diff --git a/STATUS b/STATUS index 19fe4d62e1..ae2a2ba0df 100644 --- a/STATUS +++ b/STATUS @@ -97,15 +97,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * slotmem_shm: Error detection - trunk patch: https://svn.apache.org/viewvc?view=revision&revision=1540161 - https://svn.apache.org/viewvc?view=revision&revision=1540163 - https://svn.apache.org/viewvc?view=revision&revision=1540178 - https://svn.apache.org/viewvc?view=revision&revision=1540179 - https://svn.apache.org/viewvc?view=revision&revision=1540220 - http://svn.apache.org/viewvc?view=revision&revision=r1542413 - 2.4.x patch: http://people.apache.org/~jim/patches/slotmem-error2.patch - +1: jim, jorton, minfrin PATCHES PROPOSED TO BACKPORT FROM TRUNK: diff --git a/include/ap_slotmem.h b/include/ap_slotmem.h index b9c810366d..e1615e91e8 100644 --- a/include/ap_slotmem.h +++ b/include/ap_slotmem.h @@ -39,6 +39,7 @@ #include "apr_shm.h" #include "apr_global_mutex.h" #include "apr_file_io.h" +#include "apr_md5.h" #if APR_HAVE_UNISTD_H #include /* for getpid() */ diff --git a/modules/slotmem/mod_slotmem_shm.c b/modules/slotmem/mod_slotmem_shm.c index ae6daadb1b..1f7e557cd9 100644 --- a/modules/slotmem/mod_slotmem_shm.c +++ b/modules/slotmem/mod_slotmem_shm.c @@ -178,6 +178,7 @@ static void store_slotmem(ap_slotmem_instance_t *slotmem) apr_status_t rv; apr_size_t nbytes; const char *storename; + unsigned char digest[APR_MD5_DIGESTSIZE]; storename = slotmem_filename(slotmem->gpool, slotmem->name, 1); @@ -200,20 +201,27 @@ static void store_slotmem(ap_slotmem_instance_t *slotmem) } nbytes = (slotmem->desc.size * slotmem->desc.num) + (slotmem->desc.num * sizeof(char)) + AP_UNSIGNEDINT_OFFSET; - /* XXX: Error handling */ - apr_file_write_full(fp, slotmem->persist, nbytes, NULL); + apr_md5(digest, slotmem->persist, nbytes); + rv = apr_file_write_full(fp, slotmem->persist, nbytes, NULL); + if (rv == APR_SUCCESS) { + rv = apr_file_write_full(fp, digest, APR_MD5_DIGESTSIZE, NULL); + } apr_file_close(fp); + if (rv != APR_SUCCESS) { + apr_file_remove(storename, slotmem->gpool); + } } } -/* should be apr_status_t really */ -static void restore_slotmem(void *ptr, const char *name, apr_size_t size, - apr_pool_t *pool) +static apr_status_t restore_slotmem(void *ptr, const char *name, apr_size_t size, + apr_pool_t *pool) { const char *storename; apr_file_t *fp; apr_size_t nbytes = size; - apr_status_t rv; + apr_status_t rv = APR_SUCCESS; + unsigned char digest[APR_MD5_DIGESTSIZE]; + unsigned char digest2[APR_MD5_DIGESTSIZE]; storename = slotmem_filename(pool, name, 1); @@ -224,20 +232,42 @@ static void restore_slotmem(void *ptr, const char *name, apr_size_t size, rv = apr_file_open(&fp, storename, APR_READ | APR_WRITE, APR_OS_DEFAULT, pool); if (rv == APR_SUCCESS) { - apr_finfo_t fi; - if (apr_file_info_get(&fi, APR_FINFO_SIZE, fp) == APR_SUCCESS) { - if (fi.size == nbytes) { - apr_file_read(fp, ptr, &nbytes); + rv = apr_file_read(fp, ptr, &nbytes); + if ((rv == APR_SUCCESS || rv == APR_EOF) && nbytes == size) { + rv = APR_SUCCESS; /* for successful return @ EOF */ + /* + * if at EOF, don't bother checking md5 + * - backwards compatibility + * */ + if (apr_file_eof(fp) != APR_EOF) { + apr_size_t ds = APR_MD5_DIGESTSIZE; + rv = apr_file_read(fp, digest, &ds); + if ((rv == APR_SUCCESS || rv == APR_EOF) && + ds == APR_MD5_DIGESTSIZE) { + rv = APR_SUCCESS; + apr_md5(digest2, ptr, nbytes); + if (memcmp(digest, digest2, APR_MD5_DIGESTSIZE)) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, + APLOGNO(02551) "bad md5 match"); + rv = APR_EGENERAL; + } + } } else { - apr_file_close(fp); - apr_file_remove(storename, pool); - return; + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, + APLOGNO(02552) "at EOF... bypassing md5 match check (old persist file?)"); } } + else if (nbytes != size) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, + APLOGNO(02553) "Expected %" APR_SIZE_T_FMT ": Read %" APR_SIZE_T_FMT, + size, nbytes); + rv = APR_EGENERAL; + } apr_file_close(fp); } } + return rv; } static apr_status_t cleanup_slotmem(void *param) @@ -395,8 +425,16 @@ static apr_status_t slotmem_create(ap_slotmem_instance_t **new, * sense if the restore fails? Any? */ if (type & AP_SLOTMEM_TYPE_PERSIST) { - restore_slotmem(ptr, fname, dsize, pool); - restored = 1; + rv = restore_slotmem(ptr, fname, dsize, pool); + if (rv == APR_SUCCESS) { + restored = 1; + } + else { + /* just in case, re-zero */ + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, + APLOGNO(02554) "could not restore %s", fname); + memset(ptr, 0, dsize); + } } } -- 2.50.1