jorton 2004/09/22 02:14:42 Modified: . Tag: APU_0_9_BRANCH CHANGES include Tag: APU_0_9_BRANCH apr_rmm.h test Tag: APU_0_9_BRANCH testrmm.c misc Tag: APU_0_9_BRANCH apr_rmm.c Log: Backport from HEAD: * include/apr_rmm.h: Document alignment requirement for apr_rmm_init, and hence the alignement guarantee which apr_rmm_addr_get can offer. * misc/apr_rmm.c: Ensure allocated blocks use default address alignment by using aligned structure sizes throughout. * misc/apr_rmm.c (find_block_of_size): Remove redundant conditional. (apr_rmm_init, apr_rmm_destroy, apr_rmm_free): Catch some _UNLOCK return values. * misc/apr_rmm.c (apr_rmm_overhead_get): Account for worst case alignment overhead too. * test/testrmm.c (test_rmm): Check for address alignment. PR: 29873 Revision Changes Path No revision No revision 1.117.2.13 +3 -0 apr-util/CHANGES Index: CHANGES =================================================================== RCS file: /home/cvs/apr-util/CHANGES,v retrieving revision 1.117.2.12 retrieving revision 1.117.2.13 diff -d -w -u -r1.117.2.12 -r1.117.2.13 --- CHANGES 15 Sep 2004 11:34:24 -0000 1.117.2.12 +++ CHANGES 22 Sep 2004 09:14:41 -0000 1.117.2.13 @@ -1,5 +1,8 @@ Changes with APR-util 0.9.5 + *) Guarantee and require default address alignment for block offsets + within segments in the apr_rmm interface. PR 29873. [Joe Orton] + *) SECURITY: CAN-2004-0786 (cve.mitre.org) Fix input validation in apr_uri_parse() to avoid passing negative length to memcpy for malformed IPv6 literal addresses. No revision No revision 1.9.2.2 +3 -0 apr-util/include/apr_rmm.h Index: apr_rmm.h =================================================================== RCS file: /home/cvs/apr-util/include/apr_rmm.h,v retrieving revision 1.9.2.1 retrieving revision 1.9.2.2 diff -d -w -u -r1.9.2.1 -r1.9.2.2 --- apr_rmm.h 13 Feb 2004 09:52:42 -0000 1.9.2.1 +++ apr_rmm.h 22 Sep 2004 09:14:42 -0000 1.9.2.2 @@ -48,6 +48,8 @@ * @param membuf The block of relocateable memory to be managed * @param memsize The size of relocateable memory block to be managed * @param cont The pool to use for local storage and management + * @remark Both @param membuf and @param memsize must be aligned + * (for instance using APR_ALIGN_DEFAULT). */ APU_DECLARE(apr_status_t) apr_rmm_init(apr_rmm_t **rmm, apr_anylock_t *lock, void* membuf, apr_size_t memsize, @@ -108,6 +110,7 @@ * Retrieve the physical address of a relocatable allocation of memory * @param rmm The relocatable memory block * @param entity The memory allocation to free + * @return address The address, aligned with APR_ALIGN_DEFAULT. */ APU_DECLARE(void *) apr_rmm_addr_get(apr_rmm_t *rmm, apr_rmm_off_t entity); No revision No revision 1.3.2.4 +19 -1 apr-util/test/testrmm.c Index: testrmm.c =================================================================== RCS file: /home/cvs/apr-util/test/testrmm.c,v retrieving revision 1.3.2.3 retrieving revision 1.3.2.4 diff -d -w -u -r1.3.2.3 -r1.3.2.4 --- testrmm.c 22 Mar 2004 00:58:52 -0000 1.3.2.3 +++ testrmm.c 22 Sep 2004 09:14:42 -0000 1.3.2.4 @@ -51,7 +51,7 @@ } /* We're going to want 10 blocks of data from our target rmm. */ - size = SHARED_SIZE + apr_rmm_overhead_get(FRAG_COUNT); + size = SHARED_SIZE + apr_rmm_overhead_get(FRAG_COUNT + 1); printf("Creating anonymous shared memory (%" APR_SIZE_T_FMT " bytes).....", size); rv = apr_shm_create(&shm, size, NULL, pool); @@ -87,6 +87,24 @@ else { return APR_EGENERAL; } + + printf("Checking each fragment for address alignment....."); + for (i = 0; i < FRAG_COUNT; i++) { + char *c = apr_rmm_addr_get(rmm, off[i]); + apr_size_t sc = (apr_size_t)c; + + if (off[i] == 0) { + printf("allocation failed for offset %d\n", i); + return APR_ENOMEM; + } + + if (sc & 7) { + printf("Bad alignment for fragment %d; %p not %p!\n", + i, c, (void *)APR_ALIGN_DEFAULT((apr_size_t)c)); + return APR_EGENERAL; + } + } + fprintf(stdout, "OK\n"); printf("Setting each fragment to a unique value.........."); for (i = 0; i < FRAG_COUNT; i++) { No revision No revision 1.20.2.5 +23 -24 apr-util/misc/apr_rmm.c Index: apr_rmm.c =================================================================== RCS file: /home/cvs/apr-util/misc/apr_rmm.c,v retrieving revision 1.20.2.4 retrieving revision 1.20.2.5 diff -d -w -u -r1.20.2.4 -r1.20.2.5 --- apr_rmm.c 22 Mar 2004 00:58:52 -0000 1.20.2.4 +++ apr_rmm.c 22 Sep 2004 09:14:42 -0000 1.20.2.5 @@ -33,6 +33,9 @@ apr_rmm_off_t /* rmm_block_t */ firstfree; } rmm_hdr_block_t; +#define RMM_HDR_BLOCK_SIZE (APR_ALIGN_DEFAULT(sizeof(rmm_hdr_block_t))) +#define RMM_BLOCK_SIZE (APR_ALIGN_DEFAULT(sizeof(rmm_block_t))) + struct apr_rmm_t { apr_pool_t *p; rmm_hdr_block_t *base; @@ -87,7 +90,7 @@ next = blk->next; } - if (bestsize > sizeof(rmm_block_t) + size) { + if (bestsize > RMM_BLOCK_SIZE + size) { struct rmm_block_t *blk = (rmm_block_t*)((char*)rmm->base + best); struct rmm_block_t *new = (rmm_block_t*)((char*)rmm->base + best + size); @@ -104,13 +107,9 @@ } } - if (best) { return best; } - return 0; -} - static void move_block(apr_rmm_t *rmm, apr_rmm_off_t this, int free) { struct rmm_block_t *blk = (rmm_block_t*)((char*)rmm->base + this); @@ -205,7 +204,7 @@ (*rmm)->base->abssize = size; (*rmm)->base->firstused = 0; - (*rmm)->base->firstfree = sizeof(rmm_hdr_block_t); + (*rmm)->base->firstfree = RMM_HDR_BLOCK_SIZE; blk = (rmm_block_t *)((char*)base + (*rmm)->base->firstfree); @@ -213,8 +212,7 @@ blk->prev = 0; blk->next = 0; - APR_ANYLOCK_UNLOCK(lock); - return APR_SUCCESS; + return APR_ANYLOCK_UNLOCK(lock); } APU_DECLARE(apr_status_t) apr_rmm_destroy(apr_rmm_t *rmm) @@ -247,8 +245,7 @@ rmm->base->abssize = 0; rmm->size = 0; - APR_ANYLOCK_UNLOCK(&rmm->lock); - return APR_SUCCESS; + return APR_ANYLOCK_UNLOCK(&rmm->lock); } APU_DECLARE(apr_status_t) apr_rmm_attach(apr_rmm_t **rmm, apr_anylock_t *lock, @@ -281,15 +278,15 @@ { apr_rmm_off_t this; - reqsize = APR_ALIGN_DEFAULT(reqsize); + reqsize = APR_ALIGN_DEFAULT(reqsize) + RMM_BLOCK_SIZE; APR_ANYLOCK_LOCK(&rmm->lock); - this = find_block_of_size(rmm, reqsize + sizeof(rmm_block_t)); + this = find_block_of_size(rmm, reqsize); if (this) { move_block(rmm, this, 0); - this += sizeof(rmm_block_t); + this += RMM_BLOCK_SIZE; } APR_ANYLOCK_UNLOCK(&rmm->lock); @@ -300,16 +297,16 @@ { apr_rmm_off_t this; - reqsize = APR_ALIGN_DEFAULT(reqsize); + reqsize = APR_ALIGN_DEFAULT(reqsize) + RMM_BLOCK_SIZE; APR_ANYLOCK_LOCK(&rmm->lock); - this = find_block_of_size(rmm, reqsize + sizeof(rmm_block_t)); + this = find_block_of_size(rmm, reqsize); if (this) { move_block(rmm, this, 0); - this += sizeof(rmm_block_t); - memset((char*)rmm->base + this, 0, reqsize); + this += RMM_BLOCK_SIZE; + memset((char*)rmm->base + this, 0, reqsize - RMM_BLOCK_SIZE); } APR_ANYLOCK_UNLOCK(&rmm->lock); @@ -332,7 +329,7 @@ old = apr_rmm_offset_get(rmm, entity); if ((this = apr_rmm_malloc(rmm, reqsize)) == 0) { - return this; + return 0; } blk = (rmm_block_t*)((char*)rmm->base + old); @@ -353,11 +350,11 @@ /* A little sanity check is always healthy, especially here. * If we really cared, we could make this compile-time */ - if (this < sizeof(rmm_hdr_block_t) + sizeof(rmm_block_t)) { + if (this < RMM_HDR_BLOCK_SIZE + RMM_BLOCK_SIZE) { return APR_EINVAL; } - this -= sizeof(rmm_block_t); + this -= RMM_BLOCK_SIZE; blk = (rmm_block_t*)((char*)rmm->base + this); @@ -390,8 +387,7 @@ */ move_block(rmm, this, 1); - APR_ANYLOCK_UNLOCK(&rmm->lock); - return APR_SUCCESS; + return APR_ANYLOCK_UNLOCK(&rmm->lock); } APU_DECLARE(void *) apr_rmm_addr_get(apr_rmm_t *rmm, apr_rmm_off_t entity) @@ -413,5 +409,8 @@ APU_DECLARE(apr_size_t) apr_rmm_overhead_get(int n) { - return sizeof(rmm_hdr_block_t) + n * sizeof(rmm_block_t); + /* overhead per block is at most APR_ALIGN_DEFAULT(1) wasted bytes + * for alignment overhead, plus the size of the rmm_block_t + * structure. */ + return RMM_HDR_BLOCK_SIZE + n * (RMM_BLOCK_SIZE + APR_ALIGN_DEFAULT(1)); }