diff --git a/modules/slotmem/mod_slotmem_plain.c b/modules/slotmem/mod_slotmem_plain.c index e3460089641..c03478c29f0 100644 --- a/modules/slotmem/mod_slotmem_plain.c +++ b/modules/slotmem/mod_slotmem_plain.c @@ -38,24 +38,31 @@ struct ap_slotmem_instance_t { static struct ap_slotmem_instance_t *globallistmem = NULL; static apr_pool_t *gpool = NULL; -static int slotmem_size_mul(apr_size_t a, apr_size_t b, apr_size_t *res) +static apr_status_t slotmem_layout_sizes(apr_size_t item_size, + unsigned int item_num, + apr_size_t *basesize, + apr_size_t *totalsize) { - if (a != 0 && b > ((apr_size_t)-1) / a) { - return 0; - } + apr_size_t slotdata; + apr_size_t total; - *res = a * b; - return 1; -} + if (item_num && item_size > APR_SIZE_MAX / item_num) { + return APR_EINVAL; + } + slotdata = item_size * item_num; -static int slotmem_size_add(apr_size_t a, apr_size_t b, apr_size_t *res) -{ - if (a > ((apr_size_t)-1) - b) { - return 0; + if (APR_SIZE_MAX - slotdata < item_num) { + return APR_EINVAL; } + total = slotdata + item_num; - *res = a + b; - return 1; + if (basesize) { + *basesize = slotdata; + } + if (totalsize) { + *totalsize = total; + } + return APR_SUCCESS; } static apr_status_t slotmem_do(ap_slotmem_instance_t *mem, ap_slotmem_callback_fn_t *func, void *data, apr_pool_t *pool) @@ -88,16 +95,14 @@ static apr_status_t slotmem_create(ap_slotmem_instance_t **new, const char *name ap_slotmem_instance_t *res; ap_slotmem_instance_t *next = globallistmem; apr_size_t basesize; - apr_size_t inuse_size; apr_size_t alloc_size; + apr_status_t rv; const char *fname; - if (!slotmem_size_mul(item_size, (apr_size_t)item_num, &basesize) - || !slotmem_size_mul((apr_size_t)item_num, sizeof(char), - &inuse_size) - || !slotmem_size_add(basesize, inuse_size, &alloc_size)) { - return APR_EINVAL; + rv = slotmem_layout_sizes(item_size, item_num, &basesize, &alloc_size); + if (rv != APR_SUCCESS) { + return rv; } if (name) { @@ -208,7 +213,7 @@ static apr_status_t slotmem_get(ap_slotmem_instance_t *slot, unsigned int id, un if (id >= slot->num) { return APR_EINVAL; } - if (dest_len > slot->size) { + if (dest_len > slot->size || (dest_len && !dest)) { return APR_EINVAL; } @@ -221,7 +226,9 @@ static apr_status_t slotmem_get(ap_slotmem_instance_t *slot, unsigned int id, un return ret; } *inuse=1; - memcpy(dest, ptr, dest_len); /* bounds check? */ + if (dest_len) { + memcpy(dest, ptr, dest_len); + } return APR_SUCCESS; } @@ -237,7 +244,7 @@ static apr_status_t slotmem_put(ap_slotmem_instance_t *slot, unsigned int id, un if (id >= slot->num) { return APR_EINVAL; } - if (src_len > slot->size) { + if (src_len > slot->size || (src_len && !src)) { return APR_EINVAL; } @@ -250,7 +257,9 @@ static apr_status_t slotmem_put(ap_slotmem_instance_t *slot, unsigned int id, un return ret; } *inuse=1; - memcpy(ptr, src, src_len); /* bounds check? */ + if (src_len) { + memcpy(ptr, src, src_len); + } return APR_SUCCESS; } diff --git a/modules/slotmem/mod_slotmem_shm.c b/modules/slotmem/mod_slotmem_shm.c index 46aca109706..a7c81ba59ce 100644 --- a/modules/slotmem/mod_slotmem_shm.c +++ b/modules/slotmem/mod_slotmem_shm.c @@ -72,24 +72,46 @@ struct ap_slotmem_instance_t { static struct ap_slotmem_instance_t *globallistmem = NULL; static apr_pool_t *gpool = NULL; -static int slotmem_size_mul(apr_size_t a, apr_size_t b, apr_size_t *res) +static apr_status_t slotmem_layout_sizes(apr_size_t item_size, + unsigned int item_num, + apr_size_t *basesize, + apr_size_t *persistsize, + apr_size_t *totalsize) { - if (a != 0 && b > ((apr_size_t)-1) / a) { - return 0; + apr_size_t slotdata; + apr_size_t persist; + apr_size_t total; + + if (item_num && item_size > APR_SIZE_MAX / item_num) { + return APR_EINVAL; } + slotdata = item_size * item_num; - *res = a * b; - return 1; -} + if (APR_SIZE_MAX - AP_UNSIGNEDINT_OFFSET < (apr_size_t)item_num) { + return APR_EINVAL; + } + persist = AP_UNSIGNEDINT_OFFSET + item_num; -static int slotmem_size_add(apr_size_t a, apr_size_t b, apr_size_t *res) -{ - if (a > ((apr_size_t)-1) - b) { - return 0; + if (APR_SIZE_MAX - persist < slotdata) { + return APR_EINVAL; } + persist += slotdata; - *res = a + b; - return 1; + if (APR_SIZE_MAX - AP_SLOTMEM_OFFSET < persist) { + return APR_EINVAL; + } + total = AP_SLOTMEM_OFFSET + persist; + + if (basesize) { + *basesize = slotdata; + } + if (persistsize) { + *persistsize = persist; + } + if (totalsize) { + *totalsize = total; + } + return APR_SUCCESS; } #define DEFAULT_SLOTMEM_PREFIX "slotmem-shm-" @@ -195,8 +217,13 @@ static void store_slotmem(ap_slotmem_instance_t *slotmem) if (AP_SLOTMEM_IS_CLEARINUSE(slotmem)) { slotmem_clearinuse(slotmem); } - nbytes = (slotmem->desc->size * slotmem->desc->num) + - (slotmem->desc->num * sizeof(char)) + AP_UNSIGNEDINT_OFFSET; + rv = slotmem_layout_sizes(slotmem->desc->size, slotmem->desc->num, + NULL, &nbytes, NULL); + if (rv != APR_SUCCESS) { + apr_file_close(fp); + apr_file_remove(storename, slotmem->gpool); + return; + } apr_md5(digest, slotmem->persist, nbytes); rv = apr_file_write_full(fp, slotmem->persist, nbytes, NULL); if (rv == APR_SUCCESS) { @@ -368,9 +395,7 @@ static apr_status_t slotmem_create(ap_slotmem_instance_t **new, ap_slotmem_instance_t *next = globallistmem; const char *fname, *pname = NULL; apr_shm_t *shm; - apr_size_t header_size; apr_size_t basesize; - apr_size_t inuse_size; apr_size_t size; int persist = (type & AP_SLOTMEM_TYPE_PERSIST) != 0; apr_status_t rv; @@ -379,13 +404,9 @@ static apr_status_t slotmem_create(ap_slotmem_instance_t **new, if (gpool == NULL) { return APR_ENOSHMAVAIL; } - if (!slotmem_size_mul(item_size, (apr_size_t)item_num, &basesize) - || !slotmem_size_mul((apr_size_t)item_num, sizeof(char), &inuse_size) - || !slotmem_size_add(AP_SLOTMEM_OFFSET, AP_UNSIGNEDINT_OFFSET, - &header_size) - || !slotmem_size_add(header_size, inuse_size, &size) - || !slotmem_size_add(size, basesize, &size)) { - return APR_EINVAL; + rv = slotmem_layout_sizes(item_size, item_num, &basesize, NULL, &size); + if (rv != APR_SUCCESS) { + return rv; } if (slotmem_filenames(pool, name, &fname, persist ? &pname : NULL)) { /* first try to attach to existing slotmem */ @@ -512,11 +533,8 @@ static apr_status_t slotmem_attach(ap_slotmem_instance_t **new, sharedslotdesc_t *desc; const char *fname; apr_shm_t *shm; - apr_size_t header_size; apr_size_t basesize; - apr_size_t inuse_size; apr_size_t expected_size; - apr_size_t shm_size; apr_status_t rv; if (gpool == NULL) { @@ -559,18 +577,9 @@ static apr_status_t slotmem_attach(ap_slotmem_instance_t **new, /* Read the description of the slotmem */ desc = (sharedslotdesc_t *)apr_shm_baseaddr_get(shm); - if (!slotmem_size_mul(desc->size, (apr_size_t)desc->num, &basesize) - || !slotmem_size_mul((apr_size_t)desc->num, sizeof(char), &inuse_size) - || !slotmem_size_add(AP_SLOTMEM_OFFSET, AP_UNSIGNEDINT_OFFSET, - &header_size) - || !slotmem_size_add(header_size, basesize, &expected_size) - || !slotmem_size_add(expected_size, inuse_size, &expected_size)) { - apr_shm_detach(shm); - return APR_EINVAL; - } - - shm_size = apr_shm_size_get(shm); - if (expected_size > shm_size) { + rv = slotmem_layout_sizes(desc->size, desc->num, + &basesize, NULL, &expected_size); + if (rv != APR_SUCCESS || apr_shm_size_get(shm) < expected_size) { apr_shm_detach(shm); return APR_EINVAL; } @@ -639,7 +648,7 @@ static apr_status_t slotmem_get(ap_slotmem_instance_t *slot, unsigned int id, if (id >= slot->desc->num) { return APR_EINVAL; } - if (dest_len > slot->desc->size) { + if (dest_len > slot->desc->size || (dest_len && !dest)) { return APR_EINVAL; } @@ -652,7 +661,9 @@ static apr_status_t slotmem_get(ap_slotmem_instance_t *slot, unsigned int id, return ret; } *inuse = 1; - memcpy(dest, ptr, dest_len); /* bounds check? */ + if (dest_len) { + memcpy(dest, ptr, dest_len); + } return APR_SUCCESS; } @@ -669,7 +680,7 @@ static apr_status_t slotmem_put(ap_slotmem_instance_t *slot, unsigned int id, if (id >= slot->desc->num) { return APR_EINVAL; } - if (src_len > slot->desc->size) { + if (src_len > slot->desc->size || (src_len && !src)) { return APR_EINVAL; } @@ -682,7 +693,9 @@ static apr_status_t slotmem_put(ap_slotmem_instance_t *slot, unsigned int id, return ret; } *inuse=1; - memcpy(ptr, src, src_len); /* bounds check? */ + if (src_len) { + memcpy(ptr, src, src_len); + } return APR_SUCCESS; } @@ -768,6 +781,7 @@ static apr_status_t slotmem_release(ap_slotmem_instance_t *slot, unsigned int id) { char *inuse; + int inuse_val; if (!slot) { return APR_ENOSHMAVAIL; @@ -775,11 +789,13 @@ static apr_status_t slotmem_release(ap_slotmem_instance_t *slot, inuse = slot->inuse; - if (id >= slot->desc->num || !inuse[id] ) { + inuse_val = (id < slot->desc->num) ? (int)inuse[id] : -1; + + if (id >= slot->desc->num || !inuse_val) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(02294) "slotmem(%s) release failed. Num %u/inuse[%u] %d", slot->name, slotmem_num_slots(slot), - id, (int)inuse[id]); + id, inuse_val); if (id >= slot->desc->num) { return APR_EINVAL; } else {